bake: unsubscribe the HMR socket from topics dropped by a re-subscribe

[robobun]

Problem

• Re-subscribing an HMR socket with fewer topics does not stop the dropped topics: on a released build a socket that re-subscribed with no topics keeps getting u (hot update) frames on every edit.
• On a debug build the first edit after that aborts the dev server with panic: assertion failed: ref_count > 0 in SourceMapStore::put_or_increment_ref_count, from finalize_bundle. A release build keeps a zero-reference source map entry instead.
• Cause: the unsubscribe branch of the subscribe handler had the same condition as the subscribe branch, so it never ran. The socket's own record of its topics was updated, the uws subscription was not, and the two disagreed from then on.
• Second slip, debug builds only: unsubscribing a memory visualizer socket checked the incremental visualizer counter before disarming the memory timer. With an incremental socket open, unsubscribe then re-subscribe of a memory socket hit panic: assertion failed: dev.memory_visualizer_timer.state != EventLoopTimerState::ACTIVE.

Fix

• The unsubscribe guard becomes "in the old set and not in the new set", so a re-subscribe drops the uws topics it no longer names.
• Property to check: after any subscribe frame, the uws subscriptions equal the socket's own record, which is what publishing and the source map reference count both assume.
• The memory visualizer unsubscribe now checks the memory counter, matching the subscribe side that arms the timer.
• Verification: two tests added. Without the first commit the first still receives u after dropping the topic (checked on release and debug builds); without the second commit the second trips the timer assertion on a debug build and is a no-op on builds without the hooks. No in-tree client sends a second subscribe frame today.

Background

• The dev server behind Bun.serve({ development: true }) (bake) serves a WebSocket at /_bun/hmr. Frames start with a one-byte id: s plus topic letters subscribes, n plus a route sets the page URL and is answered directly, and the server publishes ids such as u (hot update) and r (watch synchronization).
• A subscribe frame replaces the socket's whole topic set, so the handler must subscribe added topics and unsubscribe dropped ones.
• The set is tracked twice: uws (the WebSocket library) holds per-topic subscriptions and does the publishing; HmrSocket.subscriptions is the dev server's own bitset. finalize_bundle uses the uws count to decide whether to publish a hot update and the bitset to take one source map reference per receiving socket.
• The source map store reference counts a chunk's source map by the sockets it went to, so publishing to a socket that no reference was taken for can produce a count of 0.
• The M (memory) and v (incremental) visualizer topics exist only on builds with BAKE_DEBUGGING_FEATURES; since Narrow crate-internal Rust visibility across all targets and delete the code it proves dead #36184 they only maintain counters and a timer.

Original description

Repro

Against a dev server (here Bun.serve({ development: true, routes: { "/a": html } }) with a.html loading a.ts, run from the project directory), connect to the HMR socket, subscribe, then re-subscribe with fewer topics, and edit a file:

const ws = new WebSocket(`${server.url}_bun/hmr`);
ws.binaryType = "arraybuffer";
ws.onmessage = e => {
  const id = String.fromCharCode(new Uint8Array(e.data)[0]);
  console.log(id);
  if (id === "V") {
    ws.send("sh"); // subscribe: hot updates
    ws.send("s");  // re-subscribe with no topics
    // ... then edit a.ts
  }
};

Released build: the socket keeps logging u (a hot update frame) on every edit, for the rest of the connection. Debug build: the dev server aborts on the first edit with

panic: assertion failed: ref_count > 0
    SourceMapStore::put_or_increment_ref_count  src/runtime/bake/dev_server/source_map_store.rs:466
    finalize_bundle                              src/runtime/bake/DevServer.rs:4550

Cause

In the IncomingMessageId::Subscribe handler in src/runtime/bake/dev_server/hmr_socket.rs, the branch meant to call ws.unsubscribe was guarded by the same condition as the ws.subscribe branch above it (new && !old twice), so it never ran. A comment next to it already said so. The condition was carried over unchanged from the Zig version of the file.

