Skip to content

Add async cleanup APIs for external execution mode #5127

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Empty file modified scripts/build.ps1
100644 → 100755
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file has no changes. Please remove it from this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed this file from the PR as it has no changes.

Empty file.
111 changes: 109 additions & 2 deletions src/core/library.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,32 @@ QUIC_LIBRARY MsQuicLib = { 0 };

QUIC_TRACE_RUNDOWN_CALLBACK QuicTraceRundown;

typedef struct QUIC_API_TABLE_EX {
QUIC_API_TABLE Table;
QUIC_CLOSE_COMPLETE_HANDLER CloseHandler;
void* CloseContext;
} QUIC_API_TABLE_EX;

// Forward declaration
_IRQL_requires_max_(PASSIVE_LEVEL)
QUIC_STATUS
QUIC_API
MsQuicRegistrationCloseAsync(
_In_ _Pre_defensive_ __drv_freesMem(Mem)
HQUIC Handle,
_In_opt_ QUIC_REGISTRATION_CLOSE_COMPLETE_HANDLER Handler,
_In_opt_ void* Context
);

_IRQL_requires_max_(PASSIVE_LEVEL)
QUIC_STATUS
QUIC_API
MsQuicCloseAsync(
_In_ _Pre_defensive_ const void* QuicApi,
_In_opt_ QUIC_CLOSE_COMPLETE_HANDLER Handler,
_In_opt_ void* Context
);

