websocket: do not hold an exclusive client reference across user callbacks

Merged
JA
4mo ago
+1232-1501

What

The WebSocket client's receive/dispatch path no longer holds an exclusive (&mut self) reference to the client across user callbacks. The receive-side methods (consume, handle_data, the control-frame handlers, dispatch_*, clear_data, fail, terminate, handle_close, …) now take &self, and the fields they mutate use per-field interior mutability — Cell for the scalars and small fixed buffers, RefCell for the two byte FIFOs and the deflate state (each borrow is scoped and released before any dispatch into JS). This is the same shape ServerWebSocket already uses. The uWS trait handlers and the proxy-tunnel failure path additionally hold a scoped reference for the duration of the call, so the final release can only happen through the root pointer after the handler returns. The only new import is AsCtxPtr.

Net effect on unsafe in src/http_jsc/websocket_client.rs: 85 on main → 83 here — two blocks removed, none added, and several &mut *p derefs weakened to &*p.

Why

A JS message/ping/pong/close handler can synchronously call back into the client (send(), close(), terminate()) while the client is still inside its receive loop dispatching that very event. Previously the receive loop held &mut self across those callbacks, so the re-entrant call aliased an exclusive reference. With &self receivers and interior mutability, nested calls mutate state through their own short-lived borrows instead of aliasing an outstanding &mut.

Tests

test/js/web/websocket/websocket.test.js — describe("WebSocket message handler re-entrancy during a multi-frame read"), against a raw TCP server that sends three text frames (A, B, C) in one TCP segment so a single native receive pass dispatches all of them:

  • re-entrant send(): a send("from-handler") issued synchronously from the first message event still delivers all three messages, and the test decodes the client→server frame off the raw socket to prove the re-entrant write actually went out mid-loop; clean 1000 close.
  • re-entrant close(): send() + close(1000, "bye") from the first message event suppresses the remaining buffered frames (["A"] only) and closes cleanly with code 1000.
  • re-entrant terminate(): send() + terminate() from the first message event suppresses the remaining frames and closes with 1006, wasClean: false.

This change has no observable behavior delta by design, so these tests also pass on the released build; they exist as a behavior lock for the re-entrant paths plus debug/ASAN coverage of the receive loop under re-entry.

How it was verified

  • The full test/js/web/websocket/ suite and the permessage-deflate suites pass.
  • The complete Autobahn conformance suite (517 cases, including all permessage-deflate sections) produces byte-identical results to a main-lineage build.
  • Targeted re-entrancy probes (compressed send() from a handler over inbound deflate, close() under 6 MB of backpressure, ping-handler re-entry) are byte-identical to main.
  • cargo clippy --workspace, bun run rust:check-all (10/10), rustfmt, prettier all clean.