From 186d8d488f9a2bba20c529608414e83db8d8ed49 Mon Sep 17 00:00:00 2001 From: robobun Date: Wed, 15 Apr 2026 01:51:29 +0000 Subject: [PATCH 1/2] Worker: fix use-after-free when terminate() is called after the worker thread exits The Zig WebWorker struct was destroyed on the worker thread inside exitAndDeinit(), but the owning C++ Worker kept a raw pointer to it in impl_. A subsequent Worker.terminate() from JS on the parent thread would call WebWorker__notifyNeedTermination(impl_) and read the freed struct (status, requested_terminate, vm, parent_poll_ref), tripping ASAN with use-after-poison. Tie the struct's lifetime to the C++ Worker instead: deinit() now only releases owned resources (specifier, preloads, parent poll ref) and the allocation itself is freed from ~Worker() via a new WebWorker__destroy export. The worker thread now runs deinit() before WebWorker__dispatchExit drops the Zig-held ref on the C++ Worker so the struct is never touched after it can be freed. Worker::terminate() also short-circuits once any termination flag is set, avoiding a redundant cross-thread poke. --- src/bun.js/bindings/webcore/Worker.cpp | 8 +++- src/bun.js/web_worker.zig | 16 ++++++- .../worker-terminate-after-exit.test.ts | 47 +++++++++++++++++++ 3 files changed, 68 insertions(+), 3 deletions(-) create mode 100644 test/js/web/workers/worker-terminate-after-exit.test.ts diff --git a/src/bun.js/bindings/webcore/Worker.cpp b/src/bun.js/bindings/webcore/Worker.cpp index 8db7b462b33a..988eb44b3726 100644 --- a/src/bun.js/bindings/webcore/Worker.cpp +++ b/src/bun.js/bindings/webcore/Worker.cpp @@ -71,6 +71,8 @@ WTF_MAKE_TZONE_ALLOCATED_IMPL(Worker); extern "C" void WebWorker__notifyNeedTermination( void* worker); +extern "C" void WebWorker__destroy( + void* worker); static Lock allWorkersLock; static HashMap& allWorkers() WTF_REQUIRES_LOCK(allWorkersLock) @@ -228,6 +230,9 @@ Worker::~Worker() Locker locker { allWorkersLock }; allWorkers().remove(m_clientIdentifier); } + if (impl_) { + WebWorker__destroy(impl_); + } // m_contextProxy.workerObjectDestroyed(); } @@ -262,7 +267,8 @@ ExceptionOr Worker::postMessage(JSC::JSGlobalObject& state, JSC::JSValue m void Worker::terminate() { // m_contextProxy.terminateWorkerGlobalScope(); - m_terminationFlags.fetch_or(TerminateRequestedFlag); + if (m_terminationFlags.fetch_or(TerminateRequestedFlag)) + return; WebWorker__notifyNeedTermination(impl_); } diff --git a/src/bun.js/web_worker.zig b/src/bun.js/web_worker.zig index e55241a7e856..50fc984e703f 100644 --- a/src/bun.js/web_worker.zig +++ b/src/bun.js/web_worker.zig @@ -402,14 +402,22 @@ pub fn start( /// Deinit will clean up vm and everything. /// Early deinit may be called from caller thread, but full vm deinit will only be called within worker's thread. +/// The struct itself is freed separately by the owning C++ `Worker` via `WebWorker__destroy` so +/// that `WebWorker__notifyNeedTermination` never touches freed memory while JS still holds the +/// wrapper. fn deinit(this: *WebWorker) void { log("[{d}] deinit", .{this.execution_context_id}); this.parent_poll_ref.unrefConcurrently(this.parent); bun.default_allocator.free(this.unresolved_specifier); + this.unresolved_specifier = ""; for (this.preloads) |preload| { bun.default_allocator.free(preload); } bun.default_allocator.free(this.preloads); + this.preloads = &.{}; +} + +export fn WebWorker__destroy(this: *WebWorker) void { bun.default_allocator.destroy(this); } @@ -644,6 +652,12 @@ pub fn exitAndDeinit(this: *WebWorker) noreturn { } var arena = this.arena; + // Release owned resources before dispatching exit. `WebWorker__dispatchExit` drops the + // Zig-held ref on the C++ `Worker`; once the parent processes the close task and GC + // collects the wrapper, `~Worker()` will free this struct, so it must not be touched + // afterwards. + this.deinit(); + WebWorker__dispatchExit(globalObject, cpp_worker, exit_code); if (loop) |loop_| { loop_.internal_loop_data.jsc_vm = null; @@ -659,8 +673,6 @@ pub fn exitAndDeinit(this: *WebWorker) noreturn { bun.windows.libuv.Loop.shutdown(); } - this.deinit(); - if (vm_to_deinit) |vm| { vm.deinit(); // NOTE: deinit here isn't implemented, so freeing workers will leak the vm. } diff --git a/test/js/web/workers/worker-terminate-after-exit.test.ts b/test/js/web/workers/worker-terminate-after-exit.test.ts new file mode 100644 index 000000000000..30d459c1ab1d --- /dev/null +++ b/test/js/web/workers/worker-terminate-after-exit.test.ts @@ -0,0 +1,47 @@ +import { test, expect } from "bun:test"; +import { bunEnv, bunExe, isDebug } from "harness"; + +// The Zig `WebWorker` struct used to be destroyed on the worker thread in +// `exitAndDeinit`, while `Worker::terminate()` on the main thread could still +// call `WebWorker__notifyNeedTermination(impl_)` afterwards, reading freed +// memory. Calling `terminate()` immediately sets `requested_terminate`, so the +// worker thread takes the fast early-exit path (before its VM is created) and +// the struct was freed right after the close event was posted. Calling +// `terminate()` again from the close handler then touched the freed struct. +test( + "Worker.terminate() after the worker thread has exited does not use freed memory", + async () => { + const code = ` + for (let i = 0; i < 10; i++) { + const w = new Worker("nonexistent-entrypoint-58146"); + const { promise, resolve } = Promise.withResolvers(); + w.addEventListener("close", resolve); + w.addEventListener("error", () => {}); + w.terminate(); + await promise; + w.terminate(); + } + Bun.gc(true); + `; + const concurrency = 5; + for (let batch = 0; batch < 4; batch++) { + const runs = Array.from({ length: concurrency }, async () => { + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", code], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + return { stderr, exitCode }; + }); + for (const { stderr, exitCode } of await Promise.all(runs)) { + if (exitCode !== 0) { + expect(stderr).toBe(""); + } + expect(exitCode).toBe(0); + } + } + }, + isDebug ? 60_000 : undefined, +); From c934eb399d22439f43eb6a86cad949fd21585117 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Wed, 15 Apr 2026 01:53:43 +0000 Subject: [PATCH 2/2] [autofix.ci] apply automated fixes --- test/js/web/workers/worker-terminate-after-exit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/js/web/workers/worker-terminate-after-exit.test.ts b/test/js/web/workers/worker-terminate-after-exit.test.ts index 30d459c1ab1d..5647c1d72fd5 100644 --- a/test/js/web/workers/worker-terminate-after-exit.test.ts +++ b/test/js/web/workers/worker-terminate-after-exit.test.ts @@ -1,4 +1,4 @@ -import { test, expect } from "bun:test"; +import { expect, test } from "bun:test"; import { bunEnv, bunExe, isDebug } from "harness"; // The Zig `WebWorker` struct used to be destroyed on the worker thread in