The handler otherwise treats the frame as a replacement of the whole set: on_unsubscribe is called with the dropped bits and HmrSocket.subscriptions is overwritten with the new set. Only the uws subscription was left behind, so from then on the socket's bookkeeping and uws disagreed. finalize_bundle relies on the two agreeing: num_subscribers(HotUpdate) (uws) decides whether to build and publish a hot update at all, while the source map reference for the chunk is taken once per socket whose is_subscribed(HotUpdate) bit (bookkeeping) is set. With a stale uws subscription the payload is published to a socket that no reference was taken for; when that socket was the only listener and the chunk's script id is new, put_or_increment_ref_count is handed a count of 0, which is the assertion above in debug builds and a zero-reference entry that nothing removes in release builds.

Fix

The guard is now !new && old, so a re-subscribe unsubscribes the dropped uws topics, and the two views of the subscription set stay in sync in every case (added, dropped, unchanged). None of the in-tree clients send a second subscribe frame today, so this is about the protocol behaving as documented by its own handler rather than something the shipped runtime triggers.

The second commit fixes the same kind of slip in on_unsubscribe, two lines below (raised in review): it decrements emit_memory_visualizer_events but then checked emit_incremental_visualizer_events before disarming memory_visualizer_timer. The subscribe path arms the timer when the memory count goes 0 to 1 and asserts it is not already armed, so with an incremental visualizer socket connected, unsubscribing and re-subscribing a memory visualizer socket failed

panic: assertion failed: dev.memory_visualizer_timer.state != EventLoopTimerState::ACTIVE

These hooks only exist on builds with BAKE_DEBUGGING_FEATURES, and since #36184 removed the visualizer routes they only maintain the counters and the timer, so this is bookkeeping consistency rather than anything a user can reach.

Verification

Two tests in test/bake/dev/hot.test.ts, sharing a small helper that opens a second raw /_bun/hmr socket and records the id byte of every frame published to it. Ordering uses SetUrl round trips on that socket rather than timing: the server answers n only after processing the frames sent before it, and after anything it had already published to the socket.

"re-subscribing the hmr socket with fewer topics stops delivery of the dropped topics": for each subscription set hr, r, none, and hr again, rebuilds a file and checks which message ids arrived. The harness's own socket stays subscribed to r, so the r publishes still happen during every rebuild and the only variable is whether the second socket receives them. Without the first commit the r step receives ["r", "u"] and the none step receives ["r", "u"] (checked separately, on the released build and on an unfixed debug build); with it all four steps match, and the snippet above reloads three times without tripping the assertion.

"unsubscribing the last memory visualizer socket stops its timer": holds a v subscriber open, then on a second socket sends sM, s, sM and waits for a round trip. On a debug build without the second commit the dev server fails the assertion above and the test reports the socket closing; with it the round trip completes. On builds without the hooks the frames are no-ops and the test passes either way.

The whole of hot.test.ts (13 tests) passes on a debug build.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c35b00a0-0771-4646-8056-fb11635ddf78

📥 Commits

Reviewing files that changed from the base of the PR and between 9a543cc and f8f2d1b.

📒 Files selected for processing (2)

• src/runtime/bake/dev_server/hmr_socket.rs
• test/bake/dev/hot.test.ts

---

Walkthrough

The HMR socket now removes topics omitted from updated subscriptions. Memory visualizer timer cleanup uses the memory visualizer subscription count. Tests cover topic delivery changes and independent visualizer subscription cleanup.

Changes

HMR subscription lifecycle

