Skip to content

Commit 6765f86

Browse files
guhetiermtfriesen
andauthored
Ensure async task completes before sending packets (#5765)
## Description Spurious failures of Misc/WithReceiveResumeNoDataArgs.ReceiveResumeNoData/2 have been observed in the OS repository. From traces, this is due to a race between `StreamReceiveSetEnabled` (processed async) and receiving the shutdown frame sent by the client: it was possible the shutdown frame was indicated by the server before it blocks receives. ## Testing CI. This is a rare race, so validation is mostly for non-regression. ## Documentation N/A --------- Co-authored-by: Michael Friesen <3517159+mtfriesen@users.noreply.github.com>
1 parent cd94771 commit 6765f86

File tree

2 files changed

+36
-1
lines changed

2 files changed

+36
-1
lines changed

src/test/lib/DataTest.cpp

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1989,6 +1989,11 @@ QuicTestReceiveResume(
19891989
TEST_FAILURE("PauseFirst MsQuic->StreamReceiveSetEnabled(FALSE) failed, 0x%x", Status);
19901990
return;
19911991
}
1992+
1993+
//
1994+
// Ensure the receive pause task is processed.
1995+
//
1996+
DrainConnectionWorkQueue(ServerContext.Conn.Handle);
19921997
}
19931998

19941999
Status =
@@ -2206,6 +2211,9 @@ QuicTestReceiveResumeNoData(
22062211
return;
22072212
}
22082213

2214+
//
2215+
// Disable receive notifications on the server.
2216+
//
22092217
Status =
22102218
MsQuic->StreamReceiveSetEnabled(
22112219
ServerContext.Stream.Handle,
@@ -2215,6 +2223,12 @@ QuicTestReceiveResumeNoData(
22152223
return;
22162224
}
22172225

2226+
//
2227+
// Ensure the StreamReceiveSetEnabled task is processed so it doesn't race with receiving
2228+
// the shutdown.
2229+
//
2230+
DrainConnectionWorkQueue(ServerContext.Conn.Handle);
2231+
22182232
Status =
22192233
MsQuic->StreamShutdown(
22202234
ClientContext.Stream.Handle,
@@ -2227,10 +2241,18 @@ QuicTestReceiveResumeNoData(
22272241
}
22282242

22292243
if (ShutdownType == GracefulShutdown) {
2244+
//
2245+
// On a graceful shutdown, the shutdown notification is emitted only after all data
2246+
// was delivered (in this case, zero bytes and a FIN).
2247+
//
22302248
if (CxPlatEventWaitWithTimeout(ServerContext.TestEvent.Handle, TimeoutMs)) {
22312249
TEST_FAILURE("Server got shutdown event when it shouldn't have!");
22322250
return;
22332251
}
2252+
2253+
//
2254+
// Re-enable receive notifications.
2255+
//
22342256
Status =
22352257
MsQuic->StreamReceiveSetEnabled(
22362258
ServerContext.Stream.Handle,
@@ -2242,7 +2264,7 @@ QuicTestReceiveResumeNoData(
22422264
}
22432265

22442266
//
2245-
// Validate the test was shutdown as expected.
2267+
// Validate the stream was shutdown as expected.
22462268
//
22472269
if (!CxPlatEventWaitWithTimeout(ServerContext.TestEvent.Handle, TimeoutMs)) {
22482270
TEST_FAILURE("Server failed to get shutdown before timeout!");

src/test/lib/TestHelpers.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1063,3 +1063,16 @@ WaitForMsQuicInUse() {
10631063

10641064
return MsQuicInUse && Status == QUIC_STATUS_SUCCESS;
10651065
}
1066+
1067+
_IRQL_requires_max_(PASSIVE_LEVEL)
1068+
QUIC_INLINE
1069+
void
1070+
DrainConnectionWorkQueue(HQUIC Connection) {
1071+
//
1072+
// Call the GetParam API: it schedules a work item on the connection and waits for its completion,
1073+
// ensuring all prior work items have completed.
1074+
//
1075+
QUIC_STATISTICS_V2 Stats{};
1076+
uint32_t StatsSize = sizeof(Stats);
1077+
(void)MsQuic->GetParam(Connection, QUIC_PARAM_CONN_STATISTICS_V2, &StatsSize, &Stats);
1078+
}

0 commit comments

Comments
 (0)