Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
8 changes: 7 additions & 1 deletion src/jsc/event_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -996,7 +996,13 @@ impl EventLoop {
/// freshly-allocated or struct-embedded task — never null.
pub fn enqueue_task_concurrent(&self, task: core::ptr::NonNull<ConcurrentTaskItem>) {
if cfg!(debug_assertions) {
if self.vm_ref().has_terminated {
let vm = self.vm_ref();
// The main-thread VM box is process-static (never `dealloc`'d), so a
// late cross-thread push after `global_exit` → `destroy()` just
// lands in a queue that is never drained again. The assert exists
// to catch the worker case, where `has_terminated` precedes a raw
// `dealloc` and a push is UAF-adjacent.
Comment thread
robobun marked this conversation as resolved.
Outdated
if vm.has_terminated && !vm.is_main_thread() {
panic!("EventLoop.enqueueTaskConcurrent: VM has terminated");
}
}
Expand Down
86 changes: 86 additions & 0 deletions test/js/node/fs/fs-write-exit-race.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import { expect, test } from "bun:test";
import { bunEnv, bunExe, isLinux, tempDir } from "harness";

// A thread-pool `fs.write` that is still blocked in the kernel when
// `process.exit()` runs must not crash the process when it later completes.
// `BUN_DESTRUCT_VM_ON_EXIT=1` (set by the CI runner) makes `global_exit`
// call `VirtualMachine::destroy()` on the main-thread VM before `libc::exit`;
// a work-pool completion that lands after that posts to the event loop of a
// VM whose `has_terminated` is already true. The main-thread VM box is never
// freed, so the push is harmless and the debug assert was a false positive.
//
// Deterministically reproducing the race requires something to unblock the
// kernel write *after* `destroy()` has returned. The child registers a libc
// atexit handler (via bun:ffi's tinycc) that closes the FIFO reader and
// then sleeps, so the blocked `write()` returns EPIPE while the main thread
// is still inside the atexit chain.
//
// Linux-only: relies on mkfifo, blocking-pipe write semantics, and glibc's
// `__cxa_atexit`.
test.skipIf(!isLinux)(

Check warning on line 20 in test/js/node/fs/fs-write-exit-race.test.ts

View check run for this annotation

Claude / Claude Code Review

Test's __cxa_atexit trigger only reached on ASAN builds (quick_exit skips it)

The test's race trigger relies on `__cxa_atexit` running `do_close_and_sleep` after `destroy()`, but on non-ASAN Linux builds `bun_core::Global::exit()` calls `quick_exit()` (which skips `__cxa_atexit` handlers) instead of `libc_exit()` — so on `bun bd --asan=off` the write is never unblocked and the test passes vacuously even with the fix reverted. This is fine in practice (default `bun bd` on Linux and the linux-x64-asan CI lane both enable ASAN, so the test is live where it matters), but cons
Comment thread
robobun marked this conversation as resolved.
Outdated
"process.exit with a thread-pool fs.write still blocked in the kernel exits cleanly",
async () => {
using dir = tempDir("fs-write-exit-race", {
"helper.c": `
#include <unistd.h>
extern int __cxa_atexit(void (*)(void *), void *, void *);
static int g_fd = -1;
static void do_close_and_sleep(void *unused) {
(void)unused;
if (g_fd >= 0) close(g_fd);
usleep(300000);
}
void schedule_close_at_exit(int fd) {
g_fd = fd;
__cxa_atexit(do_close_and_sleep, 0, 0);
}
`,
"child.js": `
const fs = require("node:fs");
const path = require("node:path");
const cp = require("node:child_process");
const { cc, FFIType } = require("bun:ffi");

const fifo = path.join(__dirname, "pipe");
cp.spawnSync("mkfifo", [fifo]);

const rd = fs.openSync(fifo, fs.constants.O_RDONLY | fs.constants.O_NONBLOCK);
const wd = fs.openSync(fifo, fs.constants.O_WRONLY);

const { symbols } = cc({
source: path.join(__dirname, "helper.c"),
symbols: {
schedule_close_at_exit: { args: [FFIType.i32], returns: FFIType.void },
},
});
symbols.schedule_close_at_exit(rd);
Comment thread
robobun marked this conversation as resolved.

// Larger than any pipe capacity so write() blocks in the kernel until
// the reader fd closes.
fs.write(wd, Buffer.alloc(1 << 21, 0x41), () => {});

setTimeout(() => {
process.stdout.write("alive\\n");
process.exit(0);
}, 50);

Check warning on line 65 in test/js/node/fs/fs-write-exit-race.test.ts

View check run for this annotation

Claude / Claude Code Review

Uncommented 50ms setTimeout — observable signal exists

The child fixture's `setTimeout(..., 50)` before `process.exit(0)` needs a comment naming why no observable signal exists (per REVIEW.md's ≥50ms rule) — but one *does* exist: since `rd` is opened `O_NONBLOCK`, a bounded poll of `fs.readSync(rd, Buffer.alloc(1))` (catching EAGAIN) until it returns >0 proves the work-pool thread has entered `write()` and is blocked. Consider replacing the fixed 50ms with that poll, or adding the comment; the 300ms atexit `usleep` makes actual flakiness unlikely, s
Comment thread
robobun marked this conversation as resolved.
Outdated
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "child.js"],
cwd: String(dir),
env: {
...bunEnv,
BUN_DESTRUCT_VM_ON_EXIT: "1",
ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "detect_leaks=0"].filter(Boolean).join(":"),
},
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: "alive\n", stderr: "", exitCode: 0 });
},
// On an unfixed debug build the panic hook symbolizes the backtrace through
// llvm-symbolizer (~5s) before the process terminates.
15_000,
);
Loading