Layer / File(s)	Summary
Topic resubscription handling src/runtime/bake/dev_server/hmr_socket.rs, test/bake/dev/hot.test.ts	The socket unsubscribes removed topics. Raw WebSocket fixtures and tests verify hot-update and watch-sync delivery after subscription changes.
Memory visualizer timer cleanup src/runtime/bake/dev_server/hmr_socket.rs, test/bake/dev/hot.test.ts	Timer removal checks the memory visualizer subscription count. The test confirms that another visualizer subscription does not keep the memory timer active.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely describes the primary HMR socket fix.
Description check	✅ Passed	The description explains the problem, fix, background, and verification, although it uses different headings from the template.

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

Status: ready for review. The diff is done; CI is red only on build lanes that could not download vendored dependencies from github.com.

Reproduced on the released build (USE_SYSTEM_BUN=1 bun test test/bake/dev/hot.test.ts -t "re-subscribing the hmr socket"): after re-subscribing with sr, the socket still receives u frames on every rebuild; after s with no topics it still receives ["r", "u"]. On an unfixed debug build the standalone snippet in the description aborts with panic: assertion failed: ref_count > 0 in SourceMapStore::put_or_increment_ref_count.

The second commit (from review) fixes the counter checked in on_unsubscribe; on a debug build without it the second test fails with assertion failed: dev.memory_visualizer_timer.state != EventLoopTimerState::ACTIVE.

With both, test/bake/dev/hot.test.ts (13 tests) passes on a debug build.

CI: in build 93404 every lane that built passed its tests (the only test failures were retried flakes in unrelated files); the darwin x64, linux aarch64 and x64-musl build lanes failed with Failed to download after 5 attempts for WebKit, lol-html, c-ares and mimalloc tarballs. The retriggered build 93453 hit the same outage: the freebsd x64 and windows x64 build lanes (c-ares, lol-html, libuv downloads), verify-baseline (curl exit 56), and test/cli/install/migration/complex-workspace.test.ts on debian x64, where sharp's install script failed downloading libvips from github.com (socket hang up); the other 164 jobs passed. Nothing in either build points at this change.

[robobun]

^{Updated 3:06 PM PT - Aug 12th, 2026}

❌ @robobun, your commit d7b31ce has 4 failures in Build #93453 (All Failures):

• test/cli/install/migration/complex-workspace.test.ts - code 1: error: Failed to install on 🐧 13 x64
• step failed outside runner - exit 56 on 🐧 x64 - verify-baseline
• /opt/winsysroot/Windows Kits/10/Include/10.0.26100/um/winver.h - clang warning on 🪟 x64 - build-bun
• scripts/build/ci.ts - build failed on :freebsd: x64 - build-bun

---

🧪   To try this PR locally:

bunx bun-pr 37878

That installs a local version of the PR into your bun-37878 executable, so you can run:

bun-37878 --bun

[claude]

LGTM — the guard flip is exactly what the removed comment prescribed, and the test's SetUrl round-trip synchronization is a clean way to avoid timing. The inline note on on_unsubscribe is a pre-existing, BAKE_DEBUGGING_FEATURES-gated sibling of the same copy-paste class; worth folding in but not a blocker.

Extended reasoning...

Overview

One-line condition fix in src/runtime/bake/dev_server/hmr_socket.rs: the else if guarding ws.unsubscribe in the Subscribe handler was new && !old (identical to the if above it, so unreachable) and is now !new && old. The removed code comment already documented this as the intended fix. A new test in test/bake/dev/hot.test.ts opens a raw /_bun/hmr socket, cycles through subscription sets hr → r → none → hr, and asserts exactly which message ids arrive after each rebuild.

Security risks

None. The HMR socket is a dev-server-only local protocol; the change narrows delivery (unsubscribes stale topics) rather than widening any surface. No auth, crypto, or untrusted-input parsing is touched.

Level of scrutiny

