From b013585e46342a084c76ea1ca813bdd7182bb40f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 11:25:20 +0000 Subject: [PATCH 1/8] napi: settle or refuse async work queued during VM teardown instead of taking a ticket VirtualMachine::ticket() now asserts in release builds too that the VM has not finished its teardown wait (VmHandle::closed), since a ticket issued after the wait is counted by nobody and its completion would be posted onto a loop teardown is about to free. napi_async_work::schedule no longer takes a ticket once script is forbidden. Work queued while the VM is stopping (a complete callback or finalizer released by teardown) is marked cancelled and handed straight to the loop, so complete still gets napi_cancelled without a trip through the pool; work queued once the VM has closed (a finalizer running in the collection that destroys the heap) is refused with napi_cannot_run_js and stays the addon's. Also rewrites three SAFETY comments that still described the removed "pool borrow" model, and adds napi rows to worker-late-completion: the ordinary ticketed path, a re-queue from a complete callback during the wait (exactly one completion crosses the door), and a queue from an experimental addon's finalizer in the final collection (refused), built from a small C fixture against the in-tree N-API headers. --- src/jsc/VmHandle.rs | 33 ++++- src/runtime/api/JSTranspiler.rs | 8 +- src/runtime/image/Image.rs | 7 +- src/runtime/napi/napi_body.rs | 47 +++++-- src/runtime/webcore/CompressionStreamCoder.rs | 4 +- .../worker-late-completion-napi-fixture.c | 105 ++++++++++++++++ .../workers/worker-late-completion.test.ts | 118 +++++++++++++++--- 7 files changed, 290 insertions(+), 32 deletions(-) create mode 100644 test/js/web/workers/worker-late-completion-napi-fixture.c diff --git a/src/jsc/VmHandle.rs b/src/jsc/VmHandle.rs index 0e4d5cce3321..7bcf3150c436 100644 --- a/src/jsc/VmHandle.rs +++ b/src/jsc/VmHandle.rs @@ -409,6 +409,16 @@ impl VmHandle { self.0.state() == State::Open } + /// Whether the wait is over (`Closed`): no ticket can be issued any more + /// ([`VirtualMachine::ticket`] panics), a task queued on the VM's loops is + /// only released, never run, and its JSC VM is being (or has been) + /// destroyed. Any thread; meaningful on the JS thread, where the only code + /// still running this late is the final collection's finalizers. + #[inline] + pub fn closed(&self) -> bool { + self.0.state() == State::Closed + } + pub(crate) fn tickets_outstanding(&self) -> u32 { self.0.tickets.load(Ordering::SeqCst) } @@ -503,19 +513,34 @@ impl VmHandle { impl VirtualMachine { /// JS thread: a ticket for work about to leave this thread — this VM, and /// the loop it is currently ticking. Hold it in the in-flight operation - /// and drop it after the completion is posted. Infallible until the wait - /// has finished (after which nothing on this thread starts off-thread work). + /// and drop it after the completion is posted. Infallible until the VM is + /// [`closed`](Self::closed). + /// + /// Panics (in release builds too) once it is: nothing waits for a ticket + /// issued after the wait, so its completion would land on a loop that + /// teardown is about to free. The code that still runs on this thread by + /// then (the final collection's finalizers) has to check `closed()` and + /// refuse the work instead, as `napi_async_work::schedule` does; the panic + /// names the call site that did not. #[track_caller] #[inline] pub fn ticket(&self) -> Ticket { let h = self.handle_ref(); h.assert_js_thread(); - debug_assert!( - h.0.state() != State::Closed, + assert!( + !h.closed(), "off-thread work started after the VM finished draining" ); Ticket::issue(&h.0, self.current_loop_kind()) } + + /// [`VmHandle::closed`] for this VM. Once true, [`ticket`](Self::ticket) + /// panics and a task queued on the VM's loop is only released: nothing + /// native code on this thread starts for the VM will run any more. + #[inline] + pub fn closed(&self) -> bool { + self.handle_ref().closed() + } } // ── Test suite only: deterministic late completions ─────────────────────── diff --git a/src/runtime/api/JSTranspiler.rs b/src/runtime/api/JSTranspiler.rs index d99b774bd830..d2d8983320ba 100644 --- a/src/runtime/api/JSTranspiler.rs +++ b/src/runtime/api/JSTranspiler.rs @@ -651,7 +651,8 @@ impl Config { // This is going to be hard to not leak /// `transpiler.transform()` off the JS thread. The parse/print state points /// into the owning `JSTranspiler`'s config (its `Transpiler` is bit-copied), -/// which the job's Js side keeps alive and the pool borrow keeps valid. +/// which the `Strong` on the job's Js side keeps alive until the job is back +/// on the JS thread; `run` reads it while the job's ticket keeps the VM alive. pub(crate) struct TransformTask { pub input_code: bun_jsc::ThreadSafe, pub output_code: BunString, @@ -663,8 +664,9 @@ pub(crate) struct TransformTask { pub loader: Loader, pub replace_exports: bun_ast::runtime::ReplaceableExportMap, } -// SAFETY: see the type doc — VM-owned config is read only under the pool -// borrow; everything else is owned. +// SAFETY: see the type doc — the wrapper's config is read only in `run`, under +// the job's ticket (`tsconfig` is a `JsPtr` for that reason); everything else +// is owned. unsafe impl Send for TransformTask {} #[derive(bun_jsc::JsAffine)] diff --git a/src/runtime/image/Image.rs b/src/runtime/image/Image.rs index 6a90ee95521e..cd10df73b984 100644 --- a/src/runtime/image/Image.rs +++ b/src/runtime/image/Image.rs @@ -1398,8 +1398,11 @@ pub struct PipelineTask { auto_orient: bool, result: TaskResult, } -// SAFETY: `input` borrows bytes that are pinned (`Pin`) or owned by the Image -// the job's Js side keeps alive; read only under the pool borrow. The rest is owned. +// SAFETY: `input` borrows bytes that are pinned (`Pin`) or, like its path, +// owned by the Image (`PendingTask`); both holders live on the job's Js side, +// which is dropped on the JS thread only after the pool has posted this task +// back, and the pool reads through the borrows in `run`, while the job's +// ticket keeps the VM alive. The rest is owned. unsafe impl Send for PipelineTask {} /// The JS-thread half of a scheduled `PipelineTask`. diff --git a/src/runtime/napi/napi_body.rs b/src/runtime/napi/napi_body.rs index f25df298ddf8..c5ef51a30b8f 100644 --- a/src/runtime/napi/napi_body.rs +++ b/src/runtime/napi/napi_body.rs @@ -43,9 +43,10 @@ impl JSValueNapiExt for JSValue { // `Taskable` impls for the napi heap tasks dispatched through the JS event loop. impl Taskable for napi_async_work { const TAG: TaskTag = task_tag::NapiAsyncWork; - /// Work the pool handed back during teardown: its `complete` callback is - /// how the addon learns the outcome and frees the work (Node calls it from - /// environment cleanup too); script it tries to run is refused at the boundary. + /// Work handed back during teardown (by the pool, or by `schedule` itself + /// once script was forbidden): its `complete` callback is how the addon + /// learns the outcome and frees the work (Node calls it from environment + /// cleanup too); script it tries to run is refused at the boundary. unsafe fn release_unrun(this: *mut Self) { let vm = VirtualMachine::get().as_mut(); let global = vm.global(); @@ -143,6 +144,12 @@ impl NapiEnv { Self::set_last_error(Some(self), NapiStatus::pending_exception) } + /// Node's status for a call the environment can no longer serve because it + /// is shutting down. + pub(crate) fn cannot_run_js(&self) -> napi_status { + Self::set_last_error(Some(self), NapiStatus::cannot_run_js) + } + /// Checks both `env->m_pendingException` (set by `napi_throw*`) and the JSC /// VM exception slot. This is the gate Node.js's `NAPI_PREAMBLE` enforces. pub(crate) fn has_pending_exception(&self) -> bool { @@ -1851,17 +1858,41 @@ impl napi_async_work { drop(unsafe { bun_core::heap::take(this) }); } - pub(crate) fn schedule(&mut self) { + /// Starts the work, or returns `false` without having touched it: the VM + /// has closed and the work stays the addon's to delete. + pub(crate) fn schedule(&mut self) -> bool { if self.scheduled { - return; + return true; + } + let vm = VirtualMachine::get(); + if vm.closed() { + // Queued by a finalizer of the collection that is destroying the + // heap: the wait for off-thread work is over (a ticket would be + // waited for by nobody; `vm.ticket()` panics), and the queue would + // release the work on the spot, running `complete` in the middle + // of that collection. Nothing can run it any more. + return false; } self.scheduled = true; + if !vm.script_allowed() { + // Queued while the VM is stopping, e.g. by a `complete` or + // finalizer that teardown is releasing. The pool would only hand + // it back cancelled; hand it straight to the queue instead, with + // no ticket, and `complete` gets `napi_cancelled` from the next + // tick or from teardown's release, as it would have from the pool. + // A closed queue releases on the spot, so `complete` (which may + // delete the work) can run inside this call: last use of `self`. + let _ = self.cancel(); + vm.event_loop_mut().enqueue_task(Task::init(self)); + return true; + } self.poll_ref.ref_(bun_io::js_vm_ctx()); // The work object belongs to the addon and `execute` receives this // env, so the VM waits for it (Node likewise settles its threadpool // requests before an environment is freed). - self.ticket = Some(self.global.bun_vm().ticket()); + self.ticket = Some(vm.ticket()); WorkPool::schedule(&raw mut self.task); + true } pub(crate) unsafe fn run_from_thread_pool(task: *mut WorkPoolTask) { @@ -2227,7 +2258,9 @@ extern "C" fn napi_queue_async_work(env_: napi_env, work_: *mut napi_async_work) return env.invalid_arg(); }; debug_assert!(core::ptr::eq(env.to_js(), work.global.as_ptr())); - work.schedule(); + if !work.schedule() { + return env.cannot_run_js(); + } env.ok() } diff --git a/src/runtime/webcore/CompressionStreamCoder.rs b/src/runtime/webcore/CompressionStreamCoder.rs index 2115bcc30545..69cd34a49726 100644 --- a/src/runtime/webcore/CompressionStreamCoder.rs +++ b/src/runtime/webcore/CompressionStreamCoder.rs @@ -579,7 +579,9 @@ pub(crate) enum AsyncInput { Owned(Vec), } // SAFETY: `Pinned.ptr` is a backing store pinned + protected by the paired -// `PinnedChunk` for as long as the job lives; read only under the pool borrow. +// `PinnedChunk`, which lives on the job's Js side and so is dropped on the JS +// thread only after the pool has posted the job back; the pool reads the bytes +// in `run`, while the job's ticket keeps the VM alive. unsafe impl Send for AsyncInput {} /// The pin + GC protection on a chunk whose bytes went to the pool; released diff --git a/test/js/web/workers/worker-late-completion-napi-fixture.c b/test/js/web/workers/worker-late-completion-napi-fixture.c new file mode 100644 index 000000000000..6b3f3171fb1e --- /dev/null +++ b/test/js/web/workers/worker-late-completion-napi-fixture.c @@ -0,0 +1,105 @@ +// The napi rows of worker-late-completion.test.ts. Built against Bun's own +// N-API headers as a NAPI_VERSION_EXPERIMENTAL addon, so its finalizers run +// inline during garbage collection (including the collection that destroys +// the worker's heap) and only the basic-env subset of the API is allowed there; +// napi_queue_async_work is in that subset. Every entry point starts one +// napi_async_work at a different point in the worker's life, and every work +// reports the status its `complete` callback is eventually handed. +#define NAPI_EXPERIMENTAL +#include +#include + +typedef struct Work { + const char *name; + napi_async_work work; + // Queued by this work's `complete`, i.e. while the worker is already + // tearing down (queueFromComplete). + struct Work *then; +} Work; + +static Work first = {"first", NULL, NULL}; +static Work second = {"second", NULL, NULL}; +static Work late = {"late", NULL, NULL}; + +static void report(const char *what, const Work *w, napi_status status) { + const char *name = status == napi_ok ? "ok" + : status == napi_cancelled ? "cancelled" + : status == napi_cannot_run_js ? "cannot run js" + : "unexpected status"; + printf("%s %s: %s\n", what, w->name, name); + fflush(stdout); +} + +static void execute(napi_env env, void *data) { + (void)env; + (void)data; +} + +static void complete(napi_env env, napi_status status, void *data) { + Work *w = data; + report("complete", w, status); + if (w->then != NULL) { + report("queued from complete", w->then, + napi_queue_async_work(env, w->then->work)); + } + napi_delete_async_work(env, w->work); +} + +static void create(napi_env env, Work *w) { + napi_value name; + napi_create_string_utf8(env, w->name, NAPI_AUTO_LENGTH, &name); + napi_create_async_work(env, NULL, name, execute, complete, w, &w->work); +} + +// The ordinary path: queued while the worker runs. The completion comes back +// from the pool during the teardown's wait and is released there. +static napi_value queue(napi_env env, napi_callback_info info) { + (void)info; + create(env, &first); + report("queued", &first, napi_queue_async_work(env, first.work)); + return NULL; +} + +// `first` completes during the wait; its `complete` then queues `second`. +static napi_value queue_from_complete(napi_env env, napi_callback_info info) { + create(env, &second); + first.then = &second; + return queue(env, info); +} + +// Runs during the collection that destroys the heap, after the wait is over: +// the worker keeps the external reachable until it exits. The work itself is +// created up front because napi_create_async_work is not a basic-env API. +// Refused work is still the addon's; deleting it here (a plain free in Bun, +// though the API is not declared basic, hence the cast) keeps the leak +// checker quiet. +static void finalize(node_api_basic_env env, void *data, void *hint) { + (void)hint; + Work *w = data; + napi_status status = napi_queue_async_work(env, w->work); + report("queued from finalizer", w, status); + if (status != napi_ok) { + napi_delete_async_work((napi_env)env, w->work); + } +} + +static napi_value queue_from_finalizer(napi_env env, napi_callback_info info) { + (void)info; + create(env, &late); + napi_value external; + napi_create_external(env, &late, finalize, NULL, &external); + return external; +} + +NAPI_MODULE_INIT() { + napi_property_descriptor props[] = { + {"queue", NULL, queue, NULL, NULL, NULL, napi_default, NULL}, + {"queueFromComplete", NULL, queue_from_complete, NULL, NULL, NULL, + napi_default, NULL}, + {"queueFromFinalizer", NULL, queue_from_finalizer, NULL, NULL, NULL, + napi_default, NULL}, + }; + napi_define_properties(env, exports, sizeof(props) / sizeof(props[0]), + props); + return exports; +} diff --git a/test/js/web/workers/worker-late-completion.test.ts b/test/js/web/workers/worker-late-completion.test.ts index 7d27ea293c9a..acaa27fbac72 100644 --- a/test/js/web/workers/worker-late-completion.test.ts +++ b/test/js/web/workers/worker-late-completion.test.ts @@ -14,10 +14,14 @@ // line appeared (the work really was on another thread, really came back // during teardown, and — for ticketed work — was taken through the door at // the expected site) and the process exited cleanly; on the ASAN build the -// release paths are also checked for use-after-free and leaks. Builds with -// debug assertions only (debug, ASAN): the gate does not exist in release. +// release paths are also checked for use-after-free and leaks. The napi rows +// also cover the opposite obligation: work queued once the teardown is under +// way (from a completion the wait releases, or from a finalizer in the +// collection that destroys the heap) is settled or refused on the worker's own +// thread rather than given a ticket nothing would wait for. Builds with debug +// assertions only (debug, ASAN): the gate does not exist in release. import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, isAndroid, isASAN, isDebug, isLinux, isWindows, tempDir } from "harness"; +import { bunEnv, bunExe, isAndroid, isASAN, isDebug, isLinux, isMacOS, isWindows, tempDir } from "harness"; import fs from "node:fs"; import path from "node:path"; @@ -31,15 +35,30 @@ type Row = { parent?: string; // Runs in the parent when the worker has exited (release what the prelude holds). onExit?: string; + // A C source built into `/addon.node` (against Bun's own N-API headers) before the worker starts. + addon?: string; + // What the row's native fixture must have printed, in any order. + stdout?: string[]; env?: Record; files?: Record; skip?: boolean; } & (Ticketed | Weak); // Ticketed work: substring of the site (file) the ticket was taken at, as -// logged by "[vm] late completion from :". -type Ticketed = { ticket: string; weak?: never }; +// logged by "[vm] late completion from :". `lateCompletions` pins +// how many completions came back through the door in total; 0 for a row whose +// work is queued so late that it must be settled or refused on the worker's +// own thread instead of taking a ticket. +type Ticketed = ({ ticket: string; lateCompletions?: number } | { ticket?: never; lateCompletions: number }) & { + weak?: never; +}; // Weak posters: the task tag logged by "[vm] late post: (...)". -type Weak = { weak: string; ticket?: never }; +type Weak = { weak: string; ticket?: never; lateCompletions?: never }; + +// The napi rows need a C compiler; Windows has none on PATH in CI (the napi +// suite proper goes through node-gyp for that), so they skip there. +const cc = isWindows ? null : process.env.CC || Bun.which("cc") || Bun.which("gcc") || Bun.which("clang"); +const napiFixture = path.join(import.meta.dir, "worker-late-completion-napi-fixture.c"); +const napiAddon = `require(require("node:path").join(workerData.dir, "addon.node"))`; const ROWS: Row[] = [ // ── thread pool: bun_jsc::Job ──────────────────────────────────────────── @@ -192,6 +211,45 @@ const ROWS: Row[] = [ ticket: "js_bundle_completion_task.rs", files: { "entry.ts": `export default 1;\n` }, }, + // ── thread pool: napi_async_work (worker-late-completion-napi-fixture.c) ─ + { + // Queued while the worker runs: `execute` runs on the pool under a ticket + // and the wait releases the completion, which for napi means running the + // addon's `complete` (that is how it frees the work). + name: "napi_queue_async_work", + addon: napiFixture, + worker: `${napiAddon}.queue();`, + ticket: "napi_body.rs", + stdout: ["queued first: ok", "complete first: ok"], + skip: !cc, + }, + { + // `first`'s complete runs from the wait's release, with script already + // forbidden, and queues `second`. Work queued that late is handed back on + // this thread (complete with napi_cancelled) without going near the pool, + // so only `first` ever crossed the door. + name: "napi_queue_async_work from a complete callback during the wait", + addon: napiFixture, + worker: `${napiAddon}.queueFromComplete();`, + ticket: "napi_body.rs", + lateCompletions: 1, + stdout: ["queued first: ok", "complete first: ok", "queued from complete second: ok", "complete second: cancelled"], + skip: !cc, + }, + { + // The addon is NAPI_VERSION_EXPERIMENTAL, so the external's finalizer runs + // inline in the collection that destroys the worker's heap: after the wait, + // when a ticket would be waited for by nobody and nothing is left to run + // `complete` either, so queueing is refused (napi_cannot_run_js) and the + // addon keeps the work. The external is kept reachable so that collection + // is the one that finalizes it. + name: "napi_queue_async_work from a finalizer in the final collection", + addon: napiFixture, + worker: `globalThis.keep = ${napiAddon}.queueFromFinalizer();`, + lateCompletions: 0, + stdout: ["queued from finalizer late: cannot run js"], + skip: !cc, + }, // ── weak posters (no ticket): delivered while draining, or refused ─────── { name: "child exit reported by the waiter thread", @@ -242,10 +300,34 @@ function host(row: Row, dir: string) { `; } +// The napi_* symbols stay undefined in the addon and resolve against the bun +// that dlopens it (as node-gyp builds do); the headers are Bun's own, as in the +// in-tree header test of test/napi/napi-value-ffi.test.ts. +async function buildAddon(source: string, dir: string) { + await using proc = Bun.spawn({ + cmd: [ + cc!, + "-shared", + "-fPIC", + ...(isMacOS ? ["-undefined", "dynamic_lookup"] : []), + `-I${path.resolve(import.meta.dir, "../../../../src/runtime/napi")}`, + source, + "-o", + path.join(dir, "addon.node"), + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect({ exitCode, stderr: exitCode === 0 ? "" : stderr }).toEqual({ exitCode: 0, stderr: "" }); +} + describe.skipIf(!isDebug && !isASAN)("work that comes back after its worker began tearing down", () => { for (const row of ROWS) { test.concurrent.skipIf(!!row.skip)(row.name, async () => { using dir = tempDir("worker-late-completion", row.files ?? {}); + if (row.addon) await buildAddon(row.addon, String(dir)); await using proc = Bun.spawn({ cmd: [bunExe(), "-e", host(row, String(dir))], env: { ...bunEnv, ...row.env, BUN_DEBUG_TEST_WORKER_TEARDOWN_GATE: "1" }, @@ -254,16 +336,20 @@ describe.skipIf(!isDebug && !isASAN)("work that comes back after its worker bega }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); const lines = stderr.split("\n").filter(l => l.startsWith("[vm] ")); + const completions = lines.filter(l => l.startsWith("[vm] late completion from ")); + const { ticket, lateCompletions, weak } = row; const seen = - "ticket" in row && row.ticket - ? lines.some(l => l.startsWith("[vm] late completion from ") && l.includes(row.ticket)) - : lines.some(l => l.startsWith(`[vm] late post: ${row.weak} (`)); + weak !== undefined + ? lines.some(l => l.startsWith(`[vm] late post: ${weak} (`)) + : (ticket === undefined || completions.some(l => l.includes(ticket))) && + (lateCompletions === undefined || completions.length === lateCompletions); + const expected = { exitCode: 0, seen: true, printed: row.stdout && [...row.stdout].sort() }; + const actual = { exitCode, seen, printed: row.stdout && stdout.split("\n").filter(Boolean).sort() }; expect({ - exitCode, - seen, + ...actual, // On a failure, everything the host printed. - detail: exitCode === 0 && seen ? "" : stdout + stderr, - }).toEqual({ exitCode: 0, seen: true, detail: "" }); + detail: Bun.deepEquals(actual, expected) ? "" : stdout + stderr, + }).toEqual({ ...expected, detail: "" }); }); } }); @@ -329,7 +415,9 @@ describe.skipIf(isWindows)("terminate() cancels a read parked on the io loop", ( // and then complete cleanly. Here the job is a read() blocking a pool thread on // a FIFO nobody has written to yet (node:fs reads that way), so its duration is // entirely the test's to decide — no timing thresholds. Debug builds also name -// what the wait is waiting for. +// what the wait is waiting for. (Starting a debug/ASAN host and its worker plus +// the 2s that report waits on purpose runs past bun test's 5s default, hence +// the explicit ceiling.) describe.skipIf(isWindows)("terminate() waits for work that cannot be cancelled", () => { test("a pool thread parked in read() holds the worker's teardown until it returns", async () => { using dir = tempDir("worker-terminate-waits", {}); @@ -413,5 +501,5 @@ describe.skipIf(isWindows)("terminate() waits for work that cannot be cancelled" ]); expect(stdout).toBe("terminating\nexit code: 1\n"); expect(await proc.exited).toBe(0); - }); + }, 30_000); }); From 9e49eae58aea8a4bc5a0687d12e21d8646eff0ed Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 11:51:53 +0000 Subject: [PATCH 2/8] test: install the napi fixture's exports without napi_define_properties; do not pipe cc's stdout The ASAN lane runs worker-late-completion with BUN_JSC_validateExceptionChecks, and napi_define_properties' method path trips it (NapiClass::finishCreation's throw scope is never checked before defineOwnProperty), which aborted every napi row there. napi_create_function + napi_set_named_property are clean under it, as is the rest of what the rows exercise. --- .../worker-late-completion-napi-fixture.c | 23 +++++++++++-------- .../workers/worker-late-completion.test.ts | 2 +- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/test/js/web/workers/worker-late-completion-napi-fixture.c b/test/js/web/workers/worker-late-completion-napi-fixture.c index 6b3f3171fb1e..1e246f68b61d 100644 --- a/test/js/web/workers/worker-late-completion-napi-fixture.c +++ b/test/js/web/workers/worker-late-completion-napi-fixture.c @@ -91,15 +91,20 @@ static napi_value queue_from_finalizer(napi_env env, napi_callback_info info) { return external; } +// One napi_create_function + napi_set_named_property per export rather than +// napi_define_properties: the ASAN lane runs this file with +// BUN_JSC_validateExceptionChecks=1, which napi_define_properties' method +// path does not pass yet. +static void export_function(napi_env env, napi_value exports, const char *name, + napi_callback callback) { + napi_value function; + napi_create_function(env, name, NAPI_AUTO_LENGTH, callback, NULL, &function); + napi_set_named_property(env, exports, name, function); +} + NAPI_MODULE_INIT() { - napi_property_descriptor props[] = { - {"queue", NULL, queue, NULL, NULL, NULL, napi_default, NULL}, - {"queueFromComplete", NULL, queue_from_complete, NULL, NULL, NULL, - napi_default, NULL}, - {"queueFromFinalizer", NULL, queue_from_finalizer, NULL, NULL, NULL, - napi_default, NULL}, - }; - napi_define_properties(env, exports, sizeof(props) / sizeof(props[0]), - props); + export_function(env, exports, "queue", queue); + export_function(env, exports, "queueFromComplete", queue_from_complete); + export_function(env, exports, "queueFromFinalizer", queue_from_finalizer); return exports; } diff --git a/test/js/web/workers/worker-late-completion.test.ts b/test/js/web/workers/worker-late-completion.test.ts index acaa27fbac72..42081d5be206 100644 --- a/test/js/web/workers/worker-late-completion.test.ts +++ b/test/js/web/workers/worker-late-completion.test.ts @@ -316,7 +316,7 @@ async function buildAddon(source: string, dir: string) { path.join(dir, "addon.node"), ], env: bunEnv, - stdout: "pipe", + stdout: "ignore", stderr: "pipe", }); const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); From 35e34de5f7898eccdba2d9a59524549de60c5a0d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 11:58:25 +0000 Subject: [PATCH 3/8] Tighten the comments added for the teardown checks --- src/jsc/VmHandle.rs | 26 +++++++------------ src/runtime/api/JSTranspiler.rs | 5 ++-- src/runtime/image/Image.rs | 9 +++---- src/runtime/napi/napi_body.rs | 22 ++++++---------- src/runtime/webcore/CompressionStreamCoder.rs | 5 ++-- 5 files changed, 26 insertions(+), 41 deletions(-) diff --git a/src/jsc/VmHandle.rs b/src/jsc/VmHandle.rs index 7bcf3150c436..ee6247c0da75 100644 --- a/src/jsc/VmHandle.rs +++ b/src/jsc/VmHandle.rs @@ -409,11 +409,9 @@ impl VmHandle { self.0.state() == State::Open } - /// Whether the wait is over (`Closed`): no ticket can be issued any more - /// ([`VirtualMachine::ticket`] panics), a task queued on the VM's loops is - /// only released, never run, and its JSC VM is being (or has been) - /// destroyed. Any thread; meaningful on the JS thread, where the only code - /// still running this late is the final collection's finalizers. + /// Is the wait over? Past it nothing started for the VM runs any more: + /// [`VirtualMachine::ticket`] panics and a queued task is only released. + /// Any thread; meaningful on the JS thread (a final collection's finalizer). #[inline] pub fn closed(&self) -> bool { self.0.state() == State::Closed @@ -513,15 +511,13 @@ impl VmHandle { impl VirtualMachine { /// JS thread: a ticket for work about to leave this thread — this VM, and /// the loop it is currently ticking. Hold it in the in-flight operation - /// and drop it after the completion is posted. Infallible until the VM is - /// [`closed`](Self::closed). + /// and drop it after the completion is posted. /// - /// Panics (in release builds too) once it is: nothing waits for a ticket - /// issued after the wait, so its completion would land on a loop that - /// teardown is about to free. The code that still runs on this thread by - /// then (the final collection's finalizers) has to check `closed()` and - /// refuse the work instead, as `napi_async_work::schedule` does; the panic - /// names the call site that did not. + /// Panics, in release too, once the VM is [`closed`](Self::closed): the + /// wait is over, so nothing would wait for this ticket and its completion + /// would land on a loop teardown is freeing. Callers that can run that late + /// (finalizers) check `closed()` and refuse the work; the panic names one + /// that did not. #[track_caller] #[inline] pub fn ticket(&self) -> Ticket { @@ -534,9 +530,7 @@ impl VirtualMachine { Ticket::issue(&h.0, self.current_loop_kind()) } - /// [`VmHandle::closed`] for this VM. Once true, [`ticket`](Self::ticket) - /// panics and a task queued on the VM's loop is only released: nothing - /// native code on this thread starts for the VM will run any more. + /// [`VmHandle::closed`] for this VM. #[inline] pub fn closed(&self) -> bool { self.handle_ref().closed() diff --git a/src/runtime/api/JSTranspiler.rs b/src/runtime/api/JSTranspiler.rs index d2d8983320ba..2ffb38b895f6 100644 --- a/src/runtime/api/JSTranspiler.rs +++ b/src/runtime/api/JSTranspiler.rs @@ -664,9 +664,8 @@ pub(crate) struct TransformTask { pub loader: Loader, pub replace_exports: bun_ast::runtime::ReplaceableExportMap, } -// SAFETY: see the type doc — the wrapper's config is read only in `run`, under -// the job's ticket (`tsconfig` is a `JsPtr` for that reason); everything else -// is owned. +// SAFETY: see the type doc — the wrapper's config (behind `transpiler` and +// `tsconfig`) is read only in `run`, under the job's ticket; the rest is owned. unsafe impl Send for TransformTask {} #[derive(bun_jsc::JsAffine)] diff --git a/src/runtime/image/Image.rs b/src/runtime/image/Image.rs index cd10df73b984..95c24253b8d5 100644 --- a/src/runtime/image/Image.rs +++ b/src/runtime/image/Image.rs @@ -1398,11 +1398,10 @@ pub struct PipelineTask { auto_orient: bool, result: TaskResult, } -// SAFETY: `input` borrows bytes that are pinned (`Pin`) or, like its path, -// owned by the Image (`PendingTask`); both holders live on the job's Js side, -// which is dropped on the JS thread only after the pool has posted this task -// back, and the pool reads through the borrows in `run`, while the job's -// ticket keeps the VM alive. The rest is owned. +// SAFETY: `input` borrows bytes pinned by `Pin` or, like its path, owned by the +// Image `PendingTask` holds; both sit on the job's Js side, dropped there only +// after the pool has posted this task back, and the pool reads them in `run`, +// under the job's ticket. The rest is owned. unsafe impl Send for PipelineTask {} /// The JS-thread half of a scheduled `PipelineTask`. diff --git a/src/runtime/napi/napi_body.rs b/src/runtime/napi/napi_body.rs index c5ef51a30b8f..4f307172165d 100644 --- a/src/runtime/napi/napi_body.rs +++ b/src/runtime/napi/napi_body.rs @@ -1858,30 +1858,24 @@ impl napi_async_work { drop(unsafe { bun_core::heap::take(this) }); } - /// Starts the work, or returns `false` without having touched it: the VM - /// has closed and the work stays the addon's to delete. + /// `false`: refused untouched, the work stays the addon's to delete. pub(crate) fn schedule(&mut self) -> bool { if self.scheduled { return true; } let vm = VirtualMachine::get(); if vm.closed() { - // Queued by a finalizer of the collection that is destroying the - // heap: the wait for off-thread work is over (a ticket would be - // waited for by nobody; `vm.ticket()` panics), and the queue would - // release the work on the spot, running `complete` in the middle - // of that collection. Nothing can run it any more. + // A finalizer of the collection destroying the heap: `vm.ticket()` + // would panic, and the closed queue would run `complete` right + // here, mid-collection. return false; } self.scheduled = true; if !vm.script_allowed() { - // Queued while the VM is stopping, e.g. by a `complete` or - // finalizer that teardown is releasing. The pool would only hand - // it back cancelled; hand it straight to the queue instead, with - // no ticket, and `complete` gets `napi_cancelled` from the next - // tick or from teardown's release, as it would have from the pool. - // A closed queue releases on the spot, so `complete` (which may - // delete the work) can run inside this call: last use of `self`. + // The pool would only hand it back cancelled; skip the pool and the + // ticket. The queue runs or releases it, so `complete` still gets + // `napi_cancelled`, possibly inside this call (a queue teardown has + // already drained releases on the spot): last use of `self`. let _ = self.cancel(); vm.event_loop_mut().enqueue_task(Task::init(self)); return true; diff --git a/src/runtime/webcore/CompressionStreamCoder.rs b/src/runtime/webcore/CompressionStreamCoder.rs index 69cd34a49726..7f9e809cb75f 100644 --- a/src/runtime/webcore/CompressionStreamCoder.rs +++ b/src/runtime/webcore/CompressionStreamCoder.rs @@ -579,9 +579,8 @@ pub(crate) enum AsyncInput { Owned(Vec), } // SAFETY: `Pinned.ptr` is a backing store pinned + protected by the paired -// `PinnedChunk`, which lives on the job's Js side and so is dropped on the JS -// thread only after the pool has posted the job back; the pool reads the bytes -// in `run`, while the job's ticket keeps the VM alive. +// `PinnedChunk` on the job's Js side, dropped there only after the pool has +// posted the job back; the pool reads it in `run`, under the job's ticket. unsafe impl Send for AsyncInput {} /// The pin + GC protection on a chunk whose bytes went to the pool; released From 8c517233839d85f13c9e55b15818591e3908039a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 12:38:10 +0000 Subject: [PATCH 4/8] Release what a release enqueues from the same drain; schedule napi work through its pointer A napi complete callback released by teardown can queue another work. That work is now parked and released by the drain that is running instead of being released on the spot inside the complete that queued it, so a chain of such works is released link by link, as it was when each link went through the pool. Outside a drain, a closed queue still releases on arrival. Since complete can delete the work before napi_queue_async_work returns, schedule takes the work as a raw pointer and the caller forms no reference to it. The test asserts the fixture's output in order, which pins the drain order, and the fixture reports unexpected statuses numerically. --- src/jsc/event_loop.rs | 13 ++- src/runtime/napi/napi_body.rs | 79 +++++++++++-------- .../worker-late-completion-napi-fixture.c | 31 ++++++-- .../workers/worker-late-completion.test.ts | 10 ++- 4 files changed, 88 insertions(+), 45 deletions(-) diff --git a/src/jsc/event_loop.rs b/src/jsc/event_loop.rs index 6437e63f9456..70d99b7ff180 100644 --- a/src/jsc/event_loop.rs +++ b/src/jsc/event_loop.rs @@ -49,6 +49,9 @@ pub struct EventLoop { /// Set when teardown releases the queue: from then on `enqueue_task` /// releases instead of parking (nothing will tick this loop again). closed_for_tasks: bool, + /// Set while `release_queued_tasks` is draining: a task enqueued by one + /// it releases is parked for that same drain (see `enqueue_task`). + releasing_tasks: bool, /// setImmediate() gets it's own two task queues /// When you call `setImmediate` in JS, it queues to the start of the next tick @@ -119,6 +122,7 @@ impl Default for EventLoop { Self { tasks: Queue::init(), closed_for_tasks: false, + releasing_tasks: false, immediate_tasks: Vec::new(), next_immediate_tasks: Vec::new(), yield_tasks: Vec::new(), @@ -719,10 +723,14 @@ impl EventLoop { } pub fn enqueue_task(&mut self, task: Task) { - if self.closed_for_tasks { + if self.closed_for_tasks && !self.releasing_tasks { // Teardown already released the queue and this loop never ticks // again: release the task now, as `release_queued_tasks` would have // — the queue owns refusal, like `VmHandle::post` does off-thread. + // While that release is still draining, the task is parked for it + // instead, so a released task that enqueues another (a napi + // `complete` queueing more work) has it released after itself + // rather than inside itself. // SAFETY: JS thread, JSC heap alive (teardown phase B/C). unsafe { __bun_release_task_unrun(task) }; return; @@ -761,10 +769,13 @@ impl EventLoop { self.closed_for_tasks = true; self.take_concurrent_tasks(); let _ = self.promote_yield_tasks(); + self.releasing_tasks = true; + // Also reads what the releases themselves enqueue (`enqueue_task`). while let Some(task) = self.tasks.read_item() { // SAFETY: JS thread, heap alive; `task` just left the queue. unsafe { __bun_release_task_unrun(task) }; } + self.releasing_tasks = false; // Pending immediates likewise: cancelling one drops its keep-alive on // this thread's loop, so it happens now, not after the loop is gone. self.release_pending_immediates(); diff --git a/src/runtime/napi/napi_body.rs b/src/runtime/napi/napi_body.rs index 4f307172165d..1f946f8795c8 100644 --- a/src/runtime/napi/napi_body.rs +++ b/src/runtime/napi/napi_body.rs @@ -1859,34 +1859,43 @@ impl napi_async_work { } /// `false`: refused untouched, the work stays the addon's to delete. - pub(crate) fn schedule(&mut self) -> bool { - if self.scheduled { - return true; - } - let vm = VirtualMachine::get(); - if vm.closed() { - // A finalizer of the collection destroying the heap: `vm.ticket()` - // would panic, and the closed queue would run `complete` right - // here, mid-collection. - return false; - } - self.scheduled = true; - if !vm.script_allowed() { - // The pool would only hand it back cancelled; skip the pool and the - // ticket. The queue runs or releases it, so `complete` still gets - // `napi_cancelled`, possibly inside this call (a queue teardown has - // already drained releases on the spot): last use of `self`. - let _ = self.cancel(); - vm.event_loop_mut().enqueue_task(Task::init(self)); - return true; + /// + /// # Safety + /// `this` is a live work on its JS thread. It may be gone when this + /// returns `true`: its `complete` (which addons delete the work from) can + /// run inside the call. + pub(crate) unsafe fn schedule(this: *mut Self) -> bool { + // SAFETY: fn contract. The work is only ever reached through `this`, + // one statement at a time, so no reference to it is live across + // `enqueue_task`, which may free it. + unsafe { + if (*this).scheduled { + return true; + } + let vm = VirtualMachine::get(); + if vm.closed() { + // A finalizer of the collection destroying the heap: + // `vm.ticket()` would panic, and the closed queue would run + // `complete` right here, mid-collection. + return false; + } + (*this).scheduled = true; + if !vm.script_allowed() { + // The pool would only hand it back cancelled; skip the pool and + // the ticket. Whether the queue runs it or teardown releases + // it, `complete` gets `napi_cancelled` as before. + let _ = (*this).cancel(); + vm.event_loop_mut().enqueue_task(Task::init(this)); + return true; + } + (*this).poll_ref.ref_(bun_io::js_vm_ctx()); + // The work object belongs to the addon and `execute` receives this + // env, so the VM waits for it (Node likewise settles its threadpool + // requests before an environment is freed). + (*this).ticket = Some(vm.ticket()); + WorkPool::schedule(&raw mut (*this).task); + true } - self.poll_ref.ref_(bun_io::js_vm_ctx()); - // The work object belongs to the addon and `execute` receives this - // env, so the VM waits for it (Node likewise settles its threadpool - // requests before an environment is freed). - self.ticket = Some(vm.ticket()); - WorkPool::schedule(&raw mut self.task); - true } pub(crate) unsafe fn run_from_thread_pool(task: *mut WorkPoolTask) { @@ -2247,13 +2256,17 @@ extern "C" fn napi_delete_async_work(env_: napi_env, work_: *mut napi_async_work extern "C" fn napi_queue_async_work(env_: napi_env, work_: *mut napi_async_work) -> napi_status { bun_output::scoped_log!(napi, "napi_queue_async_work"); let env = get_env!(env_); - // SAFETY: `work_` is null or the `napi_async_work` we allocated in `napi_create_async_work`. - let Some(work) = (unsafe { work_.as_mut() }) else { + if work_.is_null() { return env.invalid_arg(); - }; - debug_assert!(core::ptr::eq(env.to_js(), work.global.as_ptr())); - if !work.schedule() { - return env.cannot_run_js(); + } + // SAFETY: non-null `work_` is the `napi_async_work` we allocated in + // `napi_create_async_work`, live until `schedule` (which may free it: its + // `complete` can run inside) takes it; no reference to it is formed here. + unsafe { + debug_assert!(core::ptr::eq(env.to_js(), (*work_).global.as_ptr())); + if !napi_async_work::schedule(work_) { + return env.cannot_run_js(); + } } env.ok() } diff --git a/test/js/web/workers/worker-late-completion-napi-fixture.c b/test/js/web/workers/worker-late-completion-napi-fixture.c index 1e246f68b61d..267ec50a5328 100644 --- a/test/js/web/workers/worker-late-completion-napi-fixture.c +++ b/test/js/web/workers/worker-late-completion-napi-fixture.c @@ -22,11 +22,20 @@ static Work second = {"second", NULL, NULL}; static Work late = {"late", NULL, NULL}; static void report(const char *what, const Work *w, napi_status status) { - const char *name = status == napi_ok ? "ok" - : status == napi_cancelled ? "cancelled" - : status == napi_cannot_run_js ? "cannot run js" - : "unexpected status"; - printf("%s %s: %s\n", what, w->name, name); + switch (status) { + case napi_ok: + printf("%s %s: ok\n", what, w->name); + break; + case napi_cancelled: + printf("%s %s: cancelled\n", what, w->name); + break; + case napi_cannot_run_js: + printf("%s %s: cannot run js\n", what, w->name); + break; + default: + printf("%s %s: napi_status %d\n", what, w->name, (int)status); + break; + } fflush(stdout); } @@ -45,10 +54,18 @@ static void complete(napi_env env, napi_status status, void *data) { napi_delete_async_work(env, w->work); } +// A failure here shows up in the row's output as its own line. static void create(napi_env env, Work *w) { napi_value name; - napi_create_string_utf8(env, w->name, NAPI_AUTO_LENGTH, &name); - napi_create_async_work(env, NULL, name, execute, complete, w, &w->work); + napi_status status = + napi_create_string_utf8(env, w->name, NAPI_AUTO_LENGTH, &name); + if (status == napi_ok) { + status = + napi_create_async_work(env, NULL, name, execute, complete, w, &w->work); + } + if (status != napi_ok) { + report("created", w, status); + } } // The ordinary path: queued while the worker runs. The completion comes back diff --git a/test/js/web/workers/worker-late-completion.test.ts b/test/js/web/workers/worker-late-completion.test.ts index 42081d5be206..538ec3b09188 100644 --- a/test/js/web/workers/worker-late-completion.test.ts +++ b/test/js/web/workers/worker-late-completion.test.ts @@ -37,7 +37,7 @@ type Row = { onExit?: string; // A C source built into `/addon.node` (against Bun's own N-API headers) before the worker starts. addon?: string; - // What the row's native fixture must have printed, in any order. + // Exactly what the row's native fixture must have printed, in order. stdout?: string[]; env?: Record; files?: Record; @@ -227,7 +227,9 @@ const ROWS: Row[] = [ // `first`'s complete runs from the wait's release, with script already // forbidden, and queues `second`. Work queued that late is handed back on // this thread (complete with napi_cancelled) without going near the pool, - // so only `first` ever crossed the door. + // so only `first` ever crossed the door; and it is released after the + // complete that queued it returns, not inside it (the order below), so a + // chain of such works is released link by link rather than recursively. name: "napi_queue_async_work from a complete callback during the wait", addon: napiFixture, worker: `${napiAddon}.queueFromComplete();`, @@ -343,8 +345,8 @@ describe.skipIf(!isDebug && !isASAN)("work that comes back after its worker bega ? lines.some(l => l.startsWith(`[vm] late post: ${weak} (`)) : (ticket === undefined || completions.some(l => l.includes(ticket))) && (lateCompletions === undefined || completions.length === lateCompletions); - const expected = { exitCode: 0, seen: true, printed: row.stdout && [...row.stdout].sort() }; - const actual = { exitCode, seen, printed: row.stdout && stdout.split("\n").filter(Boolean).sort() }; + const expected = { exitCode: 0, seen: true, printed: row.stdout }; + const actual = { exitCode, seen, printed: row.stdout && stdout.split("\n").filter(Boolean) }; expect({ ...actual, // On a failure, everything the host printed. From 204e9ffecc08b90dd55f0a15283855a096579176 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 14:06:08 +0000 Subject: [PATCH 5/8] event loop: launder self across the teardown release loop A release re-enters the loop through the VM's pointer and, while the drain runs, pushes onto the queue the drain is reading; the extern release call receives nothing derived from the noalias receiver, so go through a laundered pointer, re-escaped after every release, as the other re-entrant loops here do. --- src/jsc/event_loop.rs | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/src/jsc/event_loop.rs b/src/jsc/event_loop.rs index 70d99b7ff180..e0bab29150a4 100644 --- a/src/jsc/event_loop.rs +++ b/src/jsc/event_loop.rs @@ -766,19 +766,35 @@ impl EventLoop { /// JSC heap alive, children joined): called on every turn of the wait, and /// once more after `Closed`. pub fn release_queued_tasks(&mut self) { - self.closed_for_tasks = true; - self.take_concurrent_tasks(); - let _ = self.promote_yield_tasks(); - self.releasing_tasks = true; - // Also reads what the releases themselves enqueue (`enqueue_task`). - while let Some(task) = self.tasks.read_item() { + // R-2 noalias mitigation — see `run_callback` above. `&mut self` is + // `noalias` and `__bun_release_task_unrun` receives nothing derived + // from it, but a release re-enters this loop through + // `vm.event_loop_mut()`: `enqueue_task` reads the two flags stored + // below and, while we are draining, pushes onto `tasks`, which the next + // `read_item` must observe. So everything here goes through a + // laundered pointer, re-escaped after every release. + let mut this: *mut Self = core::hint::black_box(core::ptr::from_mut(self)); + // SAFETY: `this` is the unique live `EventLoop`; every access through + // it is a short-lived borrow that ends before a release runs. + unsafe { + (*this).closed_for_tasks = true; + (*this).take_concurrent_tasks(); + let _ = (*this).promote_yield_tasks(); + (*this).releasing_tasks = true; + } + // SAFETY: as above. + while let Some(task) = unsafe { (*this).tasks.read_item() } { // SAFETY: JS thread, heap alive; `task` just left the queue. unsafe { __bun_release_task_unrun(task) }; + this = core::hint::black_box(this); + } + // SAFETY: as above. Pending immediates likewise: cancelling one drops + // its keep-alive on this thread's loop, so it happens now, not after + // the loop is gone. + unsafe { + (*this).releasing_tasks = false; + (*this).release_pending_immediates(); } - self.releasing_tasks = false; - // Pending immediates likewise: cancelling one drops its keep-alive on - // this thread's loop, so it happens now, not after the loop is gone. - self.release_pending_immediates(); } /// Cancel (never run) every queued ImmediateObject; each cancel drops the From 784210dbc4c0a06a4e36ef71512d1248c0427249 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 14:11:55 +0000 Subject: [PATCH 6/8] test: leave the FIFO test on the runner's timeout --- test/js/web/workers/worker-late-completion.test.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/js/web/workers/worker-late-completion.test.ts b/test/js/web/workers/worker-late-completion.test.ts index 538ec3b09188..07cb9d2b5f4b 100644 --- a/test/js/web/workers/worker-late-completion.test.ts +++ b/test/js/web/workers/worker-late-completion.test.ts @@ -417,9 +417,7 @@ describe.skipIf(isWindows)("terminate() cancels a read parked on the io loop", ( // and then complete cleanly. Here the job is a read() blocking a pool thread on // a FIFO nobody has written to yet (node:fs reads that way), so its duration is // entirely the test's to decide — no timing thresholds. Debug builds also name -// what the wait is waiting for. (Starting a debug/ASAN host and its worker plus -// the 2s that report waits on purpose runs past bun test's 5s default, hence -// the explicit ceiling.) +// what the wait is waiting for. describe.skipIf(isWindows)("terminate() waits for work that cannot be cancelled", () => { test("a pool thread parked in read() holds the worker's teardown until it returns", async () => { using dir = tempDir("worker-terminate-waits", {}); @@ -503,5 +501,5 @@ describe.skipIf(isWindows)("terminate() waits for work that cannot be cancelled" ]); expect(stdout).toBe("terminating\nexit code: 1\n"); expect(await proc.exited).toBe(0); - }, 30_000); + }); }); From 635f56079104a1e6cae5b7c383fcac3295235892 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 14:42:26 +0000 Subject: [PATCH 7/8] test: accept either status for the napi work queued before the worker stopped Whether its execute ran depends on the pool reaching it before the worker began stopping; the gate only fixes when the completion arrives. The lines whose outcome the teardown decides are still matched exactly and in order. --- .../workers/worker-late-completion.test.ts | 34 +++++++++++++------ 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/test/js/web/workers/worker-late-completion.test.ts b/test/js/web/workers/worker-late-completion.test.ts index 07cb9d2b5f4b..fcad8bb2afa7 100644 --- a/test/js/web/workers/worker-late-completion.test.ts +++ b/test/js/web/workers/worker-late-completion.test.ts @@ -37,8 +37,9 @@ type Row = { onExit?: string; // A C source built into `/addon.node` (against Bun's own N-API headers) before the worker starts. addon?: string; - // Exactly what the row's native fixture must have printed, in order. - stdout?: string[]; + // Exactly what the row's native fixture must have printed, in order; a + // RegExp where more than one outcome is correct. + stdout?: (string | RegExp)[]; env?: Record; files?: Record; skip?: boolean; @@ -59,6 +60,8 @@ type Weak = { weak: string; ticket?: never; lateCompletions?: never }; const cc = isWindows ? null : process.env.CC || Bun.which("cc") || Bun.which("gcc") || Bun.which("clang"); const napiFixture = path.join(import.meta.dir, "worker-late-completion-napi-fixture.c"); const napiAddon = `require(require("node:path").join(workerData.dir, "addon.node"))`; +// See the first napi row. +const FIRST_SETTLED = /^complete first: (ok|cancelled)$/; const ROWS: Row[] = [ // ── thread pool: bun_jsc::Job ──────────────────────────────────────────── @@ -213,14 +216,17 @@ const ROWS: Row[] = [ }, // ── thread pool: napi_async_work (worker-late-completion-napi-fixture.c) ─ { - // Queued while the worker runs: `execute` runs on the pool under a ticket + // Queued while the worker runs: the work goes to the pool under a ticket // and the wait releases the completion, which for napi means running the - // addon's `complete` (that is how it frees the work). + // addon's `complete` (that is how it frees the work). Its status depends on + // whether the pool reached the work before the worker began stopping + // (`execute` ran: ok) or after (it did not: cancelled); the gate only fixes + // when the completion arrives, so either is correct here. name: "napi_queue_async_work", addon: napiFixture, worker: `${napiAddon}.queue();`, ticket: "napi_body.rs", - stdout: ["queued first: ok", "complete first: ok"], + stdout: ["queued first: ok", FIRST_SETTLED], skip: !cc, }, { @@ -235,7 +241,7 @@ const ROWS: Row[] = [ worker: `${napiAddon}.queueFromComplete();`, ticket: "napi_body.rs", lateCompletions: 1, - stdout: ["queued first: ok", "complete first: ok", "queued from complete second: ok", "complete second: cancelled"], + stdout: ["queued first: ok", FIRST_SETTLED, "queued from complete second: ok", "complete second: cancelled"], skip: !cc, }, { @@ -345,13 +351,19 @@ describe.skipIf(!isDebug && !isASAN)("work that comes back after its worker bega ? lines.some(l => l.startsWith(`[vm] late post: ${weak} (`)) : (ticket === undefined || completions.some(l => l.includes(ticket))) && (lateCompletions === undefined || completions.length === lateCompletions); - const expected = { exitCode: 0, seen: true, printed: row.stdout }; - const actual = { exitCode, seen, printed: row.stdout && stdout.split("\n").filter(Boolean) }; + const printed = stdout.split("\n").filter(Boolean); + const printedAsExpected = + row.stdout === undefined || + (printed.length === row.stdout.length && + row.stdout.every((want, i) => (typeof want === "string" ? printed[i] === want : want.test(printed[i])))); + const ok = exitCode === 0 && seen && printedAsExpected; expect({ - ...actual, + exitCode, + seen, + printedAsExpected, // On a failure, everything the host printed. - detail: Bun.deepEquals(actual, expected) ? "" : stdout + stderr, - }).toEqual({ ...expected, detail: "" }); + detail: ok ? "" : stdout + stderr, + }).toEqual({ exitCode: 0, seen: true, printedAsExpected: true, detail: "" }); }); } }); From f0d2cd443a1257355a6b851613bf7c493fdd9ccf Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 20:45:49 +0000 Subject: [PATCH 8/8] ci: retrigger