Skip to content

RegistrationClose frees a QUIC_WORKER_POOL while IsExternal workers' ExecutionContexts are still linked into the external CXPLAT_WORKER_POOL, when MsQuicLib.CustomExecutions is set #6308

Description

@srsubra

Describe the bug

Environment
Windows, x64, external/custom execution provider (app-driven ExecutionPoll/CxPlatWorkerPool*), non-QUIC_EXECUTION_PROFILE_TYPE_MAX_THROUGHPUT registration (so IsExternal = TRUE), MsQuicLib.CustomExecutions enabled.

Summary
When an application uses MsQuic's external/custom execution model, MsQuicRegistrationClose can free memory (Worker->ExecutionContext, embedded in the QUIC_WORKER_POOL array) that is still linked into the application's own CXPLAT_WORKER_POOL execution-context list, because the teardown path for IsExternal workers under MsQuicLib.CustomExecutions skips the only mechanism that would unlink it. The app's next ExecutionPoll/CxPlatRunExecutionContexts call then reads freed memory (InterlockedFetchAndClearBoolean(&Context->Ready)), causing an access violation.

Root cause, traced call by call

  1. QuicWorkerInitialize (src/core/worker.c): when ExecProfile != QUIC_EXECUTION_PROFILE_TYPE_MAX_THROUGHPUT, sets Worker->IsExternal = TRUE and calls:

CxPlatWorkerPoolAddExecutionContext(MsQuicLib.WorkerPool, &Worker->ExecutionContext, Partition->Index);

This links Worker->ExecutionContext into the app-owned external worker pool (MsQuicLib.WorkerPool) — the same pool the app's own ExecutionPoll walks via CxPlatRunExecutionContexts.

  1. MsQuicRegistrationClose (src/core/registration.c):
    CxPlatRundownReleaseAndWait(&Registration->Rundown);
    QuicWorkerPoolUninitialize(Registration->WorkerPool); // <-- frees Worker->ExecutionContext memory
    CxPlatRundownUninitialize(&Registration->Rundown);
    ...
    CXPLAT_FREE(Registration, QUIC_POOL_REGISTRATION);

  2. QuicWorkerPoolUninitialize (src/core/worker.c):
    for (uint16_t i = 0; i < WorkerPool->WorkerCount; i++) {
    QuicWorkerUninitialize(&WorkerPool->Workers[i]);
    }
    CXPLAT_FREE(WorkerPool, QUIC_POOL_WORKER); // frees every Workers[i].ExecutionContext too

  3. QuicWorkerUninitialize (src/core/worker.c) — the actual gap:
    Worker->Enabled = FALSE;
    if (Worker->ExecutionContext.Context) {
    if (MsQuicLib.CustomExecutions) {
    QuicWorkerLoopCleanup(Worker); // <-- taken for us: synchronous, no unlink
    } else {
    QuicWorkerThreadWake(Worker);
    CxPlatEventWaitForever(Worker->Done);
    }
    }

The else branch is how msquic normally lets an IsExternal worker unlink itself: QuicWorkerThreadWake calls CxPlatWakeExecutionContext(&Worker->ExecutionContext), which causes the app's next poll to invoke QuicWorkerLoop again; QuicWorkerLoop sees !Worker->Enabled, calls QuicWorkerLoopCleanup, and returns FALSE — the documented external-execution contract for "unlink me from the pool now." CxPlatEventWaitForever(Worker->Done) blocks until that has actually happened.

But when MsQuicLib.CustomExecutions is set, the if branch is taken instead: QuicWorkerLoopCleanup is called directly, synchronously, from the closing thread, completely bypassing the wake/return-FALSE protocol. Nothing ever tells CxPlatRunExecutionContexts to unlink Worker->ExecutionContext from the list.

  1. QuicWorkerPoolUninitialize then frees the whole array (CXPLAT_FREE(WorkerPool, ...)) — including every still-linked ExecutionContext — while the app's own poll loop may (and, per msquic's own XDP teardown design, must) still be actively walking that same list.

  2. The next time the app's poll thread calls CxPlatRunExecutionContexts on that partition, the first thing it does is InterlockedFetchAndClearBoolean(&Context->Ready) on the now-freed struct → AV. This matches our crash exactly (msquic!InterlockedFetchAndClearBoolean+0x4, inlined in CxPlatRunExecutionContexts, reading a freed CXPLAT_EXECUTION_CONTEXT).

Why this is distinct from the known platform_worker.c TODO
This is a second, separate gap from the acknowledged // TODO - Handle synchronized cleanup for external event queues? in CxPlatWorkerPoolDestroyWorker. That TODO is about the worker-pool-level datapath/XDP partition cleanup; this one is in core/worker.c's QUIC_WORKER-level cleanup, triggered by RegistrationClose, and is unconditional whenever MsQuicLib.CustomExecutions is set — it doesn't require XDP at all, just custom execution + a non-MAX_THROUGHPUT registration.

Suggested fix direction
QuicWorkerUninitialize should drive the wake/return-FALSE protocol for Worker->IsExternal workers regardless of MsQuicLib.CustomExecutions — i.e. always take (something like) the else branch's wake-and-wait-for-Done path when IsExternal is set, so the external pool unlinks the execution context via the normal Callback return-FALSE contract before QuicWorkerPoolUninitialize frees the backing memory.

Questions for the maintainers
Is the if (MsQuicLib.CustomExecutions) { QuicWorkerLoopCleanup(Worker); } short-circuit in QuicWorkerUninitialize intentional — i.e. is the app expected to guarantee something (like "stop polling entirely before RegistrationClose") that would make this safe, and if so, where is that documented? (Our reading of CxPlatDpRawUninitialize's dependence on continued polling during MsQuicClose suggests apps generally can't stop polling early without risking a separate hang, so this seems like a real contradiction in requirements.)
Should QuicWorkerUninitialize unconditionally use the wake/Done-wait path whenever Worker->IsExternal is true, dropping the MsQuicLib.CustomExecutions branch entirely?
Is this considered the same class of bug as the platform_worker.c TODO, or tracked separately? Is there an existing issue for this exact core/worker.c path we should link to instead of filing a duplicate?
Would a PR adding the wake/wait sequencing here be welcome, or is there a reason (e.g. a deadlock risk we're not seeing) that this was deliberately skipped for custom executions?

Affected OS

  • Windows
  • Linux
  • macOS
  • Other (specify below)

Additional OS information

windows 11

MsQuic version

2.5.9

Steps taken to reproduce bug

Repeated MsQuicRegistrationOpen → (no connections) → MsQuicRegistrationClose in a tight loop, external/custom execution, while a dedicated app thread continuously drives ExecutionPoll on the same worker pool. Eventually a poll iteration races the CXPLAT_FREE in QuicWorkerPoolUninitialize and reads freed memory.

Expected behavior

QUIC endpoint restarts after proper cleanup

Actual outcome

hangs/dumps

Additional details

No response

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions