Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1820,6 +1820,12 @@ pub struct RuntimeHooks {
/// never lazily created. Called from `WebWorker::shutdown` / `global_exit`
/// right after `close_all_socket_groups`.
pub close_dns_for_terminate: fn(),
/// `ObjectURLRegistry::singleton().sweep_owner(owner)` — drop every
/// `URL.createObjectURL` entry registered by the given script-execution
/// context. Called from `WebWorker::shutdown` so a worker that never calls
/// `revokeObjectURL` does not pin its blob payloads for the lifetime of
/// the process. The registry lives in `bun_runtime::webcore` (forward-dep).
Comment thread
robobun marked this conversation as resolved.
pub sweep_object_urls_for_owner: fn(owner: i32),
}

/// Canonical `EventLoopCtx` vtable for a `*mut VirtualMachine` owner — the JS
Expand Down
6 changes: 6 additions & 0 deletions src/jsc/web_worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1313,6 +1313,12 @@ impl WebWorker {
if let Some(rare) = vm.rare_data.as_deref_mut() {
rare.release_js_handles();
}
if let Some(hooks) = runtime_hooks() {
// Drop this worker's URL.createObjectURL entries from the
// process-global registry; nothing else revokes them once the
// worker is gone.
Comment thread
robobun marked this conversation as resolved.
(hooks.sweep_object_urls_for_owner)(vm.initial_script_execution_context_identifier);
}
exit_code = i32::from(vm.exit_handler.exit_code);
global_object = Some(vm.global);
}
Expand Down
7 changes: 7 additions & 0 deletions src/runtime/jsc_hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1487,6 +1487,7 @@ pub(crate) static __BUN_RUNTIME_HOOKS: RuntimeHooks = RuntimeHooks {
retroactively_report_discovered_tests,
cancel_all_timers,
close_dns_for_terminate,
sweep_object_urls_for_owner,
};

// ════════════════════════════════════════════════════════════════════════════
Expand Down Expand Up @@ -1636,6 +1637,12 @@ fn close_dns_for_terminate() {
}
}

/// `RuntimeHooks::sweep_object_urls_for_owner` — drop every blob-URL registry
/// entry whose recording context is `owner`.
Comment thread
robobun marked this conversation as resolved.
fn sweep_object_urls_for_owner(owner: i32) {
crate::webcore::object_url_registry::ObjectURLRegistry::singleton().sweep_owner(owner);
}

