Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
3 changes: 2 additions & 1 deletion src/jsc/VmHandle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,8 @@ impl VmHandle {

/// [`stop`](Self::stop), raise a JSC `TerminationException` in the VM at
/// its next safepoint, and wake its loop. Any thread (a parent's
/// `worker.terminate()`); no-op once the VM is closed.
/// `worker.terminate()`, the worker's own `process.exit()` / uncaught
/// error); no-op once the VM is closed.
Comment thread
robobun marked this conversation as resolved.
Outdated
pub fn request_termination(&self) {
self.stop();
if let Some(_a) = self.enter() {
Expand Down
27 changes: 7 additions & 20 deletions src/jsc/web_worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1093,13 +1093,11 @@ impl WebWorker {
// process.exit() from an exit handler does not re-arm the trap.
let vm_ptr = self.vm_ptr();
if !vm_ptr.is_null() {
// The wake is needed even on the loop's own thread: from an
// immediate this runs ahead of the turn's poll, which `spin()`
// waits out before it re-reads `requested_terminate`.
Comment thread
robobun marked this conversation as resolved.
Outdated
// SAFETY: this thread's live VM.
unsafe {
// As for a parent's terminate(): nothing more may enter script
// (Node's `Stop(env)` on the worker's own exit).
(*vm_ptr).handle_ref().stop();
(*vm_ptr).jsc_vm().notify_need_termination();
}
unsafe { (*vm_ptr).handle_ref().request_termination() };
}
}

Expand Down Expand Up @@ -1251,20 +1249,9 @@ fn on_unhandled_rejection(
// second time (a second `workerGlobalScopeDestroyed` → double deref of
// the proxy's thread-held reference).
//
// Instead, arm the JSC termination trap so any further JS halts at the
// next safepoint, and let the stack unwind normally back to `spin()`,
// whose loop observes `requested_terminate` and reaches the single
// `shutdown()` call at its bottom with no live JSC frames above it. The
// promise-rejection path in `spin()` (line ~1044) gets there even sooner:
// `uncaught_exception` returns `handled == false`, so `spin()` calls
// `return self.shutdown()` directly — same observable ordering.
// `vm.jsc_vm` is the worker's live `JSC::VM*` (we just used it via
// `global_object`); `notify_need_termination` is documented thread-safe
// (VMTraps). The gate closes with it, as for exit()/terminate(): the
// native→JS entries still reached this tick refuse rather than run into
// the pending termination.
vm.handle_ref().stop();
vm.jsc_vm().notify_need_termination();
// Instead, request the stop as `exit()` does and unwind back to `spin()`,
// which reaches the single `shutdown()` call with no live JSC frames above it.
Comment thread
robobun marked this conversation as resolved.
Outdated
vm.handle_ref().request_termination();
}

/// Resolve a worker entry-point specifier to a path the module loader can
Expand Down
45 changes: 45 additions & 0 deletions test/js/node/worker_threads/worker_threads.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2307,6 +2307,51 @@ test("closing the only ref'd port from setImmediate lets the process exit", asyn
expect(exitCode).toBe(0);
});

// A worker's own stop requests (process.exit(), an uncaught error) have to wake
// its loop the way the parent's terminate() does: made from an immediate they
// land after the turn's tick and before its poll, and the run loop only looks
// for them again once the poll returns. With a parentPort listener keeping the
// loop alive, nothing else ends that poll: the exit used to wait for the idle GC
// timer (about a second), and without it (disabled here) never happened.
describe("a worker that stops itself from an immediate exits right away", () => {
test.concurrent.each([
["process.exit()", "process.exit(7);", { errors: [], code: 7 }],
[
"process.exit() from a nextTick the immediate queued",
"process.nextTick(() => process.exit(7));",
{ errors: [], code: 7 },
],
["an uncaught exception", 'throw new Error("boom");', { errors: ["boom"], code: 1 }],
])("%s", async (_label, stop, expected) => {
const workerSrc = `const { parentPort } = require("node:worker_threads");
parentPort.on("message", () => {});
// Scheduled a little after startup so the worker's startup GC timers have
// fired by then and nothing is left that would end the poll on its own.
setTimeout(() => setImmediate(() => { parentPort.postMessage("stopping"); ${stop} }), 300);`;
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`const { Worker } = require("node:worker_threads");
const w = new Worker(${JSON.stringify(workerSrc)}, { eval: true });
const seen = { messages: [], errors: [] };
w.on("message", m => seen.messages.push(m));
w.on("error", e => seen.errors.push(e.message));
w.on("exit", code => console.log(JSON.stringify({ ...seen, code })));`,
],
env: { ...bunEnv, BUN_GC_TIMER_DISABLE: "1" },
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: JSON.stringify({ messages: ["stopping"], ...expected }) + "\n",
stderr: "",
exitCode: 0,
});
});
});

// Node's setupPortReferencing: the parent side of parentPort keeps the parent
// alive while the Worker has 'message' listeners, independently of unref().
test("an unref'ed worker with a 'message' listener still delivers to the parent", async () => {
Expand Down
Loading