Skip to content
Closed
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
30 changes: 27 additions & 3 deletions src/runtime/node/node_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,20 @@ mod _impl {

// ───────────────────────────── execArgv ─────────────────────────────

/// `WebWorker::argv`/`exec_argv` borrow `StringImpl*` from the parent
/// thread's `WorkerOptions` vector; those impls are not thread-safe, so
/// copy the bytes into a worker-local impl before handing them to JSC.
Comment thread
robobun marked this conversation as resolved.
fn clone_parent_worker_option_string(wtf: bun_core::WTFStringImpl) -> BunString {
// SAFETY: each entry borrows live storage in the parent `WorkerOptions`
// for this worker's lifetime (see `WebWorker::argv`/`exec_argv`).
let impl_ = unsafe { &*wtf };
if impl_.is_8bit() {
BunString::clone_latin1(impl_.latin1_slice())
} else {
BunString::clone_utf16(impl_.utf16_slice())
}
}

// The C++ caller
// (headers.h) declares `EncodedJSValue Bun__Process__createExecArgv(JSGlobalObject*)`,
// not a `JSHostFunctionType`. Hand-roll the shim instead of `#[bun_jsc::host_fn]`.
Expand All @@ -199,7 +213,10 @@ mod _impl {
// was explicitly overridden for the worker?
if let Some(exec_argv) = worker.exec_argv() {
return JSValue::create_array_from_iter(global_object, exec_argv.iter(), |&wtf| {
BunString::init(wtf).to_js(global_object)
let s = clone_parent_worker_option_string(wtf);
let r = s.to_js(global_object);
s.deref();
r
});
}
}
Expand Down Expand Up @@ -332,7 +349,14 @@ mod _impl {

// argv omits "bun" because it could be "bun run" or "bun" and it's kind of ambiguous
// argv also omits the script name
let mut args_list: Vec<BunString> = Vec::with_capacity(args_count + 2);
// Scope-exit `deref` releases the +1 from `clone_*` in the worker
// branch; it is a no-op for the ZigString/Static entries.
Comment thread
robobun marked this conversation as resolved.
let mut args_list =
scopeguard::guard(Vec::<BunString>::with_capacity(args_count + 2), |v| {
for a in &v {
a.deref();
}
});

if vm.standalone_module_graph.is_some() {
// Don't break user's code because they did process.argv.slice(2)
Expand Down Expand Up @@ -370,7 +394,7 @@ mod _impl {

if let Some(worker) = worker {
for &arg in worker.argv() {
args_list.push(BunString::init(arg));
args_list.push(clone_parent_worker_option_string(arg));
}
} else {
for arg in &vm.argv {
Expand Down
40 changes: 40 additions & 0 deletions test/js/web/workers/worker-argv-cross-thread-fixture.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions test/js/web/workers/worker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,24 @@ describe("worker_threads", () => {
expect(process.execArgv).toEqual(original_execArgv);
});

// A web Worker with an explicit argv/execArgv whose worker thread reads
// process.argv, followed by a node:worker_threads Worker in the same
// process, used to crash on Windows (STATUS_STACK_BUFFER_OVERRUN) because
// building process.argv wrapped the parent-thread StringImpl instead of
// copying it. The crash only reproduced under `bun test`, so spawn the
// fixture as a test subprocess; a crash surfaces as a non-zero exit instead
// of taking out this test runner.
test("web Worker argv followed by worker_threads Worker does not crash", async () => {
await using proc = Bun.spawn({
cmd: [bunExe(), "test", path.join(import.meta.dir, "worker-argv-cross-thread-fixture.ts")],
env: bunEnv,
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toContain("2 pass");
expect(exitCode).toBe(0);
});

test("worker with eval = false validates the filename", () => {
// eval:false is equivalent to omitting eval, so a bare string that isn't a
// path is rejected synchronously like Node (ERR_WORKER_PATH), rather than
Expand Down
Loading