Repository navigation
quic: a reactor shutdown tells its peers with a CONNECTION_CLOSE instead of leaving them to time out - #380
Open
MDA2AV wants to merge 1 commit into
Open
quic: a reactor shutdown tells its peers with a CONNECTION_CLOSE instead of leaving them to time out#380MDA2AV wants to merge 1 commit into
MDA2AV wants to merge 1 commit into
Conversation
…ead of leaving them to time out
This was referenced Oct 17, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #195.
What was wrong. At a reactor shutdown,
TeardownQuicremoved each connection before evicting it.QuicRemoveConnectionfrees and zeroes the peer address, soSendwas already a no-op whenOnEvictedran.QuicEngineConnection.OnEvictedonly calledDestroy()anyway. A farewell written in time would still have been lost:TeardownQuicruns inReactor.Teardown, after the loop has exited, so an SQE queued there is never submitted, andCloseUdpFdscloses the socket two lines later. Every peer was left to its own idle timeout.What changed. Three parts, and each is needed (see Proof):
TeardownQuic:OnEvictednow runs beforeQuicRemoveConnection, so the peer address is still live for the farewell. The engine's ownTeardownremoves the connection first, which makes the second removal a no-op through the existing set-membership gate.QuicEngineConnection.OnEvicted:ReactorShutdowngoes through the engine's existingTeardown(WriteTransportFarewell(0)).ReadTimeout(the idle sweep) andTimerFaultstill callDestroy()and stay silent (RFC 9000 10.1).UdpSendTo: once_stopRequestedis set, a datagram goes out with a directsendmsg(MSG_DONTWAIT)instead of an SQE, because no loop pass may submit again. The slot is released through the existingOnUdpSendCompletion, which also logs a failure.Supporting core hunks:
Reactor.Teardownsets_stopRequested = truefirst, because a faulted loop exit never sets it. Without this, a faulted exit would send the farewells to the ring. On a reactor that died with its SQ stuck full,GetSqeOrFlushwould then throw out ofRun()'sfinally, skippingCloseUdpFds, the TCP closes and the ring dispose. Side effect: sibling reactors stop forwarding datagrams to a faulted reactor slightly earlier, sinceQuicTryForwardchecks the same flag.Native.Socket.cs:sendmsgandMSG_DONTWAIT, both internal, so there is no public API change.QuicConnection.OnEvicteddoc: it no longer says the connection is always unregistered when this runs.The core diff is 5 files, +19/-2.
Why a direct send rather than submit-and-wait. Entering the ring from
Teardownhas side effects:DEFER_TASKRUN, aGETEVENTSenter runs the deferred task work of armed multishots whose sockets became ready. An armed accept could take a new connection whose CQE teardown then drops, leaking its fd.CloseTcpListenershas already closed.A direct
sendmsgtouches only the UDP socket. It submits nothing and processes no CQEs, so the ring's state is untouched, and it works on a broken ring too.The farewell code. Transport
NO_ERROR.WriteTransportFarewell(0)callsiq_conn_close_liberr(0), and in ngtcp2ngtcp2_ccerr_set_liberr(0)resolves tongtcp2_err_infer_quic_transport_error_code(0), which isNGTCP2_NO_ERROR(checked at the pinned ngtcp2 7f7e24b). This is the engine's own immediate-close path, the oneCloseFromEngineand the retention backstop use, with "no error" as the code.H3_NO_ERRORwould be wrong on a raw QUIC (echo) connection.NO_ERRORfor a connection "closed abruptly in the absence of any error".Close(code).The wait bound. There is no wait. Each farewell is one non-blocking
sendmsg; the flag is needed because the UDP sockets are blocking. Shutdown cannot hang on it.FlushGsosends first.EAGAIN, logged asudp send error: -11), as a lost packet would be. The peer then falls back to its idle timeout, which is the behaviour today.Proof. The pending test
quic: a reactor shutdown tells its peers instead of leaving them to time out(QuicTeardownWireTests) is promoted to aTest. Each part was reverted in turn, with E2E registering only that file:the peer was left to time out: 2 datagrams arrived after the shutdown and none was a CONNECTION_CLOSE.the peer was left to time out: 0 datagrams arrived after the shutdown and none was a CONNECTION_CLOSE.The file's other four tests pass in every run: application Close, retention push-back, retention backstop, and the idle sweep staying silent.
Suites. Main 71aab91 against this branch, same box,
taskset -c 0-7,12-31:udp send errorline in any run.ioxideandioxide.ngtcp2build for net10.0 and net11.0 with no warnings.UdpSendTogains one volatile bool read per datagram, of the flag the loop already reads on every pass. Benched with the batch in http3: a request whose body does not match its content-length is refused with H3_MESSAGE_ERROR instead of being served #383 (Http3/ManagedBuffered): +0.5% req/s and -0.4% CPU/req against main, where main against itself was +0.4%. Flat.Interactions checked.
QuicClientEnginealso makes aQuicEngineConnection, so these get the same farewell on their own UDP socket, which is still open inTeardownQuic. A client still inCLIENT_INITIALsends nothing, because ngtcp2 refuses to write a close in that state.OnEvictednow run at shutdown while the connection is still in_quicConnSetand its CID routes. Nothing routes after the loop, so in practice this only meansSendstill works. The handler is now woken byMarkClosedafterOnEvictedrather than before. The engine closes its send gate before that wake, as it does on every otherTeardownpath.All eight together. This batch's PRs (#376 to #383) were merged onto main 71aab91 in one scratch tree. The only conflict is the binary
libioxide_ngtcp2.sobetween #376 and #377, rebuilt there from the merged shim with the script's defaults. Everything builds, and Unit 102, E2E 309, Http 88, Chaos 80, Tls 169 (14 kTLS cases skipped) and File 5 pass, with 0 failed and 0 pending. The repo's only pending test left is the Redis suite's (#336).Noticed, not changed.
QuicRemoveConnectionstill queues aSubmitCancelfor a pinned (migrated) connection at teardown, throughGetSqeOrFlush. On a reactor whose SQ is stuck full, that can still throw. This predates this PR too.Overlap. Merges cleanly (
git merge-tree) with #376, #377, #378 and #379 (issues #349, #200, #354 and #353). It touches no native code, so thelibioxide_ngtcp2.sothat #349 and #200 rebuild is unaffected.