Add detailed Registration reference tracking (#5573)

diff --git a/src/core/configuration.c b/src/core/configuration.c
index 087bc2958431d6fb47db9f6de20aa556f401f2aa..96653ecf104235ed98c194f28e198d9fef29b73a 100644 (file)
--- a/src/core/configuration.c
+++ b/src/core/configuration.c
@@ -196,7 +196,7 @@ MsQuicConfigurationOpen(
 
     QuicConfigurationSettingsChanged(Configuration);
 
-    BOOLEAN Result = CxPlatRundownAcquire(&Registration->Rundown);
+    BOOLEAN Result = QuicRegistrationRundownAcquire(Registration, QUIC_REG_REF_CONFIGURATION);
     CXPLAT_FRE_ASSERT(Result);
 
     CxPlatLockAcquire(&Registration->ConfigLock);
@@ -257,7 +257,7 @@ QuicConfigurationUninitialize(
 
     QuicSettingsCleanup(&Configuration->Settings);
 
-    CxPlatRundownRelease(&Configuration->Registration->Rundown);
+    QuicRegistrationRundownRelease(Configuration->Registration, QUIC_REG_REF_CONFIGURATION);
 
     QuicTraceEvent(
         ConfigurationDestroyed,

diff --git a/src/core/connection.c b/src/core/connection.c
index 79a5d52b3ee2386e0d7410a341df7e64ef30899a..1bae7707242deaf61e1527e1b0bf4daeb5a872c0 100644 (file)
--- a/src/core/connection.c
+++ b/src/core/connection.c
@@ -406,7 +406,7 @@ QuicConnFree(
         QuicPerfCounterDecrement(Partition, QUIC_PERF_COUNTER_CONN_CONNECTED);
     }
     if (Connection->Registration != NULL) {
-        CxPlatRundownRelease(&Connection->Registration->Rundown);
+        QuicRegistrationRundownRelease(Connection->Registration, QUIC_REG_REF_CONNECTION);
     }
     if (Connection->CloseReasonPhrase != NULL) {
         CXPLAT_FREE(Connection->CloseReasonPhrase, QUIC_POOL_CLOSE_REASON);
@@ -494,7 +494,7 @@ QuicConnUnregister(
         CxPlatDispatchLockAcquire(&Connection->Registration->ConnectionLock);
         CxPlatListEntryRemove(&Connection->RegistrationLink);
         CxPlatDispatchLockRelease(&Connection->Registration->ConnectionLock);
-        CxPlatRundownRelease(&Connection->Registration->Rundown);
+        QuicRegistrationRundownRelease(Connection->Registration, QUIC_REG_REF_CONNECTION);
 
         QuicTraceEvent(
             ConnUnregistered,
@@ -516,7 +516,7 @@ QuicConnRegister(
 {
     QuicConnUnregister(Connection);
 
-    if (!CxPlatRundownAcquire(&Registration->Rundown)) {
+    if (!QuicRegistrationRundownAcquire(Registration, QUIC_REG_REF_CONNECTION)) {
         return FALSE;
     }
     Connection->State.Registered = TRUE;
@@ -539,7 +539,7 @@ QuicConnRegister(
     if (RegistrationShuttingDown) {
         Connection->State.Registered = FALSE;
         Connection->Registration = NULL;
-        CxPlatRundownRelease(&Registration->Rundown);
+        QuicRegistrationRundownRelease(Registration, QUIC_REG_REF_CONNECTION);
     } else {
         QuicTraceEvent(
             ConnRegistered,

diff --git a/src/core/connection.h b/src/core/connection.h
index a8a391f9a600acc9ad8ccbe0b20c703d4342b29f..56b41b07c01fb76326acc661b5add680e83cddad 100644 (file)
--- a/src/core/connection.h
+++ b/src/core/connection.h
@@ -1108,7 +1108,7 @@ QuicConnRelease(
 #if DEBUG
     CXPLAT_TEL_ASSERT(Connection->RefTypeCount[Ref] > 0);
     uint16_t result = (uint16_t)InterlockedDecrement16((volatile short*)&Connection->RefTypeCount[Ref]);
-    CXPLAT_TEL_ASSERT(result != 0xFFFF);
+    CXPLAT_TEL_ASSERT(result != UINT16_MAX);
 #else
     UNREFERENCED_PARAMETER(Ref);
 #endif

diff --git a/src/core/listener.c b/src/core/listener.c
index 1f75b62b4d5a16ef50227efe42b30197d5103f2f..f8e27ff1da039c695b626f8c6d90b78445fed4fc 100644 (file)
--- a/src/core/listener.c
+++ b/src/core/listener.c
@@ -92,7 +92,7 @@ MsQuicListenerOpen(
 
     BOOLEAN RegistrationShuttingDown;
 
-    BOOLEAN Result = CxPlatRundownAcquire(&Registration->Rundown);
+    BOOLEAN Result = QuicRegistrationRundownAcquire(Registration, QUIC_REG_REF_LISTENER);
     CXPLAT_DBG_ASSERT(Result); UNREFERENCED_PARAMETER(Result);
 
     CxPlatDispatchLockAcquire(&Registration->ConnectionLock);
@@ -103,7 +103,7 @@ MsQuicListenerOpen(
     CxPlatDispatchLockRelease(&Registration->ConnectionLock);
 
     if (RegistrationShuttingDown) {
-        CxPlatRundownRelease(&Registration->Rundown);
+        QuicRegistrationRundownRelease(Registration, QUIC_REG_REF_LISTENER);
         CxPlatEventUninitialize(Listener->StopEvent);
         CXPLAT_FREE(Listener, QUIC_POOL_LISTENER);
         Listener = NULL;
@@ -160,7 +160,7 @@ QuicListenerFree(
     CxPlatEventUninitialize(Listener->StopEvent);
     CXPLAT_DBG_ASSERT(Listener->AlpnList == NULL);
     CXPLAT_FREE(Listener, QUIC_POOL_LISTENER);
-    CxPlatRundownRelease(&Registration->Rundown);
+    QuicRegistrationRundownRelease(Registration, QUIC_REG_REF_LISTENER);
 }
 
 _IRQL_requires_max_(PASSIVE_LEVEL)

diff --git a/src/core/registration.c b/src/core/registration.c
index 31983f2ed1d7df3a60fff051a82b2b67c29466d9..6f3da219ce6f635d5c04babd67ba4448bdc727ff 100644 (file)
--- a/src/core/registration.c
+++ b/src/core/registration.c
@@ -46,9 +46,22 @@ QuicRegistrationClose(
         CxPlatLockRelease(&MsQuicLib.Lock);
     }
 
+#if DEBUG
+    CXPLAT_DBG_ASSERT(!CxPlatRefDecrement(&Registration->RefTypeBiasedCount[QUIC_REG_REF_HANDLE_OWNER]));
+#endif
+
     CxPlatRundownReleaseAndWait(&Registration->Rundown);
 
     QuicWorkerPoolUninitialize(Registration->WorkerPool);
+#if DEBUG
+    for (uint32_t i = 0; i < QUIC_REG_REF_COUNT; i++) {
+        //
+        // Since this is after CxPlatRundownReleaseAndWait returns,
+        // no thread should be modifying these counters.
+        //
+        CXPLAT_DBG_ASSERT(QuicReadLongPtrNoFence(&Registration->RefTypeBiasedCount[i]) == 1);
+    }
+#endif
     CxPlatRundownUninitialize(&Registration->Rundown);
     CxPlatDispatchLockUninitialize(&Registration->ConnectionLock);
     CxPlatLockUninitialize(&Registration->ConfigLock);
@@ -171,6 +184,10 @@ MsQuicRegistrationOpen(
     CxPlatListInitializeHead(&Registration->Connections);
     CxPlatListInitializeHead(&Registration->Listeners);
     CxPlatRundownInitialize(&Registration->Rundown);
+#if DEBUG
+    CxPlatRefInitializeMultiple(Registration->RefTypeBiasedCount, QUIC_REG_REF_COUNT);
+    CxPlatRefIncrement(&Registration->RefTypeBiasedCount[QUIC_REG_REF_HANDLE_OWNER]);
+#endif
     CxPlatEventInitialize(&Registration->CloseEvent, TRUE, FALSE);
     Registration->AppNameLength = (uint8_t)(AppNameLength + 1);
     if (AppNameLength != 0) {
@@ -238,6 +255,9 @@ Error:
     if (Registration != NULL) {
         CXPLAT_DBG_ASSERT(!Registration->CloseThread);
         CxPlatEventUninitialize(Registration->CloseEvent);
+#if DEBUG
+        CxPlatRefDecrement(&Registration->RefTypeBiasedCount[QUIC_REG_REF_HANDLE_OWNER]);
+#endif
         CxPlatRundownUninitialize(&Registration->Rundown);
         CxPlatDispatchLockUninitialize(&Registration->ConnectionLock);
         CxPlatLockUninitialize(&Registration->ConfigLock);

diff --git a/src/core/registration.h b/src/core/registration.h
index 7ec803817a80bddba2b9b920334f92c28931cf0a..49ba65fa2cf42c32dd332c3ee5821f81b8aeb437 100644 (file)
--- a/src/core/registration.h
+++ b/src/core/registration.h
@@ -21,6 +21,17 @@ typedef enum QUIC_CONNECTION_ACCEPT_RESULT {
     QUIC_CONNECTION_REJECT_APP
 } QUIC_CONNECTION_ACCEPT_RESULT;
 
+typedef enum QUIC_REGISTRATION_REF {
+
+    QUIC_REG_REF_HANDLE_OWNER,
+    QUIC_REG_REF_CONFIGURATION,
+    QUIC_REG_REF_CONNECTION,
+    QUIC_REG_REF_LISTENER,
+
+    QUIC_REG_REF_COUNT
+
+} QUIC_REGISTRATION_REF;
+
 //
 // Represents per application registration state.
 //
@@ -100,6 +111,15 @@ typedef struct QUIC_REGISTRATION {
     //
     CXPLAT_RUNDOWN_REF Rundown;
 
+#if DEBUG
+    //
+    // Detailed ref counts. The actual reference count is in the Rundown.
+    // Note: These ref counts are biased by 1, so lowest they go is 1. It is an
+    // error for them to ever be zero.
+    //
+    CXPLAT_REF_COUNT RefTypeBiasedCount[QUIC_REG_REF_COUNT];
+#endif
+
     //
     // Shutdown error code if set.
     //
@@ -148,6 +168,52 @@ typedef struct QUIC_REGISTRATION {
 #define QUIC_REG_VERIFY(Registration, Expr)
 #endif
 
+//
+// Adds a rundown reference to the Registration.
+//
+_IRQL_requires_max_(DISPATCH_LEVEL)
+QUIC_INLINE
+BOOLEAN
+QuicRegistrationRundownAcquire(
+    _In_ QUIC_REGISTRATION* Registration,
+    _In_ QUIC_REGISTRATION_REF Ref
+    )
+{
+    BOOLEAN Result = CxPlatRundownAcquire(&Registration->Rundown);
+#if DEBUG
+    if (Result) {
+        //
+        // Only increment the detailed ref count if the Rundown acquire succeeded.
+        //
+        CxPlatRefIncrement(&Registration->RefTypeBiasedCount[Ref]);
+    }
+#else
+    UNREFERENCED_PARAMETER(Ref);
+#endif
+
+    return Result;
+}
+
+//
+// Releases a rundown reference on the Registration.
+//
+_IRQL_requires_max_(DISPATCH_LEVEL)
+QUIC_INLINE
+void
+QuicRegistrationRundownRelease(
+    _In_ QUIC_REGISTRATION* Registration,
+    _In_ QUIC_REGISTRATION_REF Ref
+    )
+{
+#if DEBUG
+    CXPLAT_DBG_ASSERT(!CxPlatRefDecrement(&Registration->RefTypeBiasedCount[Ref]));
+#else
+    UNREFERENCED_PARAMETER(Ref);
+#endif
+
+    CxPlatRundownRelease(&Registration->Rundown);
+}
+
 //
 // Tracing rundown for the registration.
 //

diff --git a/src/inc/quic_platform_posix.h b/src/inc/quic_platform_posix.h
index 193a4d15868d0810dca601aa5fe28c243d970513..e58ca124e8e5d14618f1af3bfb2e4628f6089c4d 100644 (file)
--- a/src/inc/quic_platform_posix.h
+++ b/src/inc/quic_platform_posix.h
@@ -291,7 +291,9 @@ InterlockedDecrement64(
     return __sync_sub_and_fetch(Addend, (int64_t)1);
 }
 
-#define QuicReadPtrNoFence(p) ((void*)(*p)) // TODO
+#define QuicReadPtrNoFence(p) __atomic_load_n((p), __ATOMIC_RELAXED)
+
+#define QuicReadLongPtrNoFence(p) __atomic_load_n((p), __ATOMIC_RELAXED)
 
 //
 // Assertion interfaces.
@@ -671,6 +673,12 @@ CxPlatRefInitializeEx(
     _In_ uint32_t Initial
     );
 
+void
+CxPlatRefInitializeMultiple(
+    _Out_writes_(Count) CXPLAT_REF_COUNT* RefCounts,
+    _In_ uint32_t Count
+    );
+
 void
 CxPlatRefIncrement(
     _Inout_ CXPLAT_REF_COUNT* RefCount

diff --git a/src/inc/quic_platform_winkernel.h b/src/inc/quic_platform_winkernel.h
index 87320f83ea456d13e210782829b2dbe274fe17a2..1fdcc49667503ed830891996d48b117a73397c43 100644 (file)
--- a/src/inc/quic_platform_winkernel.h
+++ b/src/inc/quic_platform_winkernel.h
@@ -401,6 +401,18 @@ CxPlatRefInitialize(
     *RefCount = 1;
 }
 
+QUIC_INLINE
+void
+CxPlatRefInitializeMultiple(
+    _Out_writes_(Count) CXPLAT_REF_COUNT* RefCounts,
+    _In_ uint32_t Count
+    )
+{
+    for (uint32_t i = 0; i < Count; i++) {
+        CxPlatRefInitialize(&RefCounts[i]);
+    }
+}
+
 #define CxPlatRefUninitialize(RefCount)
 
 QUIC_INLINE

diff --git a/src/inc/quic_platform_winuser.h b/src/inc/quic_platform_winuser.h
index b64f002099610f14086317ae622e77c7bea98857..d070ea83d37d5a5c93df70544c5ad9470f14922b 100644 (file)
--- a/src/inc/quic_platform_winuser.h
+++ b/src/inc/quic_platform_winuser.h
@@ -604,6 +604,18 @@ CxPlatRefInitializeEx(
     *RefCount = (LONG_PTR)Initial;
 }
 
+QUIC_INLINE
+void
+CxPlatRefInitializeMultiple(
+    _Out_writes_(Count) CXPLAT_REF_COUNT* RefCounts,
+    _In_ uint32_t Count
+    )
+{
+    for (uint32_t i = 0; i < Count; i++) {
+        CxPlatRefInitialize(&RefCounts[i]);
+    }
+}
+
 #define CxPlatRefUninitialize(RefCount)
 
 QUIC_INLINE

diff --git a/src/platform/platform_posix.c b/src/platform/platform_posix.c
index f7091964ca0781c89fc9bf54ecb3c52ddad00884..7f7a53599375908fbc11dc76ce3b2b18ea2bcef0 100644 (file)
--- a/src/platform/platform_posix.c
+++ b/src/platform/platform_posix.c
@@ -312,6 +312,17 @@ CxPlatRefInitializeEx(
     *RefCount = (int64_t)Initial;
 }
 
+void
+CxPlatRefInitializeMultiple(
+    _Out_writes_(Count) CXPLAT_REF_COUNT* RefCounts,
+    _In_ uint32_t Count
+    )
+{
+    for (uint32_t i = 0; i < Count; i++) {
+        CxPlatRefInitialize(&RefCounts[i]);
+    }
+}
+
 void
 CxPlatRefIncrement(
     _Inout_ CXPLAT_REF_COUNT* RefCount