_IRQL_requires_max_(PASSIVE_LEVEL)
void
QuicLibApplyLoadBalancingSetting(
Expand Down Expand Up @@ -1840,11 +1866,15 @@ MsQuicOpenVersion(
}
ReleaseRefOnFailure = TRUE;

QUIC_API_TABLE* Api = CXPLAT_ALLOC_NONPAGED(sizeof(QUIC_API_TABLE), QUIC_POOL_API);
if (Api == NULL) {
QUIC_API_TABLE_EX* ApiEx = CXPLAT_ALLOC_NONPAGED(sizeof(QUIC_API_TABLE_EX), QUIC_POOL_API);
if (ApiEx == NULL) {
Status = QUIC_STATUS_OUT_OF_MEMORY;
goto Exit;
}

QUIC_API_TABLE* Api = &ApiEx->Table;
ApiEx->CloseHandler = NULL;
ApiEx->CloseContext = NULL;

Api->SetContext = MsQuicSetContext;
Api->GetContext = MsQuicGetContext;
Expand All @@ -1856,6 +1886,7 @@ MsQuicOpenVersion(
Api->RegistrationOpen = MsQuicRegistrationOpen;
Api->RegistrationClose = MsQuicRegistrationClose;
Api->RegistrationShutdown = MsQuicRegistrationShutdown;
Api->RegistrationCloseAsync = MsQuicRegistrationCloseAsync;

Api->ConfigurationOpen = MsQuicConfigurationOpen;
Api->ConfigurationClose = MsQuicConfigurationClose;
Expand Down Expand Up @@ -1894,6 +1925,9 @@ MsQuicOpenVersion(
#endif

Api->ConnectionPoolCreate = MsQuicConnectionPoolCreate;

Api->RegistrationCloseAsync = MsQuicRegistrationCloseAsync;
Api->CloseAsync = MsQuicCloseAsync;

*QuicApi = Api;

Expand Down Expand Up @@ -1932,6 +1966,79 @@ MsQuicClose(
}
}

_IRQL_requires_max_(PASSIVE_LEVEL)
void
QuicLibraryCleanupComplete(
_In_ void* Context
)
{
QUIC_API_TABLE_EX* ApiEx = (QUIC_API_TABLE_EX*)Context;

QUIC_CLOSE_COMPLETE_HANDLER Handler = ApiEx->CloseHandler;
void* HandlerContext = ApiEx->CloseContext;

CXPLAT_FREE(ApiEx, QUIC_POOL_API);

if (Handler != NULL) {
Handler(HandlerContext);
}
}

_IRQL_requires_max_(PASSIVE_LEVEL)
QUIC_STATUS
QUIC_API
MsQuicCloseAsync(
_In_ _Pre_defensive_ const void* QuicApi,
_In_opt_ QUIC_CLOSE_COMPLETE_HANDLER Handler,
_In_opt_ void* Context
)
{
if (QuicApi != NULL) {
QuicTraceLogVerbose(
LibraryMsQuicClose,
"[ api] MsQuicCloseAsync");

QUIC_API_TABLE_EX* ApiEx = (QUIC_API_TABLE_EX*)QuicApi;
ApiEx->CloseHandler = Handler;
ApiEx->CloseContext = Context;

CxPlatLockAcquire(&MsQuicLib.Lock);
BOOLEAN LastRef = (MsQuicLib.OpenRefCount == 1);
CxPlatLockRelease(&MsQuicLib.Lock);

if (!LastRef) {
// Not the last reference, can clean up immediately
CXPLAT_FREE(ApiEx, QUIC_POOL_API);
MsQuicRelease();
MsQuicLibraryUnload();

if (Handler != NULL) {
Handler(Context);
}

return QUIC_STATUS_SUCCESS;
} else {
// This is the last reference so cleanup operations might block
QuicTraceLogWarning(
LibraryCleanupAsyncNotSupported,
"[ lib] Async cleanup not fully implemented! Potential deadlock can occur.");

// TODO: Properly implement async cleanup with platform event queue
CXPLAT_FREE(ApiEx, QUIC_POOL_API);
MsQuicRelease();
MsQuicLibraryUnload();

if (Handler != NULL) {
Handler(Context);
}

return QUIC_STATUS_SUCCESS;
}
}

return QUIC_STATUS_SUCCESS;
}

_IRQL_requires_max_(DISPATCH_LEVEL)
QUIC_BINDING*
QuicLibraryLookupBinding(
Expand Down
128 changes: 116 additions & 12 deletions src/core/registration.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@
#include "registration.c.clog.h"
#endif

typedef struct QUIC_REGISTRATION_EX {
QUIC_REGISTRATION Registration;
QUIC_REGISTRATION_CLOSE_COMPLETE_HANDLER CloseHandler;
void* CloseContext;
} QUIC_REGISTRATION_EX;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of making a new EX struct. Just put this stuff inside the existing registration struct.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed the QUIC_REGISTRATION_EX struct and instead added the CloseHandler and CloseContext fields directly to the existing QUIC_REGISTRATION struct. This eliminates the need for a separate wrapper struct and makes the code cleaner. 49d380c


_IRQL_requires_max_(PASSIVE_LEVEL)
QUIC_STATUS
QUIC_API
Expand Down Expand Up @@ -61,16 +67,20 @@ MsQuicRegistrationOpen(
goto Error;
}

Registration = CXPLAT_ALLOC_NONPAGED(RegistrationSize, QUIC_POOL_REGISTRATION);
if (Registration == NULL) {
QUIC_REGISTRATION_EX* RegistrationEx = CXPLAT_ALLOC_NONPAGED(RegistrationSize + sizeof(QUIC_REGISTRATION_EX) - sizeof(QUIC_REGISTRATION), QUIC_POOL_REGISTRATION);
if (RegistrationEx == NULL) {
QuicTraceEvent(
AllocFailure,
"Allocation of '%s' failed. (%llu bytes)",
"registration",
sizeof(QUIC_REGISTRATION) + AppNameLength + 1);
sizeof(QUIC_REGISTRATION_EX) + AppNameLength + 1);
Status = QUIC_STATUS_OUT_OF_MEMORY;
goto Error;
}

Registration = &RegistrationEx->Registration;
RegistrationEx->CloseHandler = NULL;
RegistrationEx->CloseContext = NULL;

CxPlatZeroMemory(Registration, RegistrationSize);
Registration->Type = QUIC_HANDLE_TYPE_REGISTRATION;
Expand Down Expand Up @@ -143,6 +153,54 @@ MsQuicRegistrationOpen(
return Status;
}

_IRQL_requires_max_(PASSIVE_LEVEL)
void
QuicRegistrationRundownComplete(
_In_ void* Context
)
{
QUIC_REGISTRATION_EX* RegistrationEx = CXPLAT_CONTAINING_RECORD(Context, QUIC_REGISTRATION_EX, Registration);
QUIC_REGISTRATION* Registration = &RegistrationEx->Registration;

QuicWorkerPoolUninitialize(Registration->WorkerPool);
CxPlatRundownUninitialize(&Registration->Rundown);
CxPlatDispatchLockUninitialize(&Registration->ConnectionLock);
CxPlatLockUninitialize(&Registration->ConfigLock);

// Store callback info locally before freeing
QUIC_REGISTRATION_CLOSE_COMPLETE_HANDLER Handler = RegistrationEx->CloseHandler;
void* HandlerContext = RegistrationEx->CloseContext;

CXPLAT_FREE(RegistrationEx, QUIC_POOL_REGISTRATION);

// Call the callback as the very last thing in the function
if (Handler != NULL) {
Handler(HandlerContext);
}
}

// Forward declaration
_IRQL_requires_max_(PASSIVE_LEVEL)
QUIC_STATUS
QUIC_API
MsQuicRegistrationCloseAsync(
_In_ _Pre_defensive_ __drv_freesMem(Mem)
HQUIC Handle,
_In_opt_ QUIC_REGISTRATION_CLOSE_COMPLETE_HANDLER Handler,
_In_opt_ void* Context
);

// Helper function for MsQuicRegistrationClose
void
QUIC_API
QuicRegistrationCloseComplete(
void* Context
)
{
CXPLAT_EVENT* Event = (CXPLAT_EVENT*)Context;
CxPlatEventSet(*Event);
}

_IRQL_requires_max_(PASSIVE_LEVEL)
void
QUIC_API
Expand All @@ -158,12 +216,57 @@ MsQuicRegistrationClose(
QUIC_TRACE_API_REGISTRATION_CLOSE,
Handle);

// Create an event to wait on
CXPLAT_EVENT CompletionEvent;
CxPlatEventInitialize(&CompletionEvent, TRUE, FALSE);

// Call the async version with our event as the context
MsQuicRegistrationCloseAsync(
Handle,
QuicRegistrationCloseComplete,
&CompletionEvent);

// Wait for the event to be set by the completion callback
CxPlatEventWaitForever(CompletionEvent);

// Clean up the event
CxPlatEventUninitialize(CompletionEvent);

QuicTraceEvent(
ApiExit,
"[ api] Exit");
}
}

_IRQL_requires_max_(PASSIVE_LEVEL)
QUIC_STATUS
QUIC_API
MsQuicRegistrationCloseAsync(
_In_ _Pre_defensive_ __drv_freesMem(Mem)
HQUIC Handle,
_In_opt_ QUIC_REGISTRATION_CLOSE_COMPLETE_HANDLER Handler,
_In_opt_ void* Context
)
{
QUIC_STATUS Status = QUIC_STATUS_INVALID_PARAMETER;

if (Handle != NULL && Handle->Type == QUIC_HANDLE_TYPE_REGISTRATION) {
QuicTraceEvent(
ApiEnter,
"[ api] Enter %u (%p).",
QUIC_TRACE_API_REGISTRATION_CLOSE_ASYNC,
Handle);

#pragma prefast(suppress: __WARNING_25024, "Pointer cast already validated.")
QUIC_REGISTRATION* Registration = (QUIC_REGISTRATION*)Handle;
QUIC_REGISTRATION_EX* RegistrationEx = CXPLAT_CONTAINING_RECORD(Registration, QUIC_REGISTRATION_EX, Registration);

RegistrationEx->CloseHandler = Handler;
RegistrationEx->CloseContext = Context;

QuicTraceEvent(
RegistrationCleanup,
"[ reg][%p] Cleaning up",
"[ reg][%p] Cleaning up (async)",
Registration);

if (Registration->ExecProfile != QUIC_EXECUTION_PROFILE_TYPE_INTERNAL) {
Expand All @@ -172,19 +275,20 @@ MsQuicRegistrationClose(
CxPlatLockRelease(&MsQuicLib.Lock);
}

CxPlatRundownReleaseAndWait(&Registration->Rundown);

QuicWorkerPoolUninitialize(Registration->WorkerPool);
CxPlatRundownUninitialize(&Registration->Rundown);
CxPlatDispatchLockUninitialize(&Registration->ConnectionLock);
CxPlatLockUninitialize(&Registration->ConfigLock);

CXPLAT_FREE(Registration, QUIC_POOL_REGISTRATION);
Status = QUIC_STATUS_SUCCESS;

// Release the rundown - this starts the cleanup process
CxPlatRundownRelease(&Registration->Rundown);

// Return pending as we don't know when it will complete
Status = QUIC_STATUS_PENDING;

QuicTraceEvent(
ApiExit,
"[ api] Exit");
}

return Status;
}

_IRQL_requires_max_(DISPATCH_LEVEL)
Expand Down
14 changes: 14 additions & 0 deletions src/generated/linux/library.c.clog.h
Original file line number Diff line number Diff line change
Expand Up @@ -626,10 +626,24 @@ tracepoint(CLOG_LIBRARY_C, ApiExit );\



/*----------------------------------------------------------
// Decoder Ring for LibraryCleanupAsyncNotSupported
// [ lib] Async cleanup not fully implemented! Potential deadlock can occur.
// QuicTraceLogWarning(
LibraryCleanupAsyncNotSupported,
"[ lib] Async cleanup not fully implemented! Potential deadlock can occur.");
----------------------------------------------------------*/
#ifndef _clog_2_ARGS_TRACE_LibraryCleanupAsyncNotSupported
#define _clog_2_ARGS_TRACE_LibraryCleanupAsyncNotSupported(uniqueId, encoded_arg_string)\
tracepoint(CLOG_LIBRARY_C, LibraryCleanupAsyncNotSupported );\

#endif


#ifdef __cplusplus
}
#endif
#endif
#ifdef CLOG_INLINE_IMPLEMENTATION
#include "quic.clog_library.c.clog.h.c"
#endif
15 changes: 15 additions & 0 deletions src/generated/linux/library.c.clog.h.lttng.h
Original file line number Diff line number Diff line change
Expand Up @@ -619,3 +619,18 @@ TRACEPOINT_EVENT(CLOG_LIBRARY_C, ApiExit,
TP_FIELDS(
)
)


/*----------------------------------------------------------
// Decoder Ring for LibraryCleanupAsyncNotSupported
// [ lib] Async cleanup not fully implemented! Potential deadlock can occur.
// QuicTraceLogWarning(
LibraryCleanupAsyncNotSupported,
"[ lib] Async cleanup not fully implemented! Potential deadlock can occur.");
----------------------------------------------------------*/
TRACEPOINT_EVENT(CLOG_LIBRARY_C, LibraryCleanupAsyncNotSupported,
TP_ARGS(
),
TP_FIELDS(
)
)
Loading