Skip to content

quic: a reactor shutting down frees every connection without telling the peer #195

Description

@MDA2AV

Every death path in the QUIC engine was funnelled through Teardown() earlier in this release, so the peer always hears a CONNECTION_CLOSE. One path cannot use it, and it is the one that ends every connection at once: reactor shutdown.

TeardownQuic evicts by removing first and notifying second (src/ioxide/Reactor/Transport/Quic/Reactor.Quic.cs:327):

foreach (QuicConnection conn in _quicSweepScratch)
{
    QuicRemoveConnection(conn);
    conn.OnEvicted(QuicEvictReason.ReactorShutdown);
}

QuicRemoveConnection frees and zeroes the transport-owned peer address on its way out (:229), and QuicConnection.Send is a no-op without one (src/ioxide/Connection/Quic/QuicConnection.cs:119):

if (PeerAddr == 0)
{
    return;
}

So by the time OnEvicted runs there is nothing to send to. QuicEngineConnection.OnEvicted calls Destroy() directly, and it could not do otherwise: routing itself through Teardown would build a CONNECTION_CLOSE and then drop it on the floor.

The idle-sweep case at :251 is the same shape and is correct as it stands. RFC 9000 section 10.1 discards an idle connection silently, and the peer is by definition not listening. Shutdown is different: it is an immediate close, and section 10.2 says an immediate close is signalled with a CONNECTION_CLOSE frame.

What a peer sees today is nothing at all. It keeps a connection open against a server that has gone, until something else reclaims it - and between two ioxide endpoints there is nothing else, because the shim leaves max_idle_timeout unset, so the negotiated idle timeout is disabled (RFC 9000 section 10.1: the effective value is the minimum of the two advertised non-zero values, and neither side advertises one). The client's own transport sweep is the only backstop, which is a local timer rather than a protocol event the application can distinguish from a network partition.

The seam already exists and is unused: OnEvicted takes a QuicEvictReason (QuicConnection.cs:54) and QuicEngineConnection ignores it, so the two cases are already told apart at the call site and simply not acted on differently.

The fix is a reordering in the protected core, which is why this is filed rather than changed. In TeardownQuic only, notify before removing:

foreach (QuicConnection conn in _quicSweepScratch)
{
    conn.OnEvicted(QuicEvictReason.ReactorShutdown);   // may still Send
    QuicRemoveConnection(conn);
}

QuicEngineConnection.OnEvicted then routes the shutdown case through Teardown(farewell) and keeps calling Destroy() directly for IdleTimeout. Double removal is already safe - QuicRemoveConnection gates its release on _quicConnSet.Remove(conn) returning true, so the second call from inside Teardown is a no-op - but the ordering swap is the part that needs a decision, since it means user code (OnEvicted is a public virtual) runs while the connection is still routable and still in _quicConnSet.

Worth deciding alongside it: whether the farewell should carry an application code (Close(0), H3_NO_ERROR for an h3 server) or a transport NO_ERROR, and whether a shutdown should wait for those datagrams to leave the ring before the reactor stops - today TeardownQuic runs during teardown and nothing flushes afterwards.

Found by the second-round TLS review; verified by reading the eviction order rather than observed on the wire, because no test asserts that any teardown path puts a CONNECTION_CLOSE on the wire at all.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingseverity:mediumPerf cliff / hardening gap

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions