Commit graph Crafter.Network/tests/ShouldSurviveConnectionChurn/main.cpp
Author SHA1 Message Date
catbot
ffc118f825 fix(quic): close stream handles whose async StreamStart lost the teardown race
A client that sends on a stream while its connection is being torn down —
the shape any long-lived client with periodic acks or keepalives has —
would hang at exit inside MsQuicRegistrationClose, and occasionally
bugcheck on an msquic worker thread instead.

Root cause: StreamStart is asynchronous. It returns QUIC_STATUS_PENDING
and queues the start onto the connection, so it can still fail later, and
it fails with QUIC_STATUS_INVALID_STATE whenever the connection is shut
down before the queued start runs. Passed QUIC_STREAM_START_FLAG_NONE,
msquic leaves such a stream neither started nor shut down, so
SHUTDOWN_COMPLETE never arrives. The stream close is driven entirely off
that event, so the handle stays open: ~ClientQUIC's bounded drain expires,
the connection is closed with a stream msquic still considers open, and
the registration's rundown at exit() waits forever for the handle.

Instrumenting the handle lifecycle over the reproducer shows one leaked
locally-opened stream per failing run, correlating 1:1 with a
START_COMPLETE carrying 0x1 (INVALID_STATE).

Fixes:

  - OpenStream passes QUIC_STREAM_START_FLAG_SHUTDOWN_ON_FAIL, so a start
    that loses the race is followed by SHUTDOWN_COMPLETE like any other
    stream and its handle is closed.

  - QUICStream::Stop also aborts the receive direction. GRACEFUL closes
    only the send side and msquic rejects it combined with any other flag,
    so a bidirectional stream whose peer keeps its send side open never
    reached SHUTDOWN_COMPLETE either.

  - The msquic stream handle is now refcounted, held by the callback plus
    every app thread inside an msquic call on it, and closed by whoever
    lets go last. StreamSend/StreamShutdown/GetParam could previously run
    against a handle a worker thread had just closed — a use-after-free
    msquic reports as a bugcheck. A refcount rather than an exclusion lock
    because GetParam blocks on the connection's worker, which must never
    be the thread waiting.

  - ~ClientQUIC drops undispatched peer streams before closing the
    connection rather than with the rest of Impl afterwards, so the drain
    covers them.

The reproducer (tests/ShouldSurviveConnectionChurn) grows a client-opened
bidirectional control stream with a thread sending across the teardown,
and no longer ends in std::_Exit — returning from main runs the static
MsQuicRuntime destructor, and its RegistrationClose is where a leaked
handle shows up. Before: 5 of 15 runs clean, 10 hung. After: 90 of 90
clean, with every opened handle observed closed and the drain never
expiring.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-25 23:18:01 +00:00
catbot
25e8c84794 test(quic): cover connection teardown
Every other QUIC test ends in std::_Exit(0), so nothing in the suite ever
ran a ~ClientQUIC. This adds 25 build-up/tear-down cycles with reader
threads parked inside RecieveSync -- the shape a reconnect loop has --
which aborts on ~10/20 runs against the current implementation.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-25 17:06:25 +00:00