Repository navigation
Conversation
…refused datagram as sent A UDP send error that belongs to one destination stopped the sends of every session on the endpoint. `packets_out` reported each errno except EMSGSIZE to lsquic as EAGAIN. lsquic reads that as a blocked socket: it pauses the engine, and at the next tick it drops each waiting packet that holds only DATAGRAM frames. - The endpoint passes no receive-error callback, so its socket never gets IP_RECVERR. An ICMP error about one peer no longer fails a send to another peer. `uws::udp::Socket::create` takes the callback as an `Option`. - `packets_out` counts a datagram that the kernel refuses as sent, which lsquic sees as a packet lost on the wire. Only a socket that would block pauses the engine. EMSGSIZE still reaches lsquic. - With no receive-error callback, uSockets closes the socket when a receive fails. `on_close` now ends each session with no packet and closes the endpoint with RECEIVE_FAILURE. `packets_out` with no socket reports the batch as sent, and the close does not wait for the connections that an engine still holds. - `QuicEndpoint.closing` and the endpoint reuse of `connect()` also read the native closing state. A `connect()` after a receive error uses another endpoint. - `bsd_sendmmsg` and `bsd_recvmmsg` get the fault-injection hooks that `bsd_sendmsg` and `bsd_recvmsg` have, so a test can make a UDP send or receive fail. Release builds compile them out.
|
Status How this was reproduced, on linux x64 with release builds of main (bd599f5) and of this branch:
The tests are in CI passed on 803bd64 (build 126065). Each review thread is resolved. The PR waits for a maintainer review. PR: #44800 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughUDP fault injection and send-error handling have changed. QUIC endpoints now handle socket closure by closing sessions and completing endpoint closure. The JavaScript endpoint wrapper includes native closing state in its closing checks and inspection output. Tests cover send failures, receive errors, and endpoint closure. ChangesQUIC UDP Error Handling
Suggested reviewers: Priority: ⬆️ High Merge Risk: ⚪ Minimal · up to
|
There was a problem hiding this comment.
Beyond the inline finding, I also checked the FFI change: recv_error_cb becoming Option<extern "C" fn> matches the C side, which already branches on a NULL recv_error_cb in us_udp_socket_create (udp.c:238) and on u->on_recv_error in loop.c, and the only other caller (src/runtime/socket/udp_socket.rs) passes Some(...). The on_close deref of the user pointer is gated by the self.socket == Some(socket) identity check, so a close issued from release_native after the handle was taken is a no-op.
Extended reasoning...
The change touches the node:quic endpoint's lsquic packets_out send verdict, the uSockets UDP FFI signature (nullable recv_error_cb), a new socket-closed path that silently closes sessions and finishes the endpoint close with CLOSECONTEXT_RECEIVE_FAILURE, and fault-injection hooks in bsd.c; no auth or crypto surface. An inline finding on the ENOBUFS arm of unsent_stops_batch was confirmed, so a human should weigh that before merging; the FFI nullability and on_close pointer-liveness concerns were checked against udp.c and loop.c and are consistent.
A send that fails with ENOBUFS counted as sent, so lsquic saw a lost packet. ENOBUFS means that the output queue of the interface is full. It is common on macOS and it clears by itself. - `packets_out` reports ENOBUFS to lsquic as a blocked socket, as `us_quic_packets_out` does for HTTP/3. uSockets arms the writable poll only when a send would block, so the endpoint arms it with the new `us_udp_socket_poll_writable`. `on_drain` then sends the packets. - `bsd_sendmmsg` on Windows reports WSAENOBUFS as ENOBUFS and not as EIO. `Bun.udpSocket` reads the Winsock code, so its errors do not change. - The tests in quic-endpoint.test.ts have no type errors, and one `afterEach` clears the fault rules.
|
Updated 1:09 PM PT - Oct 15th, 2026
✅ @robobun, your commit 803bd64077859da0fedcec50c1c521619afb3eb7 passed in 🧪 To try this PR locally: bunx bun-pr 44800That installs a local version of the PR into your bun-44800 --bun |
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline finding, I also checked three things on the new revision and ruled them out: the on_close -> on_socket_closed re-entry during release_native (it takes the handle before close(), so the self.socket.get() != Some(socket) guard returns early); the None arm of close_silently pushing Closed without session.schedule_process() (the endpoint process pass walks every session's process_events, so the close is delivered); and the dropped EWOULDBLOCK clause from the old errno rewrite (us_udp_socket_send returns 0 and arms WRITABLE on would-block rather than -1, so the rv == 0 arm covers it on every platform).
Extended reasoning...
The change touches the node:quic endpoint send/receive error handling in src/runtime/node/quic/endpoint.rs, the uSockets UDP socket creation signature (recv_error_cb now optional) and a new us_udp_socket_poll_writable, plus a close_silently path in session.rs and JS-side closing state in quic.ts; it touches no auth, crypto or injection surface. The ENOBUFS re-arm busy-loop finding is posted inline; the other specific lifecycle and errno-mapping concerns examined were ruled out from the code.
One part of this review's analysis that stopped early was run once more. It finished.
The writable poll is the wrong signal for ENOBUFS. The socket stays writable while the output queue of the interface is full, so the poll made the loop send again at once, without a pause. - `packets_out` still reports ENOBUFS to lsquic as a blocked socket. It now sets `retry_unsent`, and the next `process` pass sends the waiting packets again. The timer runs that pass 1 ms later. - `us_udp_socket_poll_writable` is removed again. - Each comment that this branch adds in src/ is one line.
Problem
node:quicendpoint. One forged datagram stalls alisten()endpoint for 4 s, aconnect()to a refused address for 11 s.IP_RECVERR(src/runtime/node/quic/endpoint.rs:1376), andpackets_out(:792) reports each errno exceptEMSGSIZEasEAGAIN.Fix
IP_RECVERR.packets_outcounts a refused datagram as sent. Only a blocked socket orENOBUFSpauses the engine.on_closecloses the sessions and the endpoint, as Node does.test/js/node/quic/quic-endpoint.test.ts(17 tests, 14 fail on main), QUIC, UDP and HTTP/3 suites. Self-reviewed: 18 concerns raised, 17 addressed. Rejected: the errno in the close error (needs a uSockets field).Background
node:quicis experimental.EAGAINfrompackets_outas a blocked socket: it stops the engine and drops DATAGRAM-only packets.IP_RECVERR, a queued ICMP error fails the socket's next send to any destination.Downsides
stats.packetsSentcounts refused datagrams.ENOBUFS: 800 failed sends per second (main: 3).Notes
Reproductions
Release builds of bd599f5 ("main") and of the first revision of this change on top of it, linux x64. The later commits change only the
ENOBUFScase. Release builds of a407eae and of 803bd64 give the same result for the two cases that were run again: 132 and 200 of 200 datagrams with 2 dead peers, and 11024 ms and 3 ms for the request beside a refused session.listen()endpoint: worst round trip of its live session (3 runs)connect("192.0.2.1:443")on the default client endpoint (EINVAL): datagrams that arrivesendmmsgcalls255.255.255.255:443(EACCES) or[::1]:443(EAFNOSUPPORT)ERR_QUIC_TRANSPORT_ERRORat 11.1 slisten()endpoint, 2 dead peers: datagrams the live client receivessendmmsg/recvmsg(MSG_ERRQUEUE)callsconnect()to a dead portENOBUFS, live session closed at 11.1 s127.0.0.1, andconnect()reuses it. Each send from it to an address that is not on the loopback fails withEINVAL.EINVAL. On main that reply stays first in the batch and fails again in each pass.tc), so they are probes and not tests.What lsquic does with a short return
send_batch(vendor/lsquic/src/liblsquic/lsquic_engine.c) readsn_sent < nwithEAGAINas a blocked socket. It clearsENPUB_CAN_SENDfor the engine and gives each unsent packet back to its connection as a delayed packet.EMSGSIZE, it also closes the connection that owns the first unsent packet. WithEMSGSIZEit aborts that connection unless the packet is an MTU probe.lsquic_send_ctl_squeeze_sched. That drops each delayed packet that holds only regenerable frames, and a DATAGRAM frame counts as regenerable. Stream data waits.cn_last_sent, so its packet is first in each batch. That is why one refused destination holds the engine until its handshake timeout (10 s).packets_out. So the callback reports the datagram as sent, and loss recovery does the rest.The send verdict
unsent_stops_batch(endpoint.rs) decides what a failed send means.us_quic_packets_out(packages/bun-usockets/src/quic.c) is the same verdict for HTTP/3 since #42732, and the doc comment names it.EAGAIN: the engine waits foron_drainENOBUFS(the output queue of the interface is full)EAGAIN: the endpoint sends again in its next pass, 1 ms laterEAGAIN: lsquic sends again 1 s laterEMSGSIZEEINVAL,EACCES,EPERM,ENETUNREACH, ...)EAGAIN: the engine stops, and nothing arms the writable pollENOBUFSis backpressure, as in libuv and inquic.c. It is common on macOS and it clears by itself.retry_unsent, and its nextprocesspass callslsquic_engine_send_unsent_packets. The timer runs that pass 1 ms later.bsd_sendmmsgnow reportsWSAENOBUFSasENOBUFSand not asEIO.Bun.udpSocketreads the Winsock code, so its errors do not change.Why both changes
Each change was built alone against the first 8 tests (debug build).
[::1]:443192.0.2.1:443(Linux)255.255.255.255:443(Linux)IP_RECVERR. A send to a live peer still fails with the error of a dead peer, and that datagram is then dropped. The "fragmentation needed" andENOBUFSrows above also stay as on main.IP_RECVERRoff it has nothing to consume.Each other clause has a test that fails when a build removes the clause:
EMSGSIZEarm of the verdictEMSGSIZEcounts as sentENOBUFSarm of the verdictENOBUFScounts as sent, or the pause has no retrylisten()endpoint|| self.socket.get().is_none()in the close gate ofprocess()Nonearm ofclose_silently(a session with no connection yet)closingreads the native state (quic.ts)packets_outwith no socket now returnsnand not 0. It runs when a connection that an engine still holds (a handshake) sends after uSockets closed the socket. A short return there would make lsquic read a stale errno. No test can observe this value, so no test covers it.Measurements
Release builds of a407eae (the merge base) and of 803bd64, unless a line says otherwise.
perf,valgrind,straceandbloatyare not installed on this machine. The counts come from gdb,objdump,size,nmandLD_PRELOADcounters.packets_out, one packet, the send succeeds: 189 to 192 instructions, 25 to 25 conditional branches, 2 to 2 calls (gdbnextiover 10 calls, callee bodies excluded). The 3 instructions are register moves. The loop now continues after a refused datagram, and the compiler allocates registers differently. The verdict is in a cold function that runs only when a send fails.process: +3 instructions in each pass (one compare, one store and one branch forretry_unsent, read from the disassembly).sendmmsgcalls, one message each and 0 failed, against 1704 and 1702.size), data unchanged.packets_out1450 to 1462 bytes,process21,082 to 21,169 bytes,unsent_stops_batch63 bytes,on_close6 to 702 bytes.setsockoptcalls for an endpoint socket: 5 to 4 on IPv4, 5 to 3 on IPv6. An endpoint that is never used creates no socket, as before (first revision).192.0.2.1:443makes 7 failingsendmmsgcalls, andpacketsSentis 7. On main it makes 14 failing calls, andpacketsSentis 0 (first revision).The outage cost. A shim refuses each send of the client with
ENETUNREACHfor the given time. The session writes 64 KB each 50 ms. The table gives the time from the end of the outage to the next byte at the server.packets_outcannot keep both behaviours.The
ENOBUFScost. A shim makes eachsendmmsgof the process fail withENOBUFSfor 1 s, while a stream waits for its answer (debug builds).sendmmsgcalls in the windowWhat a user can see
ENOBUFSleaves 1 ms later. On main it waited for 1 s.endpoint.closedrejects withERR_QUIC_ENDPOINT_CLOSED("Receive failure"). With an injectedrecvmmsgerror, main keeps the sessions open.endpoint.closingis true from the receive error on, and aconnect()with noendpointoption then uses another endpoint. Before this, it took the dying endpoint and failed withERR_QUIC_ENDPOINT_CLOSED.on_closewas empty. This is read from the code, not run.ERR_QUIC_TRANSPORT_ERROR, as on main.Node (v26.3.0)
Endpoint::UDP::Bindpasses onlyUV_UDP_IPV6ONLYandUV_UDP_REUSEPORT(endpoint.cc#L496-L504). libuv setsIP_RECVERRonly forUV_UDP_LINUX_RECVERR(udp.c#L470-L474).uv_udp_sendrequest, andEndpoint::PacketDoneignores the status (endpoint.cc#L1921-L1930). The statistics count at hand-off (endpoint.cc#L956-L957).RECEIVE_FAILURE(endpoint.cc#L458-L461).Endpoint::Destroycloses each session withCloseMethod::SILENT, then closes the socket and emits the close (endpoint.cc#L1291-L1340, session_manager.cc#L126-L145).SEND_FAILUREwhenuv_udp_sendoruv_udp_try_sendreturns an error other thanEAGAIN. On Windows an immediateWSASendToerror does that. Bun drops the datagram on each platform and never emitsSEND_FAILURE.Tests
[::1]:443on each platform: an IPv6 destination cannot leave the IPv4 socket of the endpoint. Linux also runs192.0.2.1:443and255.255.255.255:443. No packet leaves the machine.IP_RECVERRdoes not exist elsewhere.socketFaultInjectionfrombun:internal-for-testing, so they run on debug and ASAN builds and skip on Windows. For them,bsd_sendmmsgandbsd_recvmmsgget the hooks thatbsd_sendmsgandbsd_recvmsghave (3 lines each, compiled out of release builds). These are the only changes in uSockets.listen()tests are set in the pass that sends. The round-trip tests count the failed sends of both endpoints.test/js/node/quic/(39 tests),test/js/bun/udp/(294),serve-http3.test.ts(79),fetch-http3-client(67),fetch-http3-adversarial(27),fetch-http3-cold-post(2),fetch-http3-syscall-fault(7),socket-fault-injection(17).cd test && bun checkreports no type error in the quic tests.quic-sni.test.ts"connect() with the default verifyPeer...",udp_socket.test.ts"does not leak the hostname",fetch-http3-cold-post,fetch-http3-adversarial"8 concurrent POST /echo" with a 4 MB body, andfetch-http3-syscall-fault"a DPLPMTUD probe above the egress MTU...". Each passes when it runs alone.test-dgram-*pass, and 227 of the 235 vendoredtest-quic-*pass on the debug build. The other 8 fail or are unstable on the debug build of main too: 2 neednode:stream/iter, 1 asserts a round-trip time under 10 ms, and 5 aretest-quic-callback-error-*.bun run rust:check-allpasses forx86_64-pc-windows-msvcandaarch64-apple-darwin.Other PRs
us_quic_packets_outinquic.c). The notes of quic: drop a datagram the kernel refuses to send instead of pausing the HTTP/3 engine #42732 left thenode:quicsender for its own PR. This is that PR. After it, bothpackets_outsenders have the verdict (2 of 2), and 2 of the 3 users ofIP_RECVERRare done.Bun.udpSocketandnode:dgramare udp: stop setting IP_RECVERR on UDP sockets #35922.IP_RECVERRfor each unconnected uSockets UDP socket. It removes the ICMP case on Linux and leaves the refused-address case. The two PRs share no lines. If this PR merges first,Nonestays correct. If udp: stop setting IP_RECVERR on UDP sockets #35922 merges first, the 3 ICMP tests pass on the base, andNonethen decides only that a receive error closes the endpoint.UdpHandleralways registerson_recv_error, which putsIP_RECVERRback on the endpoint socket. The port needs an opt-out fornode:quic. The 3 ICMP tests fail without one.IP_RECVERRon the endpoint socket as its trigger. After this PR it needs another trigger, for example thesendmsgfault hook.sendmsghook tobsd_sendmmsg, so the hunk is identical. It also makes uSockets reportENOBUFSas "would block" and arm the poll. The endpoint then gets the poll-driven retry of the table above.bsd_recvmsg, which has its own hook. With both PRs arecvmsgrule is checked once for eachbsd_recvmmsgcall and once for each message on that path.Not in this change
us_quic_udp_on_close(quic.c) only clears the socket pointer, andus_quic_packets_outthen reportsEBADFasEAGAIN. After a receive error on an HTTP/3 socket the engine stays paused. Therecvmsghook of this PR makes that reproducible. It needs its own PR, because the right result for aBun.servelistener is not Node's endpoint close.connect_verneg_probe) is the other send on the endpoint socket. It has no engine, so the verdict does not apply. Its result is still ignored, and a probe that the socket refuses is lost like one lost on the wire.us_quic_packets_outarms the writable poll forENOBUFS, so HTTP/3 sends again in each loop turn while it lasts. The retry in the next pass fits there too.node:quicin uSockets is possible. It would rewrite the merged HTTP/3 sender.EAGAIN.node:quicsends one datagram for eachsendmmsgcall.Self-review
ENOBUFSdecision (after review it is backpressure, with a retry in the next pass), the comment that names the HTTP/3 twin, the send hook and its tests, the reconnect after a receive error, the tests for the close gate and for a session with no connection, the outage cost, and the lists above.Receive failure (-12)forENOMEM, and this PR printsReceive failure (0). The errno reaches a handler only through the receive-error callback, and today that callback also turnsIP_RECVERRon. A new field onus_udp_socket_twould carry it. After udp: stop setting IP_RECVERR on UDP sockets #35922 the callback no longer turnsIP_RECVERRon, and the endpoint can take the errno from it with no change in uSockets.no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/quic/quic-endpoint.test.ts