Low-to-medium. The Rust change is a single boolean condition whose correct form was already spelled out in a comment at the site, and the surrounding handler already did the matching bookkeeping (on_unsubscribe(!new_bits & self.subscriptions) and self.subscriptions = new_bits) — only the uws-side unsubscribe was dead. The test is where most of the review effort went: it wires error/close to reject every pending promise, uses SetUrl round-trips on the same socket for ordering (no sleeps), and relies on dev.write resolving via the harness socket's watch-synchronization ack so the rebuild's publishes have already been enqueued to the second socket before the post-write roundTrip() drains them. Checked bake-harness.ts to confirm dev.write → batchChanges → waitForHotReload awaits the watch_synchronization event, so the ordering assumption holds without a client attached.

Other factors

The bug hunter flagged one same-class pre-existing issue in on_unsubscribe (memory-visualizer timer removal checks emit_incremental_visualizer_events instead of emit_memory_visualizer_events). It's real but gated behind feature_flags::BAKE_DEBUGGING_FEATURES, not introduced here, and a one-word fix — noted inline as a nit rather than a blocker. Also checked that the test's [...new Set(received)].sort() normalization can't mask the failure mode: the unfixed build produces ["r", "u"] on the r-only and empty steps, which the exact-array assertions reject. No prior human review comments to address.

[robobun]

Folded the on_unsubscribe counter fix into this PR as a second commit (f8f2d1b), since it is the same kind of slip in the same handler. It now disarms memory_visualizer_timer based on emit_memory_visualizer_events, and a second test covers it: with a v subscriber connected, sM, s, sM on another socket used to fail the memory_visualizer_timer.state != EventLoopTimerState::ACTIVE assertion on debug builds. The two tests share a small helper for the raw hmr socket; the PR description is updated accordingly.

[claude]

LGTM — the follow-up commit addresses the on_unsubscribe counter mismatch I flagged, with a test covering it.

Reviewed: the two one-line condition fixes in hmr_socket.rs (both were copy-paste slips, the first already flagged by a code comment); the new openHmrSocket helper and both tests use SetUrl round-trips for ordering rather than sleeps, wire error/close to reject the awaited promise, and clean up via using. Checked that a.html/b.html map to /a and /b under the multi-file branch of indexHtmlScript, so the alternating roundTrip probe always resolves. The second test has no explicit expect but is not vacuous — the assertion failure closes the socket, which rejects roundTrip().

Extended reasoning...

Overview

Two one-line fixes in src/runtime/bake/dev_server/hmr_socket.rs, plus two tests and a shared helper in test/bake/dev/hot.test.ts.

The first change flips the else if guard in the Subscribe handler from new && !old (dead — identical to the if above it, and a code comment already said so) to !new && old, so re-subscribing with fewer topics actually calls ws.unsubscribe for the dropped ones. Without it, self.subscriptions and the uws subscription set diverge, which the PR description traces to the ref_count > 0 assertion in SourceMapStore.

The second change is the fix for the same-class issue I raised on the previous revision: on_unsubscribe now gates memory_visualizer_timer removal on emit_memory_visualizer_events == 0 instead of the unrelated incremental counter. The author added a dedicated test and updated the PR description to cover it.

Security risks

None. This is internal dev-server HMR socket bookkeeping. The Subscribe handler already bounds topics.len() and validates each byte via HmrTopic::from_u8; nothing here changes what input is accepted or how it's parsed.

Level of scrutiny

Low-to-medium. Both source changes are single-token corrections to conditions whose intent is unambiguous from the surrounding code (the subscribe branch mirrors the unsubscribe branch; the timer-arm branch mirrors the timer-disarm branch). Neither changes control flow beyond making the documented path reachable. The visualizer fix is additionally gated behind BAKE_DEBUGGING_FEATURES.

The tests warranted closer reading. openHmrSocket follows the pattern of the existing "hot update frames are not delivered to application websocket topics" test just above it. Ordering is established by SetUrl round-trips on the same socket (per-socket WebSocket frame ordering guarantees the n response arrives after anything already queued), not by sleeping. onerror/onclose are wired into fail(), which rejects both the open promise and every pending roundTrip, so a server crash surfaces as a test failure rather than a hang. I checked indexHtmlScript in bake-harness.ts: with two HTML files it maps a.html→/a and b.html→/b, so the alternating probe always hits a valid route and the SetUrl handler always replies. The dedupe-and-sort on received makes the first test's assertions robust to multiple r frames per rebuild.

