webcore: revoke ObjectURLRegistry entries on Worker teardown

[robobun]

What

URL.createObjectURL(blob) stores a duped Blob in a process-global ObjectURLRegistry. When called inside a Worker, those entries were never removed if the worker exited (naturally or via terminate()) without an explicit URL.revokeObjectURL(), so the Blob data leaked for the lifetime of the process.

Repro

import { resolveObjectURL } from "node:buffer";
import { Worker } from "node:worker_threads";

const w = new Worker(`
  const { parentPort } = require("node:worker_threads");
  parentPort.postMessage(URL.createObjectURL(new Blob(["x"])));
  setInterval(() => {}, 1e6);
`, { eval: true });

const url = await new Promise(r => w.once("message", r));
await w.terminate();
console.log(resolveObjectURL(url)); // before: Blob, after: undefined

Cause

ObjectURLRegistry is a singleton UUID → Entry map with no notion of which context created the entry. WebWorker.exitAndDeinit() tears down the worker's VM but never touches the registry, so every worker-created entry is orphaned.

Fix

• Tag each Entry with the creating VM's ScriptExecutionContext id (via a new VirtualMachine.scriptExecutionContextId() helper).
• Add ObjectURLRegistry.revokeEntriesForContext(context_id) which drops all matching entries under the registry lock.
• Call it from WebWorker.exitAndDeinit() alongside the existing CronJob.clearAllForVM cleanup, before WebWorker__dispatchExit so the parent's close event observes the URLs as already revoked.

This mirrors WebKit's BlobURLRegistry::unregisterURLsForContext / PublicURLManager::stop() and the FileAPI spec's unloading-cleanup step for blob URL stores.

Verification

New tests in test/js/web/workers/worker_blob.test.ts:

• blob URLs created inside a Worker are revoked when the worker terminates — creates several blob URLs in a worker, confirms they resolve while alive, then asserts they're gone after terminate(). Also asserts a parent-context URL is left alone.
• blob URLs created inside a Worker are revoked when the worker exits naturally — same for natural worker exit.

Both fail on main (Received: Blob (N bytes) where undefined is expected) and pass with this change.

[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: e96d4566-96c5-4850-ac0a-9778ab99a251

📥 Commits

Reviewing files that changed from the base of the PR and between 07d887e and 9e26a2e.

📒 Files selected for processing (4)

• src/bun.js/VirtualMachine.zig
• src/bun.js/web_worker.zig
• src/bun.js/webcore/ObjectURLRegistry.zig
• test/js/web/workers/worker_blob.test.ts

---

Walkthrough

This PR implements automatic revocation of Blob/Object URLs created inside Workers upon Worker termination or natural exit. It adds context tracking to the ObjectURLRegistry so each registered URL is tagged with its originating execution context, then revokes all matching URLs during Worker shutdown.

Changes

Worker-Created URL Auto-Revocation

Layer / File(s)	Summary
Helper Function src/bun.js/VirtualMachine.zig	Adds exported inline scriptExecutionContextId() that returns the VM's initial_script_execution_context_identifier cast to u32.
Data Shape src/bun.js/webcore/ObjectURLRegistry.zig	Entry struct gains context_id: u32 field to track which execution context registered each Blob URL.
Registry Core src/bun.js/webcore/ObjectURLRegistry.zig	Entry.init() signature updated to accept context_id; register() captures context via vm.scriptExecutionContextId(); new revokeEntriesForContext() function removes all entries matching a given context ID under the registry mutex.
Worker Integration src/bun.js/web_worker.zig	Worker shutdown() calls ObjectURLRegistry.singleton().revokeEntriesForContext(this.execution_context_id) after socket teardown and before exit dispatch, clearing worker-created URLs.
Tests test/js/web/workers/worker_blob.test.ts	Imports resolveObjectURL from node:buffer; adds two new tests: one verifies Worker-created URLs become unresolvable after worker.terminate() while parent URLs persist, and another confirms auto-revocation on natural Worker exit.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: revoking ObjectURLRegistry entries when a Worker shuts down, which is the core objective of this PR.
Description check	✅ Passed	The PR description comprehensively covers both required template sections with detailed context, including What, How verified, root cause, fix, and alignment with existing patterns.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Review rate limit: 4/5 reviews remaining, refill in 12 minutes.}

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