pub(crate) fn close_isolation_handles(vm: &mut VirtualMachine) {
let state = runtime_state();
if state.is_null() {
Expand Down
29 changes: 25 additions & 4 deletions src/runtime/webcore/ObjectURLRegistry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,23 @@ impl Default for ObjectURLRegistry {

pub struct Entry {
blob: Blob,
/// `VirtualMachine::initial_script_execution_context_identifier` of the
/// registering context; see [`ObjectURLRegistry::sweep_owner`].
Comment thread
robobun marked this conversation as resolved.
owner: i32,
}

// `Entry` is auto-`Send`: its sole field is `Blob`, which already asserts
// `Send + Sync` (see `webcore_types::Blob`). No `unsafe impl` needed.
// `Entry` is auto-`Send`: `Blob` already asserts `Send + Sync`
// (see `webcore_types::Blob`). No `unsafe impl` needed.
Comment thread
robobun marked this conversation as resolved.
const _: fn() = || {
fn assert_send<T: Send>() {}
assert_send::<Entry>();
};

impl Entry {
pub fn init(blob: &Blob) -> Box<Entry> {
pub fn init(blob: &Blob, owner: i32) -> Box<Entry> {
Box::new(Entry {
blob: blob.dupe_with_content_type(true),
owner,
})
}
}
Expand All @@ -54,8 +58,9 @@ impl Drop for Entry {

impl ObjectURLRegistry {
pub fn register(&self, vm: &mut VirtualMachine, blob: &Blob) -> UUID {
let owner = vm.initial_script_execution_context_identifier;
let uuid = vm.rare_data().next_uuid();
let entry = Entry::init(blob);
let entry = Entry::init(blob, owner);

self.map.lock().insert(uuid.bytes, entry);
uuid
Expand Down Expand Up @@ -97,6 +102,22 @@ impl ObjectURLRegistry {
};
self.map.lock().contains_key(&uuid.bytes)
}

/// Drop every entry registered by `owner`. Called from
/// `WebWorker::shutdown` (via `RuntimeHooks`) so blob URLs minted inside a
/// worker are released when that worker exits. Entries from the main
/// context (or other still-live workers) are left untouched.
Comment thread
robobun marked this conversation as resolved.
pub fn sweep_owner(&self, owner: i32) {
let mut map = self.map.lock();
let keys: Vec<[u8; 16]> = map
.iter()
.filter(|(_, e)| e.owner == owner)
.map(|(k, _)| *k)
.collect();
for k in keys {
let _ = map.remove(&k);
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

fn uuid_from_pathname(pathname: &[u8]) -> Option<UUID> {
Expand Down
90 changes: 90 additions & 0 deletions test/js/web/workers/worker_blob.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { expect, test } from "bun:test";
import { bunEnv, bunExe, isWindows } from "harness";

test("Worker from a Blob", async () => {
const worker = new Worker(
Expand Down Expand Up @@ -124,3 +125,92 @@ test("Worker on a revoked blob still works", async () => {

expect(revoked).toBe("revoked.");
});

// The blob-URL registry is process-global. Before the per-owner sweep, an entry
// registered inside a worker stayed in the map after the worker exited, and the
// duped blob store pinned the payload for the lifetime of the process (roughly
// 1 MB retained per MB minted per dead worker). After the sweep, a dead
// worker's URL resolves like a revoked URL and the payload is released.
test("blob URLs created inside a Worker are released when the Worker exits", async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`
const workerBody = ${JSON.stringify(`
const urls = [];
for (let i = 0; i < 4; i++) {
const u8 = new Uint8Array(1 << 20);
u8.fill(65);
urls.push(URL.createObjectURL(new Blob([u8])));
}
postMessage(urls);
`)};
const src = URL.createObjectURL(
new Blob([workerBody], { type: "application/javascript" }),
);

async function runWorker() {
const w = new Worker(src);
const urls = await new Promise((resolve, reject) => {
w.onmessage = e => resolve(e.data);
w.onerror = e => reject(new Error("worker errored: " + e.message));
});
await new Promise(r => w.addEventListener("close", r, { once: true }));
return urls;
}

// Establish the allocator high-water mark before measuring.
for (let i = 0; i < 3; i++) await runWorker();
Bun.gc(true);
Bun.gc(true);

const rss0 = process.memoryUsage.rss();
let deadUrl;
for (let i = 0; i < 30; i++) {
const urls = await runWorker();
deadUrl ??= urls[0];
}
Bun.gc(true);
Bun.gc(true);
const growthMB = (process.memoryUsage.rss() - rss0) / 2 ** 20;

let deadUrlServed = false;
try {
await fetch(deadUrl);
deadUrlServed = true;
} catch {}

// The parent-minted worker-source URL must survive the sweep.
const parentUrlStillResolves = (await (await fetch(src)).text()) === workerBody;

console.log(JSON.stringify({ growthMB, deadUrlServed, parentUrlStillResolves }));
`,
],
env: {
...bunEnv,
// Under ASAN the 1 MB payload frees go into the quarantine (default
// quarantine_size_mb=256) instead of being returned, so the RSS delta
// measures the quarantine rather than the registry. Disable it so the
// threshold is meaningful on ASAN builds.
ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "quarantine_size_mb=0"].filter(Boolean).join(":"),
},
stdout: "pipe",
stderr: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect(stderr).toBe("");
const { growthMB, deadUrlServed, parentUrlStillResolves } = JSON.parse(stdout);
if (!isWindows) {
// 30 workers * 4 MB: when leaking, growth is ~120 MB; when swept, growth
// is allocator noise (typically under 15 MB even on debug builds). RSS
// does not drop after worker threads exit on Windows (per-thread mimalloc
// arenas stay committed), so the threshold is meaningless there.
expect(growthMB).toBeLessThan(40);
}
expect(deadUrlServed).toBe(false);
expect(parentUrlStillResolves).toBe(true);
expect(exitCode).toBe(0);
}, 60_000);
Comment thread
robobun marked this conversation as resolved.
Loading