Other factors

My previous review's only finding has been addressed in full (code + test + description). The rest of hot.test.ts is reported passing on a debug build. No outstanding human reviewer comments.

[claude]

LGTM — both fixes are the obvious correction to copy-paste slips (one already had a comment calling itself a bug), and the earlier review point about on_unsubscribe checking the wrong counter is now addressed with a test.

What was reviewed:

• The !new && old guard now mirrors the subscribe branch; on_unsubscribe(!new_bits & self.subscriptions) right after already handles the visualizer counters, so the else-if correctly only needs the ws.unsubscribe call.
• The timer disarm now checks emit_memory_visualizer_events, symmetric with the 0→1 arm path above it.
• Tests: no sleeps — ordering is via SetUrl round-trips on the same socket, and error/close are wired to reject pending promises so a server crash surfaces as a test failure rather than a hang. using handles cleanup.

Extended reasoning...

Overview

Two one-line fixes in src/runtime/bake/dev_server/hmr_socket.rs:

1. In the Subscribe handler, the else if guarding ws.unsubscribe was identical to the if above it (new && !old twice), so it never ran. The code even had a comment stating this was likely a bug. Now !new && old, so dropped topics are unsubscribed from uws and the socket's subscriptions bitset stays in sync with the uws subscription set.
2. In on_unsubscribe, the memory visualizer timer disarm was gated on emit_incremental_visualizer_events == 0 (wrong counter). Now checks emit_memory_visualizer_events == 0, matching the subscribe path that arms the timer on that counter's 0→1 transition.

Two new tests in test/bake/dev/hot.test.ts plus a shared openHmrSocket helper.

Security risks

None. This is internal dev-server subscription bookkeeping on a local-only HMR WebSocket. No auth, crypto, or untrusted-input parsing changed; the message parsing paths touched already length-check their frames.

Level of scrutiny

Low-to-medium. The Rust changes are two obvious copy-paste corrections in a handler whose surrounding structure (loop over all topics, then on_unsubscribe for the dropped set, then overwrite self.subscriptions) already assumed exactly this behavior. Per the PR description, no in-tree client sends a second subscribe frame today, so the first fix is protocol correctness rather than a user-visible behavior change; the second is behind BAKE_DEBUGGING_FEATURES. I verified the else if branch correctly only calls ws.unsubscribe (the on-unsubscribe hooks for visualizer counters are already handled by the self.on_unsubscribe(!new_bits & self.subscriptions) call two lines later).

Other factors

• The one prior review comment (mine, flagging the wrong counter in on_unsubscribe) has been fully addressed in f8f2d1b with a dedicated test; the thread is resolved.
• Test design is solid per REVIEW.md: no setTimeout/sleep — synchronization uses SetUrl round-trips whose response is ordered after any frames the server had already published on the same connection. Every failure event (error, close) rejects the awaited promise, so a dev-server crash (the pre-fix debug assertion) fails the test instead of hanging. Resources are released via using/Symbol.dispose.
• The author confirmed the first test fails on USE_SYSTEM_BUN=1 (still receives u after unsubscribing) and the second fails on an unfixed debug build with the timer-state assertion, satisfying the "fails for the right reason" requirement.
• The full hot.test.ts suite (13 tests) passes on a debug build.

[robobun]

Heads up: #37936 changes the same on_unsubscribe hunk (it also switches the condition to the memory visualizer count, and moves the remove into a DevServer::disarm_memory_visualizer_timer helper because it restores the empty tick handler that was leaving the timer node stale in the heap). It also adds tests at the same spot in test/bake/dev/hot.test.ts. The two PRs are otherwise independent; whichever lands second needs a small rebase of those two hunks.