Bun.serve: keep the WebSocket ping deadline across server sends, add resetIdleTimeoutOnSend

Closed
RO
23d ago
+222-4

Problem

  • A Bun.serve WebSocket server that keeps sending to a client never closes it on idleTimeout. At idleTimeout: 8 with one send per second, a client that ignores every ping stays open.
  • send() (packages/bun-uws/src/WebSocket.h:224) re-armed the idle timer and cleared hasTimedOut. Each send cancelled the pending ping deadline.
  • src/runtime/server/WebSocketServerContext.rs:236 hard-codes reset_idle_timeout_on_send: true.

Fix

  • send() leaves the timer and hasTimedOut alone while a ping is unanswered. An unanswered ping now closes the socket: 1006 "WebSocket timed out from inactivity".
  • New option websocket.resetIdleTimeoutOnSend, default true (current behavior). With false, server sends do not restart the timer.
  • Verified: test/js/bun/websocket/websocket-server.test.ts (6 of 10 new tests fail on main). Also bun-types.test.ts.

Background

  • uws splits idleTimeout into an idle part and a ping margin (4, 8 or 16 s). onTimeout pings after the idle part, sets hasTimedOut, and closes after the margin.
  • resetIdleTimeoutOnSend is the uws flag that lets server sends restart the idle part. Upstream defaults it to false.
  • Considered a new default. It pings clients that never saw a ping, so a maintainer must decide.

Downsides

  • A client whose pong arrives after the margin is now closed while the server sends. Measured: a 2 MB burst read at 200 KB/s, closed at 8 s (idleTimeout: 8).
  • Default options at idleTimeout: 16 with one send per second: a silent client is still open at 30 s. Option off: a client that only drains queued data stays open until the drain ends.
  • send() runs 2 more instructions (release disassembly). Binary section sizes do not change.
Notes

What changes by default, and what is opt-in

  • Default: an unanswered automatic ping closes the socket at the ping deadline, also when the server sent to the client in between. Clients that answer pings see no change. Servers with sendPings: false see no change.
  • Opt-in (resetIdleTimeoutOnSend: false): server sends stop restarting the idle timer.
  • The default of the option does not change in this PR. One possible follow-up is false when sendPings is on. A push server then pings and reaps silent clients at any idleTimeout, and its clients receive pings that they did not receive before.

Why idleTimeout: 8 shows the bug with default options

  • us_socket_timeout(s, 4) arms the timer one 4 s tick ahead, so it fires at the next sweep even if a send re-armed it. The ping always goes out. Only the hasTimedOut clear in send() kept the socket open.
  • From idleTimeout: 16 up the idle part is two or more ticks. A server that sends in every tick re-arms it before it fires, and no ping goes out. The option covers that case.

Measurements

Linux x64, loopback, release builds of the merge base (37471e585e) and of this branch. The client is a raw socket in a separate process with a fixed read rate.

Casemainthis PR
idleTimeout: 8, 1 send/s, client never writesopen at 24 s, pings at 4, 8, 12, 16, 20, 24 sping at 4.0 s, closed at 8.0 s
idleTimeout: 8, no sends, client never writesping at 4.0 s, closed at 8.0 sping at 4.0 s, closed at 8.0 s
idleTimeout: 8, 1 send/s, client answers pingsopen at 30 s (7 pings)open at 30 s (7 pings)
idleTimeout: 8, 2 MB burst then 1 send/s, client answers pings and reads 200 KB/sopen at 30 sclosed at 8.2 s (the ping reaches the client at 11 s)
same, server quiet after the burstclosed at 9.6 sclosed at 9.4 s
same, 1 MB burst then 1 send/sopen at 30 sopen at 30 s
idleTimeout: 16, default options, 1 send/s, client never writesopen at 30 s, 0 pingsopen at 30 s, 0 pings
idleTimeout: 16, option off, 1 send/s, client never writesoption does not existping at 8.0 s, closed at 16.0 s
idleTimeout: 16, option off, 1 send/s, client answers pingsoption does not existopen at 30 s, pings at 8, 16, 24 s
idleTimeout: 8, sendPings: false, 1 send/s, client never writesopen at 20 sopen at 20 s
idleTimeout: 8, option off, one 48 MB message, client reads 2 MB/s and never writesoption does not existopen during the transfer (28 drain events), closed at 32.1 s

Every close in the table is 1006 "WebSocket timed out from inactivity".

idleTimeout: 0

idleTimeout: 0 with pings on is not off on main: calculateIdleTimeoutComponents wraps 0 - 4 to 65532, which arms a 252 s timer (#34222 covers that). Measured with a client that never writes:

Servermainthis PR
no sendsping at 253 s, closed at 257 sping at 253 s, closed at 257 s
1 send/s from the startopen at 300 s, 0 pingsopen at 300 s, 0 pings
quiet for 253 s, then 1 send/sopen at 300 sclosed at 257 s

This PR changes one case there: a send that lands in the 4 s margin after that ping does not cancel the close.

Cost

  • uWS::WebSocket<false, true, void*>::send (plain TCP), path of a successful send: +1 cmpb, +1 jne, +1 jmp, -1 movb. That is 2 more instructions, 1 more load and 1 less store.
  • Symbol sizes: the four send() instantiations grow by 18, 10, 2 and 2 bytes. web_socket_server_context::on_create grows by 112 bytes. size -A reports the same size for every section of the stripped binary (.text 58159541, .rodata 19804908).
  • on_create reads one more property when Bun.serve parses the websocket object. Nothing is added per connection or per message.
  • perf and valgrind are not available in the build container, so there is no dynamic instruction count. The count is from objdump of the two release builds.

Review points that did not change the code

  • With the option off, onWritable still restarts the timer when queued data drains. This is upstream's behavior for the flag. It keeps a slow client alive during a large transfer. A peer that has gone away stops draining when the buffers in front of it are full.
  • There is no test for { resetIdleTimeoutOnSend: false } with pings on. At idleTimeout: 8 it takes the same path as the default-options test. From 16 up it needs a 16 s test. The table covers it by hand.

Tests run

  • bun bd test test/js/bun/websocket/websocket-server.test.ts -t "idleTimeout while the server keeps sending|resetIdleTimeoutOnSend": 10 pass. With src/ and packages/ from main: 6 fail (4 validation, 2 behavior).
  • The whole file with the debug ASAN build: 148 pass, 3 todo, 4 time out on a loaded machine. The 4 spawn a client process and pass when run alone.
  • bun test test/integration/bun-types/bun-types.test.ts: 22 pass.

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/bun/websocket/websocket-server.test.ts, test/integration/bun-types/fixture/serve-types.test.ts