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: 3 additions & 3 deletions src/jsc/bindings/c-bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
9 changes: 9 additions & 0 deletions src/jsc/bindings/webcore/EventNames.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 6 additions & 3 deletions src/jsc/web_worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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!(
Expand Down
98 changes: 96 additions & 2 deletions test/js/node/worker_threads/worker-shutdown-post-leak.test.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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,
);
});