From 6e1c3d6af6ecade0979ec3430221c8920e562e91 Mon Sep 17 00:00:00 2001 From: Nick Banks Date: Thu, 30 Nov 2023 13:25:33 -0500 Subject: [PATCH] Support Using Streams after Connection Closure (#3938) --- src/core/connection.c | 250 ++++++++---------- src/core/connection.h | 17 +- src/core/listener.c | 4 +- src/core/timer_wheel.c | 172 ++++++------ src/core/worker.c | 12 +- src/generated/linux/connection.c.clog.h | 76 +++--- .../linux/connection.c.clog.h.lttng.h | 84 +++--- src/generated/linux/timer_wheel.c.clog.h | 8 +- .../linux/timer_wheel.c.clog.h.lttng.h | 8 +- src/plugins/dbg/analyze.cpp | 4 +- src/plugins/dbg/dbg.vcxproj | 2 +- src/plugins/dbg/dump.cpp | 4 +- src/plugins/dbg/quictypes.h | 11 +- src/plugins/dbg/registration.cpp | 9 +- src/plugins/dbg/worker.cpp | 4 +- src/test/MsQuicTests.h | 6 +- src/test/bin/quic_gtest.cpp | 9 + src/test/bin/winkernel/control.cpp | 6 +- src/test/lib/ApiTest.cpp | 13 +- src/test/lib/OwnershipTest.cpp | 46 ++++ src/tools/spin/spinquic.cpp | 6 +- 21 files changed, 392 insertions(+), 359 deletions(-) diff --git a/src/core/connection.c b/src/core/connection.c index 8ce96c451..f69be6963 100644 --- a/src/core/connection.c +++ b/src/core/connection.c @@ -278,7 +278,6 @@ QuicConnAlloc( Error: Connection->State.HandleClosed = TRUE; - Connection->State.Uninitialized = TRUE; for (uint32_t i = 0; i < ARRAYSIZE(Connection->Packets); i++) { if (Connection->Packets[i] != NULL) { QuicPacketSpaceUninitialize(Connection->Packets[i]); @@ -317,16 +316,13 @@ QuicConnFree( CXPLAT_TEL_ASSERT(Connection->RefCount == 0); if (Connection->State.ExternalOwner) { CXPLAT_TEL_ASSERT(Connection->State.HandleClosed); - CXPLAT_TEL_ASSERT(Connection->State.Uninitialized); - CXPLAT_DBG_ASSERT(!Connection->State.Registered); } CXPLAT_TEL_ASSERT(Connection->SourceCids.Next == NULL); CXPLAT_TEL_ASSERT(CxPlatListIsEmpty(&Connection->Streams.ClosedStreams)); + QuicRangeUninitialize(&Connection->DecodedAckRanges); + QuicCryptoUninitialize(&Connection->Crypto); QuicLossDetectionUninitialize(&Connection->LossDetection); QuicSendUninitialize(&Connection->Send); - // - // Free up packet space if it wasn't freed by QuicConnUninitialize - // for (uint32_t i = 0; i < ARRAYSIZE(Connection->Packets); i++) { if (Connection->Packets[i] != NULL) { QuicPacketSpaceUninitialize(Connection->Packets[i]); @@ -351,18 +347,9 @@ QuicConnFree( Link); CXPLAT_FREE(CID, QUIC_POOL_CIDLIST); } - if (Connection->State.Registered) { - CxPlatDispatchLockAcquire(&Connection->Registration->ConnectionLock); - CxPlatListEntryRemove(&Connection->RegistrationLink); - CxPlatDispatchLockRelease(&Connection->Registration->ConnectionLock); - Connection->State.Registered = FALSE; - QuicTraceEvent( - ConnUnregistered, - "[conn][%p] Unregistered from %p", - Connection, - Connection->Registration); - } + QuicConnUnregister(Connection); if (Connection->Worker != NULL) { + QuicTimerWheelRemoveConnection(&Connection->Worker->TimerWheel, Connection); QuicOperationQueueClear(Connection->Worker, &Connection->OperQ); } if (Connection->ReceiveQueue != NULL) { @@ -411,6 +398,9 @@ QuicConnFree( if (Connection->Registration != NULL) { CxPlatRundownRelease(&Connection->Registration->Rundown); } + if (Connection->CloseReasonPhrase != NULL) { + CXPLAT_FREE(Connection->CloseReasonPhrase, QUIC_POOL_CLOSE_REASON); + } Connection->State.Freed = TRUE; QuicTraceEvent( ConnDestroyed, @@ -450,67 +440,6 @@ QuicConnShutdown( QuicConnCloseLocally(Connection, CloseFlags, ErrorCode, NULL); } -_IRQL_requires_max_(PASSIVE_LEVEL) -void -QuicConnUninitialize( - _In_ QUIC_CONNECTION* Connection - ) -{ - CXPLAT_TEL_ASSERT(Connection->State.HandleClosed); - CXPLAT_TEL_ASSERT(!Connection->State.Uninitialized); - - Connection->State.Uninitialized = TRUE; - Connection->State.UpdateWorker = FALSE; - - // - // Ensure we are shut down. - // - QuicConnShutdown( - Connection, - QUIC_CONNECTION_SHUTDOWN_FLAG_SILENT, - QUIC_ERROR_NO_ERROR, - FALSE, - FALSE); - - // - // Remove all entries in the binding's lookup tables so we don't get any - // more packets queued. - // - QUIC_PATH* Path = &Connection->Paths[0]; - if (Path->Binding != NULL) { - if (Path->EncryptionOffloading) { - QuicPathUpdateQeo(Connection, Path, CXPLAT_QEO_OPERATION_REMOVE); - } - - QuicBindingRemoveConnection(Connection->Paths[0].Binding, Connection); - } - - // - // Clean up the packet space first, to return any deferred received - // packets back to the binding. - // - for (uint32_t i = 0; i < ARRAYSIZE(Connection->Packets); i++) { - if (Connection->Packets[i] != NULL) { - QuicPacketSpaceUninitialize(Connection->Packets[i]); - Connection->Packets[i] = NULL; - } - } - - // - // Clean up the rest of the internal state. - // - QuicRangeUninitialize(&Connection->DecodedAckRanges); - QuicCryptoUninitialize(&Connection->Crypto); - QuicTimerWheelRemoveConnection(&Connection->Worker->TimerWheel, Connection); - QuicOperationQueueClear(Connection->Worker, &Connection->OperQ); - QuicLossDetectionUninitialize(&Connection->LossDetection); - QuicSendUninitialize(&Connection->Send); - - if (Connection->CloseReasonPhrase != NULL) { - CXPLAT_FREE(Connection->CloseReasonPhrase, QUIC_POOL_CLOSE_REASON); - } -} - _IRQL_requires_max_(DISPATCH_LEVEL) void QuicConnCloseHandle( @@ -526,23 +455,11 @@ QuicConnCloseHandle( (uint64_t)QUIC_STATUS_ABORTED, NULL); - if (Connection->State.SendShutdownCompleteNotif) { + if (Connection->State.ProcessShutdownComplete) { QuicConnOnShutdownComplete(Connection); } - Connection->ClientCallbackHandler = NULL; - - if (Connection->State.Registered) { - CxPlatDispatchLockAcquire(&Connection->Registration->ConnectionLock); - CxPlatListEntryRemove(&Connection->RegistrationLink); - CxPlatDispatchLockRelease(&Connection->Registration->ConnectionLock); - Connection->State.Registered = FALSE; - QuicTraceEvent( - ConnUnregistered, - "[conn][%p] Unregistered from %p", - Connection, - Connection->Registration); - } + QuicConnUnregister(Connection); QuicTraceEvent( ConnHandleClosed, @@ -551,14 +468,12 @@ QuicConnCloseHandle( } _IRQL_requires_max_(DISPATCH_LEVEL) -_Must_inspect_result_ -BOOLEAN -QuicConnRegister( - _Inout_ QUIC_CONNECTION* Connection, - _Inout_ QUIC_REGISTRATION* Registration +void +QuicConnUnregister( + _Inout_ QUIC_CONNECTION* Connection ) { - if (Connection->Registration != NULL) { + if (Connection->State.Registered) { CxPlatDispatchLockAcquire(&Connection->Registration->ConnectionLock); CxPlatListEntryRemove(&Connection->RegistrationLink); CxPlatDispatchLockRelease(&Connection->Registration->ConnectionLock); @@ -572,9 +487,19 @@ QuicConnRegister( Connection->Registration = NULL; Connection->State.Registered = FALSE; } +} - BOOLEAN Success = CxPlatRundownAcquire(&Registration->Rundown); - if (!Success) { +_IRQL_requires_max_(DISPATCH_LEVEL) +_Must_inspect_result_ +BOOLEAN +QuicConnRegister( + _Inout_ QUIC_CONNECTION* Connection, + _Inout_ QUIC_REGISTRATION* Registration + ) +{ + QuicConnUnregister(Connection); + + if (!CxPlatRundownAcquire(&Registration->Rundown)) { return FALSE; } Connection->State.Registered = TRUE; @@ -758,7 +683,7 @@ QuicConnIndicateEvent( QUIC_CONN_VERIFY( Connection, Connection->State.HandleClosed || - Connection->State.HandleShutdown || + Connection->State.ShutdownComplete || !Connection->State.ExternalOwner); Status = QUIC_STATUS_INVALID_STATE; QuicTraceLogConnWarning( @@ -1455,11 +1380,12 @@ QuicConnOnShutdownComplete( _In_ QUIC_CONNECTION* Connection ) { - Connection->State.SendShutdownCompleteNotif = FALSE; - if (Connection->State.HandleShutdown) { + Connection->State.ProcessShutdownComplete = FALSE; + if (Connection->State.ShutdownComplete) { return; } - Connection->State.HandleShutdown = TRUE; + Connection->State.ShutdownComplete = TRUE; + Connection->State.UpdateWorker = FALSE; QuicTraceEvent( ConnShutdownComplete, @@ -1467,18 +1393,7 @@ QuicConnOnShutdownComplete( Connection, Connection->State.ShutdownCompleteTimedOut); - if (Connection->State.ExternalOwner == FALSE) { - - // - // If the connection was never indicated to the application, then it - // needs to be cleaned up now. - // - - QuicConnCloseHandle(Connection); - QuicConnUninitialize(Connection); - QuicConnRelease(Connection, QUIC_CONN_REF_HANDLE_OWNER); - - } else { + if (Connection->State.ExternalOwner) { QUIC_CONNECTION_EVENT Event; Event.Type = QUIC_CONNECTION_EVENT_SHUTDOWN_COMPLETE; @@ -1497,6 +1412,38 @@ QuicConnOnShutdownComplete( Connection->ClientCallbackHandler = NULL; } + + // + // Clean up any pending state that is irrelevant now. + // + QUIC_PATH* Path = &Connection->Paths[0]; + if (Path->Binding != NULL) { + if (Path->EncryptionOffloading) { + QuicPathUpdateQeo(Connection, Path, CXPLAT_QEO_OPERATION_REMOVE); + } + + // + // Remove all entries in the binding's lookup tables so we don't get any + // more packets queued. + // + QuicBindingRemoveConnection(Connection->Paths[0].Binding, Connection); + } + + // + // Clean up the rest of the internal state. + // + QuicTimerWheelRemoveConnection(&Connection->Worker->TimerWheel, Connection); + QuicLossDetectionUninitialize(&Connection->LossDetection); + QuicSendUninitialize(&Connection->Send); + + if (!Connection->State.ExternalOwner) { + // + // If the connection was never indicated to the application, then the + // "owner" ref still resides with the stack and needs to be released. + // + QuicConnUnregister(Connection); + QuicConnRelease(Connection, QUIC_CONN_REF_HANDLE_OWNER); + } } QUIC_STATUS @@ -1546,7 +1493,7 @@ QuicConnTryClose( // Silent close forced after we already started the close process. // Connection->State.ShutdownCompleteTimedOut = FALSE; - Connection->State.SendShutdownCompleteNotif = TRUE; + Connection->State.ProcessShutdownComplete = TRUE; } return; } @@ -1555,6 +1502,17 @@ QuicConnTryClose( Connection->State.ClosedRemotely = TRUE; } else { Connection->State.ClosedLocally = TRUE; + if (!Connection->State.ExternalOwner) { + // + // Don't continue processing the connection, since it has been + // closed locally and it's not referenced externally. + // + QuicTraceLogConnVerbose( + AbandonInternallyClosed, + Connection, + "Abandoning internal, closed connection"); + Connection->State.ProcessShutdownComplete = TRUE; + } } if (!ClosedRemotely) { @@ -1766,7 +1724,7 @@ QuicConnTryClose( if (SilentClose || (Connection->State.ClosedRemotely && Connection->State.ClosedLocally)) { Connection->State.ShutdownCompleteTimedOut = FALSE; - Connection->State.SendShutdownCompleteNotif = TRUE; + Connection->State.ProcessShutdownComplete = TRUE; } } @@ -1786,7 +1744,7 @@ QuicConnProcessShutdownTimerOperation( // Now that we are closed in both directions, we can complete the shutdown // of the connection. // - Connection->State.SendShutdownCompleteNotif = TRUE; + Connection->State.ProcessShutdownComplete = TRUE; } _IRQL_requires_max_(PASSIVE_LEVEL) @@ -5367,7 +5325,7 @@ Done: QuicConnLogOutFlowStats(Connection); } - if (Connection->State.HandleShutdown || Connection->State.HandleClosed) { + if (Connection->State.ShutdownComplete || Connection->State.HandleClosed) { QuicTraceLogVerbose( PacketRxNotAcked, "[%c][RX][%llu] not acked (connection is closed)", @@ -5879,9 +5837,9 @@ QuicConnRecvDatagrams( } } - if (!Connection->State.UpdateWorker && - Connection->State.Connected && - RecvState.UpdatePartitionId) { + if (!Connection->State.UpdateWorker && Connection->State.Connected && + !Connection->State.ShutdownComplete && RecvState.UpdatePartitionId) { + CXPLAT_DBG_ASSERT(Connection->Registration); CXPLAT_DBG_ASSERT(!Connection->Registration->NoPartitioning); CXPLAT_DBG_ASSERT(RecvState.PartitionIndex != QuicPartitionIdGetIndex(Connection->PartitionID)); Connection->PartitionID = QuicPartitionIdCreate(RecvState.PartitionIndex); @@ -7625,7 +7583,7 @@ QuicConnDrainOperations( CXPLAT_PASSIVE_CODE(); - if (!Connection->State.Initialized && !Connection->State.Uninitialized) { + if (!Connection->State.Initialized && !Connection->State.ShutdownComplete) { // // TODO - Try to move this only after the connection is accepted by the // listener. But that's going to be pretty complicated. @@ -7649,8 +7607,7 @@ QuicConnDrainOperations( } } - while (!Connection->State.HandleClosed && - !Connection->State.UpdateWorker && + while (!Connection->State.UpdateWorker && OperationCount++ < MaxOperationCount) { Oper = QuicOperationDequeue(&Connection->OperQ); @@ -7673,6 +7630,9 @@ QuicConnDrainOperations( break; case QUIC_OPER_TYPE_FLUSH_RECV: + if (Connection->State.ShutdownComplete) { + break; // Ignore if already shutdown + } if (!QuicConnFlushRecv(Connection)) { // // Still have more data to recv. Put the operation back on the @@ -7684,16 +7644,25 @@ QuicConnDrainOperations( break; case QUIC_OPER_TYPE_UNREACHABLE: + if (Connection->State.ShutdownComplete) { + break; // Ignore if already shutdown + } QuicConnProcessUdpUnreachable( Connection, &Oper->UNREACHABLE.RemoteAddress); break; case QUIC_OPER_TYPE_FLUSH_STREAM_RECV: + if (Connection->State.ShutdownComplete) { + break; // Ignore if already shutdown + } QuicStreamRecvFlush(Oper->FLUSH_STREAM_RECEIVE.Stream); break; case QUIC_OPER_TYPE_FLUSH_SEND: + if (Connection->State.ShutdownComplete) { + break; // Ignore if already shutdown + } if (QuicSendFlush(&Connection->Send)) { // // We have no more data to send out so clear the pending flag. @@ -7710,6 +7679,9 @@ QuicConnDrainOperations( break; case QUIC_OPER_TYPE_TIMER_EXPIRED: + if (Connection->State.ShutdownComplete) { + break; // Ignore if already shutdown + } QuicConnProcessExpiredTimer(Connection, Oper->TIMER_EXPIRED.Type); break; @@ -7718,6 +7690,9 @@ QuicConnDrainOperations( break; case QUIC_OPER_TYPE_ROUTE_COMPLETION: + if (Connection->State.ShutdownComplete) { + break; // Ignore if already shutdown + } QuicConnProcessRouteCompletion( Connection, Oper->ROUTE.PhysicalAddress, Oper->ROUTE.PathId, Oper->ROUTE.Succeeded); break; @@ -7737,19 +7712,11 @@ QuicConnDrainOperations( QuicPerfCounterIncrement(QUIC_PERF_COUNTER_CONN_OPER_COMPLETED); } - if (!Connection->State.ExternalOwner && Connection->State.ClosedLocally) { - // - // Don't continue processing the connection, since it has been closed - // locally and it's not referenced externally. - // - QuicTraceLogConnVerbose( - AbandonInternallyClosed, - Connection, - "Abandoning internal, closed connection"); + if (Connection->State.ProcessShutdownComplete) { QuicConnOnShutdownComplete(Connection); } - if (!Connection->State.HandleClosed) { + if (!Connection->State.ShutdownComplete) { if (OperationCount >= MaxOperationCount && (Connection->Send.SendFlags & QUIC_CONN_SEND_FLAG_ACK)) { // @@ -7759,17 +7726,6 @@ QuicConnDrainOperations( // (void)QuicSendFlush(&Connection->Send); } - - if (Connection->State.SendShutdownCompleteNotif) { - QuicConnOnShutdownComplete(Connection); - } - } - - if (Connection->State.HandleClosed) { - if (!Connection->State.Uninitialized) { - QuicConnUninitialize(Connection); - } - HasMoreWorkToDo = FALSE; } QuicStreamSetDrainClosedStreams(&Connection->Streams); diff --git a/src/core/connection.h b/src/core/connection.h index 11e023e1f..8cbae68ff 100644 --- a/src/core/connection.h +++ b/src/core/connection.h @@ -36,9 +36,8 @@ typedef union QUIC_CONNECTION_STATE { BOOLEAN ClosedLocally : 1; // Locally closed. BOOLEAN ClosedRemotely : 1; // Remotely closed. BOOLEAN AppClosed : 1; // Application (not transport) closed connection. - BOOLEAN HandleShutdown : 1; // Shutdown callback delivered for handle. + BOOLEAN ShutdownComplete : 1; // Shutdown callback delivered for handle. BOOLEAN HandleClosed : 1; // Handle closed by application layer. - BOOLEAN Uninitialized : 1; // Uninitialize started/completed. BOOLEAN Freed : 1; // Freed. Used for Debugging. // @@ -120,9 +119,9 @@ typedef union QUIC_CONNECTION_STATE { BOOLEAN ShutdownCompleteTimedOut : 1; // - // The application needs to be notified of a shutdown complete event. + // The connection is shutdown and the completion for it needs to be run. // - BOOLEAN SendShutdownCompleteNotif : 1; + BOOLEAN ProcessShutdownComplete : 1; // // Indicates whether this connection shares bindings with others. @@ -223,6 +222,7 @@ typedef enum QUIC_CONNECTION_REF { QUIC_CONN_REF_LOOKUP_TABLE, // Per registered CID. QUIC_CONN_REF_LOOKUP_RESULT, // For connections returned from lookups. QUIC_CONN_REF_WORKER, // Worker is (queued for) processing. + QUIC_CONN_REF_TIMER_WHEEL, // The timer wheel is tracking the connection. QUIC_CONN_REF_ROUTE, // Route resolution is undergoing. QUIC_CONN_REF_STREAM, // A stream depends on the connection. @@ -1117,6 +1117,15 @@ QuicConnRegister( _Inout_ QUIC_REGISTRATION* Registration ); +// +// Unregisters the connection from the registration. +// +_IRQL_requires_max_(DISPATCH_LEVEL) +void +QuicConnUnregister( + _Inout_ QUIC_CONNECTION* Connection + ); + // // Tracing rundown for the connection. // diff --git a/src/core/listener.c b/src/core/listener.c index 61dfdd4b5..86f0ae651 100644 --- a/src/core/listener.c +++ b/src/core/listener.c @@ -670,7 +670,9 @@ QuicListenerClaimConnection( Connection->ClientCallbackHandler != NULL, "App MUST set callback handler or close connection!"); - Connection->State.UpdateWorker = TRUE; + if (!Connection->State.ShutdownComplete) { + Connection->State.UpdateWorker = TRUE; + } return !Connection->State.HandleClosed; } diff --git a/src/core/timer_wheel.c b/src/core/timer_wheel.c index 0f8be69dd..65bd99bc2 100644 --- a/src/core/timer_wheel.c +++ b/src/core/timer_wheel.c @@ -110,6 +110,7 @@ QuicTimerWheelUninitialize( StillInTimerWheel, Connection, "Still in timer wheel! Connection was likely leaked!"); + CXPLAT_DBG_ASSERT(!Connection); Entry = Entry->Flink; } CXPLAT_TEL_ASSERT(CxPlatListIsEmpty(&TimerWheel->Slots[i])); @@ -277,6 +278,8 @@ QuicTimerWheelRemoveConnection( if (Connection == TimerWheel->NextConnection) { QuicTimerWheelUpdate(TimerWheel); } + + QuicConnRelease(Connection, QUIC_CONN_REF_TIMER_WHEEL); } } @@ -295,96 +298,100 @@ QuicTimerWheelUpdateConnection( // CxPlatListEntryRemove(&Connection->TimerLink); - if (ExpirationTime == UINT64_MAX) { - TimerWheel->ConnectionCount--; - } + if (ExpirationTime == UINT64_MAX || Connection->State.ShutdownComplete) { + // + // No more timers left, go ahead and invalidate its link. + // + Connection->TimerLink.Flink = NULL; + QuicTraceLogVerbose( + TimerWheelRemoveConnection, + "[time][%p] Removing Connection %p.", + TimerWheel, + Connection); - } else { - - // - // It wasn't in the wheel already, so we must be adding it to the - // wheel. - // - if (ExpirationTime != UINT64_MAX) { - TimerWheel->ConnectionCount++; - } - } - - if (ExpirationTime == UINT64_MAX) { - // - // No more timers left, go ahead and invalidate its link. - // - Connection->TimerLink.Flink = NULL; - QuicTraceLogVerbose( - TimerWheelRemoveConnection, - "[time][%p] Removing Connection %p.", - TimerWheel, - Connection); - - if (Connection == TimerWheel->NextConnection) { - QuicTimerWheelUpdate(TimerWheel); - } - - } else { - - CXPLAT_DBG_ASSERT(TimerWheel->SlotCount != 0); - uint32_t SlotIndex = TIME_TO_SLOT_INDEX(TimerWheel, ExpirationTime); - - // - // Insert the connection into the slot, in the correct order. We search - // the slot's list in reverse order, with the assumption that most new - // timers will on average be later than existing ones. - // - CXPLAT_LIST_ENTRY* ListHead = &TimerWheel->Slots[SlotIndex]; - CXPLAT_LIST_ENTRY* Entry = ListHead->Blink; - - while (Entry != ListHead) { - QUIC_CONNECTION* ConnectionEntry = - CXPLAT_CONTAINING_RECORD(Entry, QUIC_CONNECTION, TimerLink); - uint64_t EntryExpirationTime = QuicConnGetNextExpirationTime(ConnectionEntry); - - if (ExpirationTime > EntryExpirationTime) { - break; + if (Connection == TimerWheel->NextConnection) { + QuicTimerWheelUpdate(TimerWheel); } - Entry = Entry->Blink; + QuicConnRelease(Connection, QUIC_CONN_REF_TIMER_WHEEL); + TimerWheel->ConnectionCount--; + return; // Nothing else to do. } + } else if (ExpirationTime != UINT64_MAX && !Connection->State.ShutdownComplete) { // - // Insert after the current entry. + // It wasn't in the wheel already, so we must be adding it to the wheel. // - CxPlatListInsertHead(Entry, &Connection->TimerLink); + TimerWheel->ConnectionCount++; + QuicConnAddRef(Connection, QUIC_CONN_REF_TIMER_WHEEL); + } else { + return; // Ignore + } + + // + // We are adding/updating the connection in the timer wheel. + // + + CXPLAT_DBG_ASSERT(ExpirationTime != UINT64_MAX); + CXPLAT_DBG_ASSERT(!Connection->State.ShutdownComplete); + CXPLAT_DBG_ASSERT(TimerWheel->SlotCount != 0); + uint32_t SlotIndex = TIME_TO_SLOT_INDEX(TimerWheel, ExpirationTime); + + // + // Insert the connection into the slot, in the correct order. We search + // the slot's list in reverse order, with the assumption that most new + // timers will on average be later than existing ones. + // + CXPLAT_LIST_ENTRY* ListHead = &TimerWheel->Slots[SlotIndex]; + CXPLAT_LIST_ENTRY* Entry = ListHead->Blink; + + while (Entry != ListHead) { + QUIC_CONNECTION* ConnectionEntry = + CXPLAT_CONTAINING_RECORD(Entry, QUIC_CONNECTION, TimerLink); + uint64_t EntryExpirationTime = QuicConnGetNextExpirationTime(ConnectionEntry); + + if (ExpirationTime > EntryExpirationTime) { + break; + } + + Entry = Entry->Blink; + } + + // + // Insert after the current entry. + // + CxPlatListInsertHead(Entry, &Connection->TimerLink); + + QuicTraceLogVerbose( + TimerWheelUpdateConnection, + "[time][%p] Updating Connection %p.", + TimerWheel, + Connection); + + // + // Make sure the next expiration time/connection is still correct. + // + if (ExpirationTime < TimerWheel->NextExpirationTime) { + TimerWheel->NextExpirationTime = ExpirationTime; + TimerWheel->NextConnection = Connection; QuicTraceLogVerbose( - TimerWheelUpdateConnection, - "[time][%p] Updating Connection %p.", + TimerWheelNextExpiration, + "[time][%p] Next Expiration = {%llu, %p}.", TimerWheel, + ExpirationTime, Connection); + } else if (Connection == TimerWheel->NextConnection) { + QuicTimerWheelUpdate(TimerWheel); + } - // - // Make sure the next expiration time/connection is still correct. - // - if (ExpirationTime < TimerWheel->NextExpirationTime) { - TimerWheel->NextExpirationTime = ExpirationTime; - TimerWheel->NextConnection = Connection; - QuicTraceLogVerbose( - TimerWheelNextExpiration, - "[time][%p] Next Expiration = {%llu, %p}.", - TimerWheel, - ExpirationTime, - Connection); - } else if (Connection == TimerWheel->NextConnection) { - QuicTimerWheelUpdate(TimerWheel); - } - - // - // Resize the timer wheel if we have too many connections for the - // current size. - // - if (TimerWheel->ConnectionCount > - TimerWheel->SlotCount * QUIC_TIMER_WHEEL_MAX_LOAD_FACTOR) { - QuicTimerWheelResize(TimerWheel); - } + // + // Resize the timer wheel if we have too many connections for the + // current size. + // + if (TimerWheel->ConnectionCount > + TimerWheel->SlotCount * QUIC_TIMER_WHEEL_MAX_LOAD_FACTOR) { + QuicTimerWheelResize(TimerWheel); } } @@ -400,6 +407,7 @@ QuicTimerWheelGetExpired( // Iterate through every slot to find all the connections that now have // expired timers. // + BOOLEAN NeedsUpdate = FALSE; for (uint32_t i = 0; i < TimerWheel->SlotCount; ++i) { CXPLAT_LIST_ENTRY* ListHead = &TimerWheel->Slots[i]; CXPLAT_LIST_ENTRY* Entry = ListHead->Flink; @@ -413,7 +421,15 @@ QuicTimerWheelGetExpired( Entry = Entry->Flink; CxPlatListEntryRemove(&ConnectionEntry->TimerLink); CxPlatListInsertTail(OutputListHead, &ConnectionEntry->TimerLink); + if (ConnectionEntry == TimerWheel->NextConnection) { + NeedsUpdate = TRUE; + } + QuicConnAddRef(ConnectionEntry, QUIC_CONN_REF_WORKER); + QuicConnRelease(ConnectionEntry, QUIC_CONN_REF_TIMER_WHEEL); TimerWheel->ConnectionCount--; } } + if (NeedsUpdate) { + QuicTimerWheelUpdate(TimerWheel); + } } diff --git a/src/core/worker.c b/src/core/worker.c index 3372bcd57..68994a564 100644 --- a/src/core/worker.c +++ b/src/core/worker.c @@ -431,6 +431,7 @@ QuicWorkerProcessTimers( QuicConnTimerExpired(Connection, TimeNow); QuicConfigurationDetachSilo(); Connection->WorkerThreadID = 0; + QuicConnRelease(Connection, QUIC_CONN_REF_WORKER); } } @@ -474,12 +475,6 @@ QuicWorkerProcessConnection( Connection->Stats.Schedule.DrainCount++; if (Connection->State.UpdateWorker) { - // - // If the connection is uninitialized already, it shouldn't have been - // queued to move to a new worker in the first place. - // - CXPLAT_DBG_ASSERT(!Connection->State.Uninitialized); - // // The connection was recently placed into this worker and needs any // pre-existing timers to be transitioned to this worker for processing. @@ -543,11 +538,6 @@ QuicWorkerProcessConnection( if (DoneWithConnection) { if (Connection->State.UpdateWorker) { - // - // The connection should never be queued to a new worker if it's - // already been uninitialized. - // - CXPLAT_DBG_ASSERT(!Connection->State.Uninitialized); // // Now that we know we want to process this connection, assign it // to the correct registration. Remove it from the current worker's diff --git a/src/generated/linux/connection.c.clog.h b/src/generated/linux/connection.c.clog.h index 1a5cab8b5..9bfed0145 100644 --- a/src/generated/linux/connection.c.clog.h +++ b/src/generated/linux/connection.c.clog.h @@ -1015,6 +1015,24 @@ tracepoint(CLOG_CONNECTION_C, IndicateConnectionShutdownComplete , arg1);\ +/*---------------------------------------------------------- +// Decoder Ring for AbandonInternallyClosed +// [conn][%p] Abandoning internal, closed connection +// QuicTraceLogConnVerbose( + AbandonInternallyClosed, + Connection, + "Abandoning internal, closed connection"); +// arg1 = arg1 = Connection = arg1 +----------------------------------------------------------*/ +#ifndef _clog_3_ARGS_TRACE_AbandonInternallyClosed +#define _clog_3_ARGS_TRACE_AbandonInternallyClosed(uniqueId, arg1, encoded_arg_string)\ +tracepoint(CLOG_CONNECTION_C, AbandonInternallyClosed , arg1);\ + +#endif + + + + /*---------------------------------------------------------- // Decoder Ring for IndicateResumed // [conn][%p] Indicating QUIC_CONNECTION_EVENT_RESUMED @@ -1516,24 +1534,6 @@ tracepoint(CLOG_CONNECTION_C, TestTPSet , arg1, arg3, arg4);\ -/*---------------------------------------------------------- -// Decoder Ring for AbandonInternallyClosed -// [conn][%p] Abandoning internal, closed connection -// QuicTraceLogConnVerbose( - AbandonInternallyClosed, - Connection, - "Abandoning internal, closed connection"); -// arg1 = arg1 = Connection = arg1 -----------------------------------------------------------*/ -#ifndef _clog_3_ARGS_TRACE_AbandonInternallyClosed -#define _clog_3_ARGS_TRACE_AbandonInternallyClosed(uniqueId, arg1, encoded_arg_string)\ -tracepoint(CLOG_CONNECTION_C, AbandonInternallyClosed , arg1);\ - -#endif - - - - /*---------------------------------------------------------- // Decoder Ring for AllocFailure // Allocation of '%s' failed. (%llu bytes) @@ -1678,26 +1678,6 @@ tracepoint(CLOG_CONNECTION_C, ConnInitializeComplete , arg2);\ -/*---------------------------------------------------------- -// Decoder Ring for ConnUnregistered -// [conn][%p] Unregistered from %p -// QuicTraceEvent( - ConnUnregistered, - "[conn][%p] Unregistered from %p", - Connection, - Connection->Registration); -// arg2 = arg2 = Connection = arg2 -// arg3 = arg3 = Connection->Registration = arg3 -----------------------------------------------------------*/ -#ifndef _clog_4_ARGS_TRACE_ConnUnregistered -#define _clog_4_ARGS_TRACE_ConnUnregistered(uniqueId, encoded_arg_string, arg2, arg3)\ -tracepoint(CLOG_CONNECTION_C, ConnUnregistered , arg2, arg3);\ - -#endif - - - - /*---------------------------------------------------------- // Decoder Ring for ConnDestroyed // [conn][%p] Destroyed @@ -1734,6 +1714,26 @@ tracepoint(CLOG_CONNECTION_C, ConnHandleClosed , arg2);\ +/*---------------------------------------------------------- +// Decoder Ring for ConnUnregistered +// [conn][%p] Unregistered from %p +// QuicTraceEvent( + ConnUnregistered, + "[conn][%p] Unregistered from %p", + Connection, + Connection->Registration); +// arg2 = arg2 = Connection = arg2 +// arg3 = arg3 = Connection->Registration = arg3 +----------------------------------------------------------*/ +#ifndef _clog_4_ARGS_TRACE_ConnUnregistered +#define _clog_4_ARGS_TRACE_ConnUnregistered(uniqueId, encoded_arg_string, arg2, arg3)\ +tracepoint(CLOG_CONNECTION_C, ConnUnregistered , arg2, arg3);\ + +#endif + + + + /*---------------------------------------------------------- // Decoder Ring for ConnRegistered // [conn][%p] Registered with %p diff --git a/src/generated/linux/connection.c.clog.h.lttng.h b/src/generated/linux/connection.c.clog.h.lttng.h index d78d68588..bdee10cac 100644 --- a/src/generated/linux/connection.c.clog.h.lttng.h +++ b/src/generated/linux/connection.c.clog.h.lttng.h @@ -1103,6 +1103,25 @@ TRACEPOINT_EVENT(CLOG_CONNECTION_C, IndicateConnectionShutdownComplete, +/*---------------------------------------------------------- +// Decoder Ring for AbandonInternallyClosed +// [conn][%p] Abandoning internal, closed connection +// QuicTraceLogConnVerbose( + AbandonInternallyClosed, + Connection, + "Abandoning internal, closed connection"); +// arg1 = arg1 = Connection = arg1 +----------------------------------------------------------*/ +TRACEPOINT_EVENT(CLOG_CONNECTION_C, AbandonInternallyClosed, + TP_ARGS( + const void *, arg1), + TP_FIELDS( + ctf_integer_hex(uint64_t, arg1, arg1) + ) +) + + + /*---------------------------------------------------------- // Decoder Ring for IndicateResumed // [conn][%p] Indicating QUIC_CONNECTION_EVENT_RESUMED @@ -1679,25 +1698,6 @@ TRACEPOINT_EVENT(CLOG_CONNECTION_C, TestTPSet, -/*---------------------------------------------------------- -// Decoder Ring for AbandonInternallyClosed -// [conn][%p] Abandoning internal, closed connection -// QuicTraceLogConnVerbose( - AbandonInternallyClosed, - Connection, - "Abandoning internal, closed connection"); -// arg1 = arg1 = Connection = arg1 -----------------------------------------------------------*/ -TRACEPOINT_EVENT(CLOG_CONNECTION_C, AbandonInternallyClosed, - TP_ARGS( - const void *, arg1), - TP_FIELDS( - ctf_integer_hex(uint64_t, arg1, arg1) - ) -) - - - /*---------------------------------------------------------- // Decoder Ring for AllocFailure // Allocation of '%s' failed. (%llu bytes) @@ -1875,29 +1875,6 @@ TRACEPOINT_EVENT(CLOG_CONNECTION_C, ConnInitializeComplete, -/*---------------------------------------------------------- -// Decoder Ring for ConnUnregistered -// [conn][%p] Unregistered from %p -// QuicTraceEvent( - ConnUnregistered, - "[conn][%p] Unregistered from %p", - Connection, - Connection->Registration); -// arg2 = arg2 = Connection = arg2 -// arg3 = arg3 = Connection->Registration = arg3 -----------------------------------------------------------*/ -TRACEPOINT_EVENT(CLOG_CONNECTION_C, ConnUnregistered, - TP_ARGS( - const void *, arg2, - const void *, arg3), - TP_FIELDS( - ctf_integer_hex(uint64_t, arg2, arg2) - ctf_integer_hex(uint64_t, arg3, arg3) - ) -) - - - /*---------------------------------------------------------- // Decoder Ring for ConnDestroyed // [conn][%p] Destroyed @@ -1936,6 +1913,29 @@ TRACEPOINT_EVENT(CLOG_CONNECTION_C, ConnHandleClosed, +/*---------------------------------------------------------- +// Decoder Ring for ConnUnregistered +// [conn][%p] Unregistered from %p +// QuicTraceEvent( + ConnUnregistered, + "[conn][%p] Unregistered from %p", + Connection, + Connection->Registration); +// arg2 = arg2 = Connection = arg2 +// arg3 = arg3 = Connection->Registration = arg3 +----------------------------------------------------------*/ +TRACEPOINT_EVENT(CLOG_CONNECTION_C, ConnUnregistered, + TP_ARGS( + const void *, arg2, + const void *, arg3), + TP_FIELDS( + ctf_integer_hex(uint64_t, arg2, arg2) + ctf_integer_hex(uint64_t, arg3, arg3) + ) +) + + + /*---------------------------------------------------------- // Decoder Ring for ConnRegistered // [conn][%p] Registered with %p diff --git a/src/generated/linux/timer_wheel.c.clog.h b/src/generated/linux/timer_wheel.c.clog.h index 299fdcac3..86f6c2127 100644 --- a/src/generated/linux/timer_wheel.c.clog.h +++ b/src/generated/linux/timer_wheel.c.clog.h @@ -113,10 +113,10 @@ tracepoint(CLOG_TIMER_WHEEL_C, TimerWheelRemoveConnection , arg2, arg3);\ // Decoder Ring for TimerWheelUpdateConnection // [time][%p] Updating Connection %p. // QuicTraceLogVerbose( - TimerWheelUpdateConnection, - "[time][%p] Updating Connection %p.", - TimerWheel, - Connection); + TimerWheelUpdateConnection, + "[time][%p] Updating Connection %p.", + TimerWheel, + Connection); // arg2 = arg2 = TimerWheel = arg2 // arg3 = arg3 = Connection = arg3 ----------------------------------------------------------*/ diff --git a/src/generated/linux/timer_wheel.c.clog.h.lttng.h b/src/generated/linux/timer_wheel.c.clog.h.lttng.h index b42f78345..d8ef1d39a 100644 --- a/src/generated/linux/timer_wheel.c.clog.h.lttng.h +++ b/src/generated/linux/timer_wheel.c.clog.h.lttng.h @@ -97,10 +97,10 @@ TRACEPOINT_EVENT(CLOG_TIMER_WHEEL_C, TimerWheelRemoveConnection, // Decoder Ring for TimerWheelUpdateConnection // [time][%p] Updating Connection %p. // QuicTraceLogVerbose( - TimerWheelUpdateConnection, - "[time][%p] Updating Connection %p.", - TimerWheel, - Connection); + TimerWheelUpdateConnection, + "[time][%p] Updating Connection %p.", + TimerWheel, + Connection); // arg2 = arg2 = TimerWheel = arg2 // arg3 = arg3 = Connection = arg3 ----------------------------------------------------------*/ diff --git a/src/plugins/dbg/analyze.cpp b/src/plugins/dbg/analyze.cpp index 3b05f1777..6ce803723 100644 --- a/src/plugins/dbg/analyze.cpp +++ b/src/plugins/dbg/analyze.cpp @@ -42,8 +42,8 @@ void EXT_CLASS::AnalyzeConnection(UINT64 Addr) Dml("The connection has been freed.\n"); } else if (state.HandleClosed) { Dml("The connection has been closed by the application and is in the process of being deleted.\n"); - } else if (state.HandleShutdown) { - Dml("The connection has completed the shutdown process and is ready to be closed by the application.\n"); + } else if (state.ShutdownComplete) { + Dml("The connection has completed the shutdown process.\n"); } else if (state.ClosedLocally || state.ClosedRemotely) { Dml("The connection is in the process of shutting down."); if (state.ClosedLocally) { diff --git a/src/plugins/dbg/dbg.vcxproj b/src/plugins/dbg/dbg.vcxproj index 4bff7e48b..437ba94a8 100644 --- a/src/plugins/dbg/dbg.vcxproj +++ b/src/plugins/dbg/dbg.vcxproj @@ -52,7 +52,7 @@ quic DynamicLibrary - v142 + v143 $(SolutionDir)..\..\artifacts\bin\windbg\$(Platform)_$(Configuration)\ $(SolutionDir)..\..\build\windbg\$(Platform)_$(Configuration)\obj\$(ProjectName)\ diff --git a/src/plugins/dbg/dump.cpp b/src/plugins/dbg/dump.cpp index e1c2952e5..0dd9703e6 100644 --- a/src/plugins/dbg/dump.cpp +++ b/src/plugins/dbg/dump.cpp @@ -86,10 +86,10 @@ EXT_COMMAND( Registration.GetAppName().Data); } - Dml(" Worker 0x%I64X\t[Proc %d] %s\n", + Dml(" Worker 0x%I64X\t[Partition %d] %s\n", Worker.Addr, Worker.Addr, - Worker.IdealProcessor(), + Worker.PartitionIndex(), Worker.StateStr()); auto Connections = Worker.GetConnections(); diff --git a/src/plugins/dbg/quictypes.h b/src/plugins/dbg/quictypes.h index a3f741da1..529a4215e 100644 --- a/src/plugins/dbg/quictypes.h +++ b/src/plugins/dbg/quictypes.h @@ -88,9 +88,8 @@ typedef union QUIC_CONNECTION_STATE { BOOLEAN ClosedLocally : 1; // Locally closed. BOOLEAN ClosedRemotely : 1; // Remotely closed. BOOLEAN AppClosed : 1; // Application (not transport) closed connection. - BOOLEAN HandleShutdown : 1; // Shutdown callback delivered for handle. + BOOLEAN ShutdownComplete : 1; // Shutdown callback delivered for handle. BOOLEAN HandleClosed : 1; // Handle closed by application layer. - BOOLEAN Uninitialized : 1; // Uninitialize started/completed. BOOLEAN Freed : 1; // Freed. Used for Debugging. // @@ -174,7 +173,7 @@ typedef union QUIC_CONNECTION_STATE { // // The application needs to be notified of a shutdown complete event. // - BOOLEAN SendShutdownCompleteNotif : 1; + BOOLEAN ProcessShutdownComplete : 1; // // Indicates whether this connection shares bindings with others. @@ -1197,7 +1196,7 @@ struct Connection : Struct { return "FREED"; } else if (state.HandleClosed) { return "CLOSED"; - } else if (state.HandleShutdown) { + } else if (state.ShutdownComplete) { return "SHUTDOWN"; } else if (state.ClosedLocally || state.ClosedRemotely) { return "SHUTTING DOWN"; @@ -1324,8 +1323,8 @@ struct Worker : Struct { } } - UINT8 IdealProcessor() { - return ReadType("IdealProcessor"); + UINT16 PartitionIndex() { + return ReadType("PartitionIndex"); } UINT32 ThreadID() { diff --git a/src/plugins/dbg/registration.cpp b/src/plugins/dbg/registration.cpp index 558181cb5..110581cae 100644 --- a/src/plugins/dbg/registration.cpp +++ b/src/plugins/dbg/registration.cpp @@ -50,9 +50,9 @@ EXT_COMMAND( auto Workers = Registration.GetWorkerPool(); UCHAR WorkerCount = Workers.WorkerCount(); for (UCHAR i = 0; i < WorkerCount; i++) { - Dml("\tProc %d\t%s\n", + Dml("\tPartition %d \t%s\n", Workers.GetWorker(i).Addr, - Workers.GetWorker(i).IdealProcessor(), + Workers.GetWorker(i).PartitionIndex(), Workers.GetWorker(i).StateStr()); } @@ -67,9 +67,10 @@ EXT_COMMAND( } auto Connection = Connection::FromRegistrationLink(LinkAddr); - Dml("\t0x%I64X\n", + Dml("\t0x%I64X\t%s\n", Connection.Addr, - Connection.Addr); + Connection.Addr, + Connection.StateStr()); } Dml("\n"); diff --git a/src/plugins/dbg/worker.cpp b/src/plugins/dbg/worker.cpp index c517317f7..231106309 100644 --- a/src/plugins/dbg/worker.cpp +++ b/src/plugins/dbg/worker.cpp @@ -23,11 +23,11 @@ EXT_COMMAND( Dml("\nWORKER (raw)\n" "\n" "\tState %s\n" - "\tIdeal Processor %u\n" + "\tPartition %u\n" "\tThread 0x%X (UM/KM)\n", Work.Addr, Work.StateStr(), - Work.IdealProcessor(), + Work.PartitionIndex(), Work.ThreadID(), Work.ThreadID(), Work.Thread()); diff --git a/src/test/MsQuicTests.h b/src/test/MsQuicTests.h index 70d36850c..3bada48f2 100644 --- a/src/test/MsQuicTests.h +++ b/src/test/MsQuicTests.h @@ -59,6 +59,7 @@ void QuicTestRegistrationShutdownBeforeConnOpen(); void QuicTestRegistrationShutdownAfterConnOpen(); void QuicTestRegistrationShutdownAfterConnOpenBeforeStart(); void QuicTestRegistrationShutdownAfterConnOpenAndStart(); +void QuicTestConnectionCloseBeforeStreamClose(); // // Rejection Tests @@ -1242,4 +1243,7 @@ typedef struct { QUIC_CTL_CODE(116, METHOD_BUFFERED, FILE_WRITE_DATA) // int - Family -#define QUIC_MAX_IOCTL_FUNC_CODE 116 +#define IOCTL_QUIC_RUN_CONN_CLOSE_BEFORE_STREAM_CLOSE \ + QUIC_CTL_CODE(117, METHOD_BUFFERED, FILE_WRITE_DATA) + +#define QUIC_MAX_IOCTL_FUNC_CODE 117 diff --git a/src/test/bin/quic_gtest.cpp b/src/test/bin/quic_gtest.cpp index 69868dc4b..19cf10e0b 100644 --- a/src/test/bin/quic_gtest.cpp +++ b/src/test/bin/quic_gtest.cpp @@ -356,6 +356,15 @@ TEST(OwnershipValidation, RegistrationShutdownAfterConnOpenAndStart) { } } +TEST(OwnershipValidation, ConnectionCloseBeforeStreamClose) { + TestLogger Logger("ConnectionCloseBeforeStreamClose"); + if (TestingKernelMode) { + ASSERT_TRUE(DriverClient.Run(IOCTL_QUIC_RUN_CONN_CLOSE_BEFORE_STREAM_CLOSE)); + } else { + QuicTestConnectionCloseBeforeStreamClose(); + } +} + TEST_P(WithBool, ValidateStream) { TestLoggerT Logger("QuicTestValidateStream", GetParam()); if (TestingKernelMode) { diff --git a/src/test/bin/winkernel/control.cpp b/src/test/bin/winkernel/control.cpp index 9643b392e..c43f4c781 100644 --- a/src/test/bin/winkernel/control.cpp +++ b/src/test/bin/winkernel/control.cpp @@ -172,7 +172,6 @@ Exit: return Status; } - _No_competing_thread_ INITCODE NTSTATUS @@ -629,6 +628,7 @@ size_t QUIC_IOCTL_BUFFER_SIZES[] = 0, 0, sizeof(INT32), + 0, }; CXPLAT_STATIC_ASSERT( @@ -1526,6 +1526,10 @@ QuicTestCtlEvtIoDeviceControl( QuicTestCtlRun(QuicDrillTestServerVNPacket(Params->Family)); break; + case IOCTL_QUIC_RUN_CONN_CLOSE_BEFORE_STREAM_CLOSE: + QuicTestCtlRun(QuicTestConnectionCloseBeforeStreamClose()); + break; + default: Status = STATUS_NOT_IMPLEMENTED; break; diff --git a/src/test/lib/ApiTest.cpp b/src/test/lib/ApiTest.cpp index cd6e8e6e6..4142520b5 100644 --- a/src/test/lib/ApiTest.cpp +++ b/src/test/lib/ApiTest.cpp @@ -1162,8 +1162,8 @@ DummyStreamCallback( } struct CloseFromCallbackContext { - uint16_t CloseCount; - volatile uint16_t CurrentCount; + short CloseCount; + volatile short CurrentCount; uint8_t RawBuffer[100]; QUIC_BUFFER BufferToSend { sizeof(RawBuffer), RawBuffer }; @@ -1192,12 +1192,7 @@ struct CloseFromCallbackContext { } } -#ifdef _WIN32 - auto count = (uint16_t)InterlockedIncrement16((volatile short*)&Ctx->CurrentCount); -#else - auto count = (uint16_t)__sync_add_and_fetch((volatile short*)&Ctx->CurrentCount, 1); -#endif - if (Ctx->CloseCount == count-1) { + if (Ctx->CloseCount == InterlockedIncrement16(&Ctx->CurrentCount) - 1) { Conn->Close(); } @@ -1218,7 +1213,7 @@ QuicTestConnectionCloseFromCallback() { for (uint16_t i = 0; i < 20; i++) { CxPlatWatchdog Watchdog(2000); - CloseFromCallbackContext Context {i, 0}; + CloseFromCallbackContext Context {(short)i, 0}; MsQuicRegistration Registration(true); TEST_QUIC_SUCCEEDED(Registration.GetInitStatus()); diff --git a/src/test/lib/OwnershipTest.cpp b/src/test/lib/OwnershipTest.cpp index d77b90796..a9c222750 100644 --- a/src/test/lib/OwnershipTest.cpp +++ b/src/test/lib/OwnershipTest.cpp @@ -230,3 +230,49 @@ void QuicTestRegistrationShutdownAfterConnOpenAndStart() TEST_TRUE(State.StateEvent.WaitTimeout(2000)); TEST_EQUAL(1, State.ShutdownCount); } + +void QuicTestConnectionCloseBeforeStreamClose() +{ + MsQuicRegistration Registration; + TEST_TRUE(Registration.IsValid()); + + MsQuicRegistration ServerRegistration{true}; + TEST_TRUE(ServerRegistration.IsValid()); + + MsQuicAlpn Alpn("MsQuicTest"); + MsQuicCredentialConfig ClientCredConfig; + MsQuicConfiguration ClientConfiguration(Registration, Alpn, ClientCredConfig); + TEST_QUIC_SUCCEEDED(ClientConfiguration.GetInitStatus()); + + MsQuicConfiguration ServerConfiguration(ServerRegistration, Alpn, ServerSelfSignedCredConfig); + TEST_QUIC_SUCCEEDED(ServerConfiguration.GetInitStatus()); + + MsQuicAutoAcceptListener Listener(ServerRegistration, ServerConfiguration, MsQuicConnection::NoOpCallback); + TEST_QUIC_SUCCEEDED(Listener.GetInitStatus()); + QUIC_ADDRESS_FAMILY QuicAddrFamily = QUIC_ADDRESS_FAMILY_INET; + QuicAddr ServerLocalAddr(QuicAddrFamily); + TEST_QUIC_SUCCEEDED(Listener.Start(Alpn, &ServerLocalAddr.SockAddr)); + TEST_QUIC_SUCCEEDED(Listener.GetLocalAddr(ServerLocalAddr)); + + { + MsQuicConnection Connection(Registration); + TEST_QUIC_SUCCEEDED(Connection.GetInitStatus()); + MsQuicStream Stream(Connection, QUIC_STREAM_OPEN_FLAG_NONE); + TEST_QUIC_SUCCEEDED(Stream.GetInitStatus()); + TEST_QUIC_SUCCEEDED(Stream.Start()); + Connection.Close(); + + QUIC_UINT62 StreamID = 0; + TEST_QUIC_SUCCEEDED(Stream.GetID(&StreamID)); + TEST_QUIC_SUCCEEDED(Stream.SetPriority(0xFFFF)); + } + + { + MsQuicConnection Connection(Registration); + TEST_QUIC_SUCCEEDED(Connection.GetInitStatus()); + MsQuicStream Stream(Connection, QUIC_STREAM_OPEN_FLAG_NONE); + TEST_QUIC_SUCCEEDED(Stream.GetInitStatus()); + TEST_QUIC_SUCCEEDED(Stream.Start()); + Connection.Close(); + } +} diff --git a/src/tools/spin/spinquic.cpp b/src/tools/spin/spinquic.cpp index 02cc2748c..60d893e66 100644 --- a/src/tools/spin/spinquic.cpp +++ b/src/tools/spin/spinquic.cpp @@ -173,17 +173,19 @@ class SpinQuicWatchdog { CXPLAT_THREAD WatchdogThread; CXPLAT_EVENT ShutdownEvent; uint32_t TimeoutMs; + CXPLAT_THREAD_ID OriginThread; static CXPLAT_THREAD_CALLBACK(WatchdogThreadCallback, Context) { auto This = (SpinQuicWatchdog*)Context; if (!CxPlatEventWaitWithTimeout(This->ShutdownEvent, This->TimeoutMs)) { - printf("Watchdog timeout fired!\n"); + printf("Watchdog timeout fired while waiting on thread 0x%x!\n", (int)This->OriginThread); CXPLAT_FRE_ASSERTMSG(FALSE, "Watchdog timeout fired!"); } CXPLAT_THREAD_RETURN(0); } public: - SpinQuicWatchdog(uint32_t WatchdogTimeoutMs) : TimeoutMs(WatchdogTimeoutMs) { + SpinQuicWatchdog(uint32_t WatchdogTimeoutMs) : + TimeoutMs(WatchdogTimeoutMs), OriginThread(CxPlatCurThreadID()) { CxPlatEventInitialize(&ShutdownEvent, TRUE, FALSE); CXPLAT_THREAD_CONFIG Config = { 0 }; Config.Name = "spin_watchdog";