Skip to content

Commit 8c8c0e5

Browse files
authored
Fix a race condition in TCP epoll code (#4993)
t's a race between PerfClientWorker::WorkerThread and TcpWorker::WorkerThread. In the middle of creating a socket from the perf client worker thread, as long as epoll has been told to monitor the socket, a notification could come up at the same time from tcp worker thread and frees the connection. #1 0x00005b7e577eec9f in CxPlatSocketContextSetEvents #2 0x00005b7e577ef725 in CxPlatSocketCreateTcpInternal #3 0x00005b7e577efb8a in SocketCreateTcp #4 0x00005b7e577ebdb9 in CxPlatSocketCreateTcp #5 0x00005b7e577db274 in TcpConnection::Start #6 0x00005b7e577def35 in PerfClientConnection::Initialize #7 0x00005b7e577df124 in PerfClientWorker::StartNewConnection #8 0x00005b7e577df140 in PerfClientWorker::WorkerThread The fix is moving the actual connection start to the tcp worker thread.
1 parent 4d6e1f1 commit 8c8c0e5

File tree

2 files changed

+18
-9
lines changed

2 files changed

+18
-9
lines changed

Diff for: src/perf/lib/Tcp.cpp

+17-9
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,7 @@ TcpConnection::TcpConnection(
404404
WriteOutput("SecConfig load FAILED\n");
405405
return;
406406
}
407+
QuicAddrSetFamily(&Route.LocalAddress, QUIC_ADDRESS_FAMILY_UNSPEC);
407408
Initialized = true;
408409
}
409410

@@ -422,7 +423,9 @@ TcpConnection::Start(
422423
}
423424
if (LocalAddress) {
424425
Family = QuicAddrGetFamily(LocalAddress);
426+
Route.LocalAddress = *LocalAddress;
425427
}
428+
426429
if (RemoteAddress) {
427430
Route.RemoteAddress = *RemoteAddress;
428431
} else {
@@ -437,15 +440,7 @@ TcpConnection::Start(
437440
}
438441
}
439442
QuicAddrSetPort(&Route.RemoteAddress, ServerPort);
440-
if (QUIC_FAILED(
441-
CxPlatSocketCreateTcp(
442-
Datapath,
443-
LocalAddress,
444-
&Route.RemoteAddress,
445-
this,
446-
&Socket))) {
447-
return false;
448-
}
443+
ConnStartQueued = true;
449444
Queue();
450445
return true;
451446
}
@@ -627,6 +622,19 @@ TcpConnection::TlsReceiveTicketCallback(
627622

628623
void TcpConnection::Process()
629624
{
625+
if (ConnStartQueued) {
626+
ConnStartQueued = false;
627+
if (QUIC_FAILED(
628+
CxPlatSocketCreateTcp(
629+
Datapath,
630+
QuicAddrGetFamily(&Route.LocalAddress) == QUIC_ADDRESS_FAMILY_UNSPEC ?
631+
nullptr : &Route.LocalAddress,
632+
&Route.RemoteAddress,
633+
this,
634+
&Socket))) {
635+
Shutdown = true;
636+
}
637+
}
630638
if (IndicateAccept) {
631639
IndicateAccept = false;
632640
TcpServer* Server = (TcpServer*)Context;

Diff for: src/perf/lib/Tcp.h

+1
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@ class TcpConnection {
183183
bool Closed{false};
184184
bool QueuedOnWorker{false};
185185
bool StartTls{false};
186+
bool ConnStartQueued{false};
186187
bool IndicateAccept{false};
187188
bool IndicateConnect{false};
188189
bool IndicateSendComplete{false};

0 commit comments

Comments
 (0)