fix(quic): stop ~ClientQUIC racing the callback to close the connection

MsQuicConnectionClose was reachable twice for one handle: ~ClientQUIC
called it unconditionally, and the SHUTDOWN_COMPLETE callback also called
it whenever AppCloseInProgress was clear -- which it is for the whole
window between the destructor's ConnectionShutdown and its ConnectionClose.
The second call trips CXPLAT_TEL_ASSERT(!Connection->State.HandleClosed)
and aborts the process. Clearing impl->connection from the callback did not
help: the destructor had already loaded it.

`connection` is now a claim token, taken under the mutex, and only whoever
wins the claim closes. The callback keeps closing peer-dropped connections
so they do not leak while their ClientQUIC lives on.

Separately, the destructor now waits (bounded, 5s) for the connection's
streams to reach StreamClose before closing it. QUICStream::Stop only
initiates a shutdown whose async completion does the close, so a
connection could otherwise be closed with streams msquic still considers
open. The count lives in a shared StreamRegistry rather than behind the
ClientQUIC*, so a stream finalising after the wait gave up decrements a
live object.

A failed handshake also closes its connection and configuration now.
Throwing from the constructor body means ~ClientQUIC never runs, so both
handles leaked -- and the connection callback holds the impl that was
about to be destroyed with the half-built object.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
catbot 2026-08-25 17:06:51 +00:00
commit 42a34de244
2 changed files with 138 additions and 16 deletions

View file

@ -149,9 +149,11 @@ namespace Crafter {
// via OnStream()).
// - Unreliable, unordered datagrams (SendDatagram() / OnDatagram()).
//
// Lifetime: ~ClientQUIC closes the connection. Streams obtained from
// OpenStream() are scoped to the connection and must be destroyed (or
// moved out) before the ClientQUIC.
// Lifetime: ~ClientQUIC shuts the connection down and closes it, waiting
// (bounded — 5s) for every stream on it to finish closing first, since
// QUICStream::Stop only initiates a shutdown that completes asynchronously.
// Streams outliving the connection are therefore safe to hold, though they
// are dead once it goes; prefer destroying or moving them out first.
//
// Browser build: the only QUIC-shaped API the browser exposes is
// WebTransport, which is HTTP/3-based and reached at a fixed URL. Here: