Bun.serve websocket: deliver queued publish() messages when unsubscribing from last topic

Merged
RO
4mo ago
+79-1

Problem

publish() queues messages per subscriber in the uWS TopicTree; they are flushed at the end of the event loop tick. When a ServerWebSocket calls unsubscribe() on its last topic in the same tick as a publish(), the subscriber object is freed with its undrained queue still attached, so messages that publish() had already accepted (and returned a non-zero byte count for) are silently discarded.

// inside a message handler
for (let i = 0; i < 5; i++) server.publish("room", "msg" + i);  // each returns 4
ws.unsubscribe("room");  // ws was only subscribed to "room"
// ws receives none of msg0..msg4

Keeping any other topic subscription on the same socket makes the messages arrive, which shows the loss is a subscriber-lifetime artifact rather than a delivery policy.

Cause

WebSocket::unsubscribe() calls TopicTree::freeSubscriber() when the last topic is removed. freeSubscriber() unlinks the subscriber from the drainable list without draining it and then deletes it, dropping any messages still referenced by messageIndices[].

Fix

Drain the subscriber before freeing it in WebSocket::unsubscribe(). The other two freeSubscriber call sites are unaffected: end() sends a close frame via send(), which drains first; the TCP onClose path has no live socket to write to.

Also corrects the backpressureLimit default in docs/runtime/http/websockets.mdx (16 MB, matching WebSocketServerContext.rs and the type docs; it said 1 MB).

Rebase note

This PR originally also rewrote the publish() return-value JSDoc in packages/bun-types/serve.d.ts to describe the (then) real behavior: the documented 0/-1 backpressure returns were unreachable and publish() always returned the byte count.

That is no longer the case. #32889 landed on main and fixes the underlying contract for real: publish() now aggregates the per-subscriber SendStatus out of uWS and returns 0/-1 on backpressure, and it updated the JSDoc accordingly. The serve.d.ts changes here described the pre-#32889 behavior and are superseded, so they were dropped during the rebase. The remaining diff is the unsubscribe lifetime fix, its regression test, and the backpressureLimit doc correction, none of which overlap with #32889.

Verification

Re-verified against main after the rebase onto #32889 and later.

# test file built with the fix reverted (origin/main's WebSocket.h)
(fail) Server > publish() then unsubscribe() from last topic in same tick delivers queued messages
  expect(bReceived).toEqual(["msg0","msg1","msg2","msg3","msg4","done"])
  Received: ["done"]

$ bun bd test test/js/bun/websocket/websocket-server.test.ts -t "publish\(\) then unsubscribe"
(pass) Server > publish() then unsubscribe() from last topic in same tick delivers queued messages