diff --git a/src/jsc/bindings/c-bindings.cpp b/src/jsc/bindings/c-bindings.cpp index 1ead5980b9ba..a60cf1f596eb 100644 --- a/src/jsc/bindings/c-bindings.cpp +++ b/src/jsc/bindings/c-bindings.cpp @@ -373,9 +373,9 @@ static char* shared_header_buffer_get() return buffer.get(); } -// A Worker returns out of shutdown() instead of calling pthread_exit, and the process can exit -// before its TLS destructors run, so the worker releases this explicitly at teardown. The getter -// re-allocates on demand. +// Bun compiles with -fno-c++-static-destructors, so the thread_local above is never destroyed when +// a worker thread exits; the worker releases it explicitly at teardown (WebWorker::shutdown). The +// getter re-allocates on demand. extern "C" void Bun__freeSharedHeaderBufferForThreadExit() { shared_header_buffer_slot().reset(); diff --git a/src/jsc/bindings/webcore/EventNames.cpp b/src/jsc/bindings/webcore/EventNames.cpp index c97850e07e5f..ed3cc3dbdeb5 100644 --- a/src/jsc/bindings/webcore/EventNames.cpp +++ b/src/jsc/bindings/webcore/EventNames.cpp @@ -43,6 +43,15 @@ const EventNames& eventNames() return *eventNames_; } +// Bun compiles with -fno-c++-static-destructors, so eventNames_ is never destroyed when its thread +// exits. A worker thread frees its table on the way out (WebWorker::shutdown), while the thread's +// AtomStringTable that holds these atoms is still alive; WebCore does the same in +// ThreadGlobalData::destroy(). +extern "C" void Bun__destroyEventNamesForThreadExit() +{ + eventNames_.reset(); +} + enum class DOMEventName : uint8_t { rename = 0, change = 1, diff --git a/src/jsc/web_worker.rs b/src/jsc/web_worker.rs index ce6fca61493a..fc1594e5b1f7 100644 --- a/src/jsc/web_worker.rs +++ b/src/jsc/web_worker.rs @@ -146,6 +146,7 @@ unsafe extern "C" { message: &mut BunString, err: JSValue, ); + safe fn Bun__destroyEventNamesForThreadExit(); safe fn Bun__freeSharedHeaderBufferForThreadExit(); // Raw FFI (no RAII guard) so `thread_main` can take the API lock and abandon // it with the VM — see the note there. @@ -1047,9 +1048,11 @@ impl WebWorker { // gone so its raw `transpiler.env` borrow is dead. drop(unsafe { bun_core::heap::take(env_loader) }); } - // This thread's C++ thread_local destructors are not guaranteed to run - // before the process exits, so free the HPACK scratch buffer that any - // http2 session on this thread allocated. + // C++ is built with -fno-c++-static-destructors, so nothing a C++ + // thread_local owns is freed when this thread returns: release the + // per-thread state script on this thread populated (the event-name + // atoms, any http2 session's HPACK scratch buffer) by hand. + Bun__destroyEventNamesForThreadExit(); Bun__freeSharedHeaderBufferForThreadExit(); drop(arena.take()); log!( diff --git a/test/js/node/worker_threads/worker-shutdown-post-leak.test.ts b/test/js/node/worker_threads/worker-shutdown-post-leak.test.ts index b6b03722c6be..1d37459d0553 100644 --- a/test/js/node/worker_threads/worker-shutdown-post-leak.test.ts +++ b/test/js/node/worker_threads/worker-shutdown-post-leak.test.ts @@ -1,7 +1,12 @@ -import { expect, test } from "bun:test"; -import { bunEnv, bunExe, isASAN, isWindows } from "harness"; +import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, isASAN, isWindows, tempDir } from "harness"; import { join } from "path"; +// Every test here starts a worker VM under debug+ASAN and then runs LSan over +// the exiting process, which already takes a few seconds on a loaded machine; +// symbolizing a report against the debug binary takes tens of seconds more. +const LEAK_TEST_TIMEOUT = 90_000; + // A worker's shutdown used to drain its concurrent queue and only then mark // the context terminating. A cross-thread postTaskTo landing in between (the // parent's stdio-backpressure ack, any MessagePort scheduleDrain) was enqueued @@ -46,4 +51,93 @@ test.skipIf(!isASAN || isWindows)( const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect({ stdout, stderr, exitCode }).toEqual({ stdout: "", stderr: "", exitCode: 0 }); }, + LEAK_TEST_TIMEOUT, ); + +// WebCore::eventNames() keeps its table of event-name atoms in a C++ +// thread_local, and Bun compiles with -fno-c++-static-destructors, so the table +// was never freed when a worker thread exited. Every worker allocates one the +// moment it comes online (WorkerMessagingProxy::workerGlobalScopeStarted, right +// after its entry script has run), even a worker whose script is empty, so the +// cases below only differ in how the thread ends; a worker that exits or +// registers listeners itself does so from a callback, once it is online. +// Malloc=1 makes WTF's fastMalloc use the system allocator, which is what lets +// LSan see the table at all. +describe.concurrent("a worker thread frees its event name table when it exits", () => { + const lsanEnv = { + ...bunEnv, + BUN_DESTRUCT_VM_ON_EXIT: "1", + Malloc: "1", + ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "detect_leaks=1"].filter(Boolean).join(":"), + LSAN_OPTIONS: `print_suppressions=0:suppressions=${join(import.meta.dirname, "../../../leaksan.supp")}`, + }; + + test.skipIf(!isASAN || isWindows).each([ + { + route: "worker_threads Worker whose event loop drains", + files: { + "main.mjs": ` + import { Worker } from "node:worker_threads"; + new Worker("", { eval: true }).on("exit", code => console.log("exit", code)); + `, + }, + stdout: "exit 0\n", + }, + { + route: "worker_threads Worker that calls process.exit()", + files: { + "main.mjs": ` + import { Worker } from "node:worker_threads"; + new Worker("setImmediate(() => process.exit(7));", { eval: true }).on("exit", code => console.log("exit", code)); + `, + }, + stdout: "exit 7\n", + }, + { + route: "worker_threads Worker terminated by its parent while it listens on parentPort", + files: { + "main.mjs": ` + import { Worker } from "node:worker_threads"; + const worker = new Worker( + \` + const { parentPort } = require("node:worker_threads"); + setImmediate(() => { + parentPort.on("message", () => {}); + parentPort.postMessage("listening"); + }); + \`, + { eval: true }, + ); + worker.on("message", () => worker.terminate()); + worker.on("exit", () => console.log("terminated")); + `, + }, + stdout: "terminated\n", + }, + { + route: "Web Worker whose event loop drains", + files: { + "main.mjs": ` + new Worker(new URL("./worker.js", import.meta.url)).addEventListener("close", () => console.log("close")); + `, + "worker.js": "", + }, + stdout: "close\n", + }, + ])( + "$route", + async ({ files, stdout: expectedStdout }) => { + using dir = tempDir("worker-event-names", files); + await using proc = Bun.spawn({ + cmd: [bunExe(), "main.mjs"], + cwd: String(dir), + env: lsanEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ stdout: expectedStdout, stderr: "", exitCode: 0 }); + }, + LEAK_TEST_TIMEOUT, + ); +});