Skip to content

Commit 07a6866

Browse files
committed
Relax reentrant assertion for custom executions instead of queuing notifications
1 parent 2912976 commit 07a6866

File tree

4 files changed

+11
-55
lines changed

4 files changed

+11
-55
lines changed

src/core/connection.c

Lines changed: 7 additions & 29 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 also
696+
// allow reentrancy because set/get param must be executed inline to not
697+
// block the thread, and the app is expected to handle reentrancy.
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,
@@ -3087,20 +3090,9 @@ QuicConnProcessPeerTransportParameters(
30873090
&Connection->Streams,
30883091
Connection->PeerTransportParams.InitialMaxBidiStreams,
30893092
Connection->PeerTransportParams.InitialMaxUniStreams,
3090-
FromResumptionTicket);
3093+
!FromResumptionTicket);
30913094

3092-
if (FromResumptionTicket) {
3093-
//
3094-
// Defer the datagram state change notification to avoid reentrant
3095-
// callbacks when called from the resumption ticket path.
3096-
//
3097-
QUIC_OPERATION* Oper;
3098-
if ((Oper = QuicConnAllocOperation(Connection, QUIC_OPER_TYPE_DATAGRAM_STATE_CHANGED)) != NULL) {
3099-
QuicConnQueueOper(Connection, Oper);
3100-
}
3101-
} else {
3102-
QuicDatagramOnSendStateChanged(&Connection->Datagram);
3103-
}
3095+
QuicDatagramOnSendStateChanged(&Connection->Datagram);
31043096

31053097
if (Connection->State.Started) {
31063098
if (Connection->State.Disable1RttEncrytion &&
@@ -7970,20 +7962,6 @@ QuicConnDrainOperations(
79707962
Connection, Oper->ROUTE.PhysicalAddress, Oper->ROUTE.PathId, Oper->ROUTE.Succeeded);
79717963
break;
79727964

7973-
case QUIC_OPER_TYPE_STREAMS_AVAILABLE:
7974-
if (Connection->State.ShutdownComplete) {
7975-
break; // Ignore if already shutdown
7976-
}
7977-
QuicStreamSetIndicateStreamsAvailable(&Connection->Streams);
7978-
break;
7979-
7980-
case QUIC_OPER_TYPE_DATAGRAM_STATE_CHANGED:
7981-
if (Connection->State.ShutdownComplete) {
7982-
break; // Ignore if already shutdown
7983-
}
7984-
QuicDatagramOnSendStateChanged(&Connection->Datagram);
7985-
break;
7986-
79877965
default:
79887966
CXPLAT_FRE_ASSERT(FALSE);
79897967
break;

src/core/operation.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,6 @@ typedef enum QUIC_OPERATION_TYPE {
3131
QUIC_OPER_TYPE_TIMER_EXPIRED, // A timer expired.
3232
QUIC_OPER_TYPE_TRACE_RUNDOWN, // A trace rundown was triggered.
3333
QUIC_OPER_TYPE_ROUTE_COMPLETION, // Process route completion event.
34-
QUIC_OPER_TYPE_STREAMS_AVAILABLE, // Indicate streams available to the app.
35-
QUIC_OPER_TYPE_DATAGRAM_STATE_CHANGED, // Indicate datagram state changed to the app.
3634

3735
//
3836
// All stateless operations follow.

src/core/stream_set.c

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ QuicStreamSetInitializeTransportParameters(
347347
_Inout_ QUIC_STREAM_SET* StreamSet,
348348
_In_ uint64_t BidiStreamCount,
349349
_In_ uint64_t UnidiStreamCount,
350-
_In_ BOOLEAN FromResumptionTicket
350+
_In_ BOOLEAN FlushIfUnblocked
351351
)
352352
{
353353
QUIC_CONNECTION* Connection = QuicStreamSetGetConnection(StreamSet);
@@ -424,21 +424,10 @@ QuicStreamSetInitializeTransportParameters(
424424
}
425425

426426
if (UpdateAvailableStreams) {
427-
if (!FromResumptionTicket) {
428-
QuicStreamSetIndicateStreamsAvailable(StreamSet);
429-
} else {
430-
//
431-
// Defer the streams-available notification to avoid reentrant
432-
// callbacks when called from the resumption ticket path.
433-
//
434-
QUIC_OPERATION* Oper;
435-
if ((Oper = QuicConnAllocOperation(Connection, QUIC_OPER_TYPE_STREAMS_AVAILABLE)) != NULL) {
436-
QuicConnQueueOper(Connection, Oper);
437-
}
438-
}
427+
QuicStreamSetIndicateStreamsAvailable(StreamSet);
439428
}
440429

441-
if (MightBeUnblocked && !FromResumptionTicket) {
430+
if (MightBeUnblocked && FlushIfUnblocked) {
442431
//
443432
// We opened the window, so start send. Rather than checking
444433
// the streams to see if one is actually unblocked, we risk starting

src/core/stream_set.h

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -131,16 +131,7 @@ QuicStreamSetInitializeTransportParameters(
131131
_Inout_ QUIC_STREAM_SET* StreamSet,
132132
_In_ uint64_t BidiStreamCount,
133133
_In_ uint64_t UnidiStreamCount,
134-
_In_ BOOLEAN FromResumptionTicket
135-
);
136-
137-
//
138-
// Indicates available streams to the app via a connection event.
139-
//
140-
_IRQL_requires_max_(PASSIVE_LEVEL)
141-
void
142-
QuicStreamSetIndicateStreamsAvailable(
143-
_Inout_ QUIC_STREAM_SET* StreamSet
134+
_In_ BOOLEAN FlushIfUnblocked
144135
);
145136

146137
//

0 commit comments

Comments
 (0)