[robobun]

^{Updated 10:23 PM PT - May 3rd, 2026}

@autofix-ci[bot], your commit 9e26a2e is building: #50851

[github-actions]

Found 1 issue this PR may fix:

1. macOS Apple Silicon: memory invisible to RSS — bmalloc slabs, worker cleanup gaps, GC safety bugs #28318 - Partially addresses "worker cleanup gaps" described in this meta-issue by revoking orphaned ObjectURLRegistry entries when workers exit

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #28318

🤖 Generated with Claude Code

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/bun.js/VirtualMachine.zig`:
- Around line 1302-1307: createForTestIsolation() currently swaps the VM global
without cleaning up ObjectURL entries, causing blob URLs to leak because the old
global reuses the same context id; before replacing the VM's global in
createForTestIsolation(), call ObjectURLRegistry.revokeEntriesForContext() using
the old global's context id (obtainable via
VirtualMachine.scriptExecutionContextId(oldGlobal) or the old VM instance) to
revoke blob/object URLs tied to that context, mirroring the worker teardown path
that uses revokeEntriesForContext() in web_worker.zig.

In `@test/js/web/workers/worker_blob.test.ts`:
- Around line 129-193: The tests currently assert URL revocation only after
awaiting the worker "close" promise, which misses regressions where revocation
happens during the close event; update both tests to perform the assertion(s)
inside the worker's "close" event handler callback instead of after awaiting:
for the first test, move the loop that checks resolveObjectURL(url) returns
undefined into the close listener created with worker.addEventListener("close",
...) (the existing closed promise callback), and for the second test likewise
perform the expect(resolveObjectURL(url)).toBeUndefined() inside the close event
handler that currently resolves the promise; keep using the same variables
(worker, url(s), resolveObjectURL) and once: true listener semantics.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3b7218e9-6a00-4430-b125-6e5900850a2e

📥 Commits

Reviewing files that changed from the base of the PR and between a97e868 and d242f64.

📒 Files selected for processing (4)

• src/bun.js/VirtualMachine.zig
• src/bun.js/web_worker.zig
• src/bun.js/webcore/ObjectURLRegistry.zig
• test/js/web/workers/worker_blob.test.ts

[claude]

I didn't find any bugs and the change follows existing patterns (same lock discipline as revoke(), same teardown placement as CronJob.clearAllForVM), but it touches the carefully-ordered worker exitAndDeinit path and CI is currently red on build-zig/build-cpp across all platforms, so this should get a human look before merging.

Extended reasoning...

Overview

This PR tags each ObjectURLRegistry entry with the creating VM's ScriptExecutionContext id, adds revokeEntriesForContext() to drop all entries for a given context under the existing registry lock, and calls it from WebWorker.exitAndDeinit() right after CronJob.clearAllForVM. A trivial VirtualMachine.scriptExecutionContextId() accessor is added, plus two regression tests in worker_blob.test.ts.

Security risks

None apparent. No new untrusted input is parsed; the new code path is a cleanup hook that iterates an internal map under an existing mutex and frees duped Blob entries. No auth, crypto, or permission surface is touched.

Level of scrutiny

Moderate-to-high. The diff is small and mechanically straightforward, but it inserts a new step into exitAndDeinit, which has a ~100-line comment block describing precise ordering invariants between JSC teardown, WebWorker__dispatchExit, arena destruction, and parent-thread observability. The new call runs on the worker thread, holds the process-global registry lock, and invokes Blob.deinit() (which derefs the underlying store) — this matches what the existing revoke() already does under the same lock, so it should be fine, but a maintainer familiar with Blob store lifetimes (file/S3-backed blobs, etc.) should confirm there's no teardown-ordering hazard.

Other factors

• CI (robobun, build #48818) reports build-zig and build-cpp failures on every platform for commit 5db3d1b; the subsequent autofix.ci commit is formatting-only, so it's unclear whether the build is green yet. I won't auto-approve past a red build.
• CodeRabbit's two minor notes (test-isolation cleanup and asserting inside the close handler) are scope-adjacent suggestions rather than defects in this change.
• Good test coverage is added for both terminate() and natural-exit paths, including a check that parent-context URLs survive.

[claude]

The core fix looks correct and well-tested, but this bundles a user-visible behavioral change (worker-created blob URLs are now auto-revoked) into the worker teardown path plus an unrelated-looking stripFlags build change in scripts/build/flags.ts — worth a human glance at both.

Extended reasoning...

Overview

This PR tags each ObjectURLRegistry.Entry with the creating VM's ScriptExecutionContext id, adds revokeEntriesForContext() (lock → collect matching UUIDs → fetchRemove + deinit), and calls it from WebWorker.exitAndDeinit() alongside the existing CronJob.clearAllForVM cleanup. A trivial VirtualMachine.scriptExecutionContextId() accessor is added, and two new tests in worker_blob.test.ts cover both terminate() and natural-exit paths, including a check that parent-context URLs are left alone. Separately, scripts/build/flags.ts narrows the Linux .eh_frame strip rule to c.lto only, to match the --no-eh-frame-hdr link flag and avoid a SIGSEGV in pthread_exit() unwind on non-LTO release builds.

Security risks

None identified. No new untrusted input parsing; the registry mutex is held for the new sweep exactly as it is for the existing revoke() path, and entry.deinit() under the lock matches the existing pattern. The build-flag change only narrows when sections are stripped (strictly more conservative).

Level of scrutiny

Medium-high. The Zig changes are small and mechanically sound (the two-pass collect-then-remove is the correct HashMap idiom, and blob.deinit() under the lock mirrors revoke()). However:

• exitAndDeinit() is a historically delicate path (the file has ~100 lines of header docs on its threading model), and this adds a new cross-thread registry mutation there.
• This is a user-visible behavioral change: code that previously relied on resolving a worker's blob URL after terminate() will now get undefined. It matches the FileAPI spec and WebKit, but it's still an observable change worth a maintainer ack.
• The flags.ts change is build-infrastructure for the production strip step. The rationale in the comment is sound and the existing comment already notes CI release always has LTO on, so this should be a no-op for shipped binaries — but build-pipeline changes deserve a human look.

Other factors

The two CodeRabbit comments (test-isolation cleanup, assertion placement) were both resolved by the author with reasonable explanations and acknowledged as out-of-scope / intentional. No bugs were found by the bug-hunting system. Test coverage for the new behavior is good. CI shows Windows build-agent provisioning failures that appear to be infra, not code-related.

[claude]

I didn't find any bugs, but this touches the worker teardown path (exitAndDeinit, threading-sensitive), introduces a user-visible behavior change for blob URL lifetime, and bundles an unrelated build-strip gating change in flags.ts — worth a quick human look.

Extended reasoning...

Overview

This PR fixes a memory leak where URL.createObjectURL() entries created inside a Worker were never removed from the process-global ObjectURLRegistry when the worker exited. It tags each registry Entry with the creating VM's ScriptExecutionContext id, adds revokeEntriesForContext() (lock-held two-pass collect-then-remove over the AutoHashMap), and calls it from WebWorker.exitAndDeinit() next to the existing CronJob.clearAllForVM cleanup. A trivial VirtualMachine.scriptExecutionContextId() accessor is added, and two regression tests are added to worker_blob.test.ts. Separately, scripts/build/flags.ts gates the .eh_frame strip step on c.lto so non-LTO linux-gnu release builds don't end up with a dangling PT_GNU_EH_FRAME phdr that crashes pthread_exit() during worker teardown.

I verified that the id used at registration time (vm.scriptExecutionContextId() → initial_script_execution_context_identifier) and at revocation time (this.execution_context_id) are the same value — initWorker at VirtualMachine.zig:1416 sets the former from the latter. The lock/iterate/remove pattern matches the existing revoke() and is correct for Zig's std.AutoHashMap (no removal during iteration). Entry.deinit() under the registry lock mirrors the existing single-revoke path, so any Blob-store deref thread-safety concern would be pre-existing.

Security risks

None identified. No auth, crypto, or untrusted-input parsing is touched. The registry mutation runs under its existing mutex; the new code path only deletes entries the dying worker itself created.

Level of scrutiny

Moderate-to-high. exitAndDeinit() is a threading-sensitive teardown path with a documented lock/ordering contract, and this is a user-visible behavior change (worker-created blob URLs now resolve to undefined after the worker exits — spec-correct and matches WebKit/Node, but still a change). The bundled flags.ts change alters how release binaries are stripped on linux-gnu; the rationale is sound and CI (always LTO) is unaffected, but it's a separate concern riding along in the same PR.

Other factors

The bug-hunting system found no issues. Both CodeRabbit inline comments were resolved by the author with reasonable explanations (test-isolation cleanup deferred as out of scope; assertion placement after await closed is equivalent and avoids a hang-on-failure mode). No CODEOWNERS apply. The new tests cover both terminate() and natural exit and assert parent-context URLs survive. Overall the implementation looks correct to me, but the combination of worker-lifecycle code, behavior change, and build-config tweak makes this worth a human reviewer's eyes rather than a bot approval.

[claude]

I didn't find any bugs, but this combines a user-visible behavioral change (worker-created blob URLs now auto-revoke on teardown) with a build-system change to Linux strip flags and touches the cross-thread worker teardown path — worth a human look.

Extended reasoning...

Overview

This PR tags each ObjectURLRegistry entry with the registering VM's ScriptExecutionContext id, adds revokeEntriesForContext() (lock-held collect-then-remove sweep), and calls it from WebWorker.exitAndDeinit() so blob URLs created inside a Worker are dropped when the Worker exits. It also adds a trivial scriptExecutionContextId() accessor on VirtualMachine, two regression tests in worker_blob.test.ts, and — separately — gates the Linux glibc .eh_frame/.eh_frame_hdr strip step on c.lto in scripts/build/flags.ts so non-LTO release builds don't SIGSEGV in pthread_exit() unwind during Worker teardown.

Security risks

None identified. No auth/crypto/permissions surface; the registry sweep runs under the existing mutex, and the only new cross-thread interaction is Blob.deinit() on the worker thread for a duped Blob whose Store already uses atomic refcounting.

Level of scrutiny

Medium-high. Individually each piece is small and well-reasoned, but together they touch:

• Worker teardown (exitAndDeinit), which the file's own header documents as a delicate cross-thread sequence with strict ordering invariants. The new call is placed after closeAllSocketGroups and before WebWorker__dispatchExit, reading this.execution_context_id while this is still guaranteed live — that placement looks correct, but this path historically rewards careful eyes.
• A user-observable behavior change: blob URLs created in a Worker are no longer resolvable from the parent after the Worker exits. Spec-aligned and matches WebKit's unregisterURLsForContext, but still a semantic change someone could be relying on.
• A release-binary build change: narrowing when .eh_frame* sections are stripped on Linux glibc. The rationale (matching the existing --no-eh-frame-hdr LTO gate so PT_GNU_EH_FRAME never dangles) is sound and well-commented, but build-flag changes that affect what ships deserve a maintainer sign-off.

Other factors

The bug-hunter found no issues. Both CodeRabbit threads were reasonably resolved by the author (test-isolation gap is pre-existing/out-of-scope; assertion placement is correct). Tests cover both terminate() and natural exit, and verify parent-context URLs are untouched. The revokeEntriesForContext implementation correctly avoids mutate-during-iterate and deinit()s each removed entry. I'm deferring rather than approving primarily because of the bundled build-flag change and the behavioral-change nature of the fix, not because of any specific concern with the code.

[claude]

I didn't find any bugs, but this combines a user-observable behavior change in worker teardown (blob URLs now auto-revoked) with a build-time strip-flag fix in scripts/build/flags.ts — worth a human look at both the concurrency placement in shutdown() and the .eh_frame strip gating.

Extended reasoning...

Overview

This PR fixes a Blob leak by tagging each ObjectURLRegistry entry with the creating VM's ScriptExecutionContext id and sweeping matching entries during WebWorker.shutdown(). It touches src/bun.js/VirtualMachine.zig (trivial accessor), src/bun.js/webcore/ObjectURLRegistry.zig (new context_id field + revokeEntriesForContext under the registry lock), src/bun.js/web_worker.zig (one call in the teardown sequence), and adds two regression tests in worker_blob.test.ts. It also includes a separate build-system change in scripts/build/flags.ts that gates .eh_frame section stripping on c.lto to avoid a SIGSEGV in pthread_exit() unwind on non-LTO GNU/Linux builds.

Security risks

None identified. The registry remains process-global and mutex-guarded; the new sweep runs under the same lock as existing revoke() and calls the same Entry.deinit() path. No new user-controlled inputs, no auth/crypto/permissions surface.

Level of scrutiny

Moderate-to-high. The Zig change is small and follows the existing CronJob.clearAllForVM / closeAllSocketGroups pattern in the same block, and Blob.deinit() under the registry lock matches the existing single-entry revoke(). However: (1) this is a user-observable behavior change — code that previously relied on worker-created blob URLs surviving terminate() will now see them gone, which is spec-correct but worth a maintainer ack; (2) the call sits in the worker-thread shutdown sequence between closeAllSocketGroups and WebWorker__dispatchExit, a concurrency-sensitive region; (3) the flags.ts change is logically independent of the registry fix and affects how release binaries are stripped on linux-gnu — it looks correct (matches the --no-eh-frame-hdr gating in linkerFlags) but a build-system reviewer should confirm.

Other factors

Two new tests cover both terminate() and natural-exit paths and verify parent-context URLs are unaffected. CodeRabbit raised two minor points (test-isolation sweep, assertion placement) that the author resolved with sound reasoning, and both threads are marked resolved. CI build #49773 is in progress. The combination of a behavior change in a critical teardown path plus an orthogonal build-flag tweak puts this outside the "simple/mechanical" bar for auto-approval.

[claude]

LGTM — focused leak fix that mirrors the adjacent CronJob.clearAllForVM teardown pattern, with tests covering both terminate and natural exit.

Extended reasoning...

Overview

This PR fixes a process-lifetime memory leak where URL.createObjectURL() calls inside a Worker left duped Blob entries in the process-global ObjectURLRegistry after the worker exited. It adds a context_id: u32 tag to each registry entry, a scriptExecutionContextId() accessor on VirtualMachine, and a revokeEntriesForContext() sweep that is invoked from WebWorker.shutdown() right alongside the existing CronJob.clearAllForVM cleanup. Two new tests in worker_blob.test.ts cover terminate() and natural exit, and explicitly verify that parent-context URLs are left intact.

Security risks

None. No new inputs are parsed; the change only adds a per-context sweep of an existing in-process map under its existing mutex. There is no auth, crypto, or untrusted-data handling involved.

Level of scrutiny

Worker teardown is concurrency-sensitive, but the addition is a single mutex-protected map sweep placed in the same shutdown phase as CronJob.clearAllForVM (after vm.onExit(), before WebWorker__dispatchExit). revokeEntriesForContext holds the registry lock for the entire collect-then-remove pass — the same lock discipline as the pre-existing revoke(), which already calls entry.deinit() under the lock from arbitrary threads. The two-pass (collect keys, then fetchRemove) avoids mutating the HashMap during iteration. The @intCast to u32 is safe since context IDs are always positive (main = 1, workers from generateIdentifier()).

Other factors

The bug hunter found no issues. Both CodeRabbit inline comments are resolved: the bun test --isolate gap is pre-existing and out of scope (this PR makes a future fix possible by adding the context_id tag), and the test-assertion-placement suggestion was reasonably declined (asserting after await closed is equivalent given the revoke happens before dispatchExit, and avoids hang-on-failure). No CODEOWNERS cover these files. The fix mirrors WebKit's BlobURLRegistry::unregisterURLsForContext and the FileAPI spec's unloading-cleanup step.