JSSynchronizationContext.Post has a post-then-release race with TSFN environment cleanup #497

Summary

JSSynchronizationContext.Post(...) checks IsDisposed and then calls into the thread-safe function (TSFN) to schedule the callback on the JS thread. The environment cleanup hook can concurrently run Dispose() — setting IsDisposed and releasing the TSFN — between the IsDisposed check and the TSFN call. This is a potential native use-after-release that the managed layer cannot fully prevent with a try/catch.

This is a follow-up to #492. The finalizer path in #492 posts its deferred DeleteReference through this same Post(...) pattern, so it is subject to this race — but the race is pre-existing: the original Dispose(disposing: true) context path used the identical SynchronizationContext.Post(...) pattern. #492 does not introduce it.

Details

JSTsfnSynchronizationContext (in src/NodeApi/Interop/JSSynchronizationContext.cs):

public override void Post(SendOrPostCallback callback, object? state)
{
    if (IsDisposed) return;              // (1) check
    _tsfn.NonBlockingCall(() => callback(state));  // (2) use
}

and cleanup:

public override void Dispose()
{
    if (IsDisposed) return;
    // remove env cleanup hook ...
    base.Dispose();                      // sets IsDisposed = true
    _tsfn.Release();                     // TSFN deleted; must not be used after this
}

Cleanup (the env cleanup hook) calls Dispose(). A poster thread can pass the IsDisposed check at (1), then the cleanup hook runs Dispose() (setting IsDisposed and releasing the TSFN), and then the poster calls NonBlockingCall at (2) on a released TSFN — a native use-after-release. The managed try/catch around the finalizer delete cannot reliably guard a native crash.

Impact

• Narrow timing window during environment teardown, when the sync context is being disposed concurrently with a Post.
• Pre-existing; affects all Post callers, not just the finalizer added in Make JSReference finalizer thread-safe to fix worker-teardown crash (exit 139) #492.

Proposed direction

Add atomic post/close coordination inside JSSynchronizationContext / JSTsfnSynchronizationContext so that posting and TSFN release cannot interleave — for example, reference-counting active posts, or taking a lock / using an interlocked state so Release() waits for (or safely rejects) in-flight NonBlockingCalls. This affects the shared synchronization-context implementation and should be designed and tested independently of the crash fix.

Status: fixed

Implemented on branch fix/jsreference-nocontext-leak (commit "Close TSFN post-then-release race in JSSynchronizationContext").

Added TsfnCallGate (src/NodeApi/Interop/TsfnCallGate.cs), an interlocked gate that reference-counts in-flight NonBlockingCall invocations:

• Post/Send wrap each native NonBlockingCall in TryEnter()/Exit().
• Dispose calls _callGate.Close() before _tsfn.Release(). Close() atomically sets a closed flag so no further TryEnter() can succeed, then spins until all in-flight calls have exited.

This guarantees that once Close() returns, no native TSFN call is in progress and none can start, so Release() cannot race a NonBlockingCall. The gate is held only around the native call — not around Send's completion wait — so TSFN release cannot deadlock against a callback that must run on the JS thread. InternalsVisibleTo was added so the gate's concurrency behavior is unit-tested directly (including a deterministic "Close waits for in-flight call" test and a concurrent-stress test).

References

• PR Make JSReference finalizer thread-safe to fix worker-teardown crash (exit 139) #492 — deferred cleanup posts through this path.
• src/NodeApi/Interop/JSSynchronizationContext.cs — JSTsfnSynchronizationContext.Post and Dispose.