Skip to content

Commit d9a2cf5

Browse files
authored
Fix reentrant assert failures in session resumption with custom executions (#5844)
When `MsQuicLib.CustomExecutions` is enabled, all `MsQuicSetParam` calls execute inline with `Connection->State.InlineApiExecution = TRUE` (see `api.c:1666`). For `QUIC_PARAM_CONN_RESUMPTION_TICKET`, this triggers a debug assertion failure at `connection.c:693`: ```c CXPLAT_DBG_ASSERT(!Connection->State.InlineApiExecution || Connection->State.HandleClosed); ``` The assertion fires because `QuicConnIndicateEvent` is called while `InlineApiExecution` is `TRUE` and `HandleClosed` is `FALSE`. Two distinct call paths trigger this: ### Path 1: Via streams-available indication ``` MsQuicSetParam(QUIC_PARAM_CONN_RESUMPTION_TICKET) -> InlineApiExecution = TRUE -> QuicConnProcessPeerTransportParameters(FromResumptionTicket=TRUE) -> QuicStreamSetInitializeTransportParameters(FlushIfUnblocked=FALSE) -> QuicStreamSetIndicateStreamsAvailable -> QuicConnIndicateEvent <-- ASSERT ``` ### Path 2: Via datagram state change ``` MsQuicSetParam(QUIC_PARAM_CONN_RESUMPTION_TICKET) -> InlineApiExecution = TRUE -> QuicConnProcessPeerTransportParameters(FromResumptionTicket=TRUE) -> QuicDatagramOnSendStateChanged -> QuicConnIndicateEvent <-- ASSERT ``` ## Fix Relax the assertion to not check app callback reentrancy when custom execution is enabled, per suggestion by @guhetier.
1 parent 5cffb35 commit d9a2cf5

File tree

2 files changed

+10
-4
lines changed

2 files changed

+10
-4
lines changed

src/core/connection.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -692,11 +692,14 @@ QuicConnIndicateEvent(
692692
// MsQuic shouldn't indicate reentrancy to the app when at all possible.
693693
// The general exception to this rule is when the connection is being
694694
// closed because the API MUST block until all work is completed, so we
695-
// have to execute the event callbacks inline.
695+
// have to execute the event callbacks inline. Custom executions always
696+
// have InlineApiExecution set, which makes this reentrancy check
697+
// unreliable, so it is skipped for custom executions.
696698
//
697699
CXPLAT_DBG_ASSERT(
698700
!Connection->State.InlineApiExecution ||
699-
Connection->State.HandleClosed);
701+
Connection->State.HandleClosed ||
702+
MsQuicLib.CustomExecutions);
700703
Status =
701704
Connection->ClientCallbackHandler(
702705
(HQUIC)Connection,

src/core/stream.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -470,13 +470,16 @@ QuicStreamIndicateEvent(
470470
// or stream is being closed because the API MUST block until all work
471471
// is completed, so we have to execute the event callbacks inline. There
472472
// is also one additional exception for start complete when StreamStart
473-
// is called synchronously on an MsQuic thread.
473+
// is called synchronously on an MsQuic thread. Custom executions
474+
// always have InlineApiExecution set, which makes this reentrancy
475+
// check unreliable, so it is skipped for custom executions.
474476
//
475477
CXPLAT_DBG_ASSERT(
476478
!Stream->Connection->State.InlineApiExecution ||
477479
Stream->Connection->State.HandleClosed ||
478480
Stream->Flags.HandleClosed ||
479-
Event->Type == QUIC_STREAM_EVENT_START_COMPLETE);
481+
Event->Type == QUIC_STREAM_EVENT_START_COMPLETE ||
482+
MsQuicLib.CustomExecutions);
480483
Status =
481484
Stream->ClientCallbackHandler(
482485
(HQUIC)Stream,

0 commit comments

Comments
 (0)