Skip to content

worker_threads: copy argv/execArgv for the worker thread instead of sharing the parent's StringImpls #37140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
robobun wants to merge 1 commit into from
Closed

worker_threads: copy argv/execArgv for the worker thread instead of sharing the parent's StringImpls #37140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 2 additions & 12 deletions src/jsc/VirtualMachine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1780,8 +1780,7 @@ pub struct RuntimeHooks {
/// the caller writes the returned bool back into
/// `transform_options.allow_addons` so the override semantics
/// ("override the existing even if it was set") match.
pub parse_worker_exec_argv_allow_addons:
unsafe fn(exec_argv: &[bun_core::WTFStringImpl]) -> Option<bool>,
pub parse_worker_exec_argv_allow_addons: fn(exec_argv: &[Box<[u8]>]) -> Option<bool>,
/// `CronJob.clearAllForVM(vm, .teardown)`. `CronJob` lives in
/// `bun_runtime::api::cron`.
pub cron_clear_all_teardown: fn(vm: &mut VirtualMachine),
Expand Down Expand Up @@ -2327,16 +2326,7 @@ impl VirtualMachine {
|| self
.worker_ref()
.and_then(crate::web_worker::WebWorker::exec_argv)
.is_some_and(|exec_argv| {
use bun_core::WTFStringImplExt as _;
exec_argv.iter().any(|&arg| {
// SAFETY: each entry borrows the C++ `WorkerOptions`
// array, kept alive by the owning `WebCore::Worker`
// for the worker's lifetime (see `WebWorker::argv`).
!arg.is_null()
&& is_bootstrap_flag(unsafe { &*arg }.to_owned_slice_z().as_bytes())
})
});
.is_some_and(|exec_argv| exec_argv.iter().any(|arg| is_bootstrap_flag(arg)));
if needs_pre_execution {
// The C++ side catches and reports any JS exception thrown while
// evaluating `internal/process/pre_execution`.
Expand Down
79 changes: 44 additions & 35 deletions src/jsc/web_worker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
use core::ptr::NonNull;
use core::sync::atomic::{AtomicBool, AtomicU32, Ordering};

use bun_core::{String as BunString, WTFStringImpl};
use bun_core::{String as BunString, WTFStringImpl, WTFStringImplExt as _};
use bun_io::KeepAlive;
use bun_threading::{Futex, Mutex};

Expand Down Expand Up @@ -97,12 +97,16 @@
mini: bool,
eval_mode: bool,
store_fd: bool,
/// Borrowed from C++ `WorkerOptions` (kept alive by the owning `Worker`).
argv_ptr: *const WTFStringImpl,
argv_len: usize,
exec_argv_ptr: *const WTFStringImpl,
exec_argv_len: usize,
inherit_exec_argv: bool,
/// Owned UTF-8 copies of the `WorkerOptions` argv/execArgv strings, made
/// on the parent thread in [`Self::create`]. The worker thread must never
/// see the parent's `WTF::StringImpl`s: wrapping one in a worker-heap
/// `JSString` lets worker-side atomization insert the shared impl into the
/// worker's thread-local atom table, and the parent's final deref (GC
/// sweep of the `Worker` wrapper) then aborts in `AtomStringImpl::remove`
/// ("the atom is in the string table of an other thread").
argv: Vec<Box<[u8]>>,
/// `None` when the worker inherits the parent's execArgv.
exec_argv: Option<Vec<Box<[u8]>>>,
/// Heap-owned by this struct; freed in `destroy()`.
unresolved_specifier: Box<[u8]>,
preloads: Vec<Box<[u8]>>,
Expand Down Expand Up @@ -436,26 +440,17 @@
self.eval_mode
}

/// Borrowed from the C++ `WorkerOptions` (kept alive by the owning
/// `WebCore::Worker`).
/// Worker-owned UTF-8 copies of the `WorkerOptions` argv strings.
#[inline]
pub fn argv(&self) -> &[WTFStringImpl] {
// SAFETY: `argv_ptr[..argv_len]` is borrowed from C++ WorkerOptions
// (BACKREF — kept alive by the owning Worker for `self`'s lifetime).
// `(null, 0)` is tolerated by `ffi::slice`.
unsafe { bun_core::ffi::slice(self.argv_ptr, self.argv_len) }
pub fn argv(&self) -> &[Box<[u8]>] {
&self.argv
}

/// `None` when
/// `inherit_exec_argv` (the worker inherits the parent's execArgv),
/// otherwise `Some(slice)` (possibly empty) borrowed from C++ WorkerOptions.
/// `None` when the worker inherits the parent's execArgv, otherwise
/// `Some(slice)` (possibly empty) of worker-owned UTF-8 copies.
#[inline]
pub fn exec_argv(&self) -> Option<&[WTFStringImpl]> {
if self.inherit_exec_argv {
return None;
}
// SAFETY: see `argv()`.
Some(unsafe { bun_core::ffi::slice(self.exec_argv_ptr, self.exec_argv_len) })
pub fn exec_argv(&self) -> Option<&[Box<[u8]>]> {
self.exec_argv.as_deref()
}

fn set_requested_terminate(&self) -> bool {
Expand Down Expand Up @@ -541,6 +536,28 @@
// SAFETY: `parent` is live (see above); borrow ends at `;`.
let store_fd = unsafe { (*parent).transpiler.resolver.store_fd };

// Copy argv/execArgv to worker-owned UTF-8 while still on the parent
// thread; see the `argv` field doc for why the `WTF::StringImpl`s must
// not cross into the worker thread.
let copy_args = |ptr: *const WTFStringImpl, len: usize| -> Vec<Box<[u8]>> {
// SAFETY: caller passed a valid (ptr, len) pair (or `(null, 0)`,
// tolerated by `ffi::slice`) of live `WTF::StringImpl*`s kept
// alive by `Worker::create` across this call.
unsafe { bun_core::ffi::slice(ptr, len) }
.iter()
.map(|&s| {
// SAFETY: each element is a live `WTF::StringImpl*` (see above).
unsafe { &*s }.to_utf8().slice().to_vec().into_boxed_slice()
})

Check warning on line 551 in src/jsc/web_worker.rs

View check run for this annotation

Claude / Claude Code Review

argv/execArgv lone surrogates are replaced with U+FFFD by the UTF-8 round-trip 

nit: the UTF-8 round-trip here replaces lone surrogates with U+FFFD — `to_utf8()` on a 16-bit impl routes through `convert_utf16_to_utf8_append` → `decode_utf16_with_fffd`, so `new Worker(src, {argv:['\uD800foo']})` now sees `process.argv[2].charCodeAt(0) === 0xFFFD` where it was `0xD800` before (the old direct-wrap path preserved code units exactly). REVIEW.md's boundary-representation section calls out WTF-8 for lone surrogates specifically. Exotic trigger, so not blocking; consider storing `B
Comment on lines +548 to +551

@claude claude Bot Aug 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 nit: the UTF-8 round-trip here replaces lone surrogates with U+FFFD — to_utf8() on a 16-bit impl routes through convert_utf16_to_utf8_append → decode_utf16_with_fffd, so new Worker(src, {argv:['\uD800foo']}) now sees process.argv[2].charCodeAt(0) === 0xFFFD where it was 0xD800 before (the old direct-wrap path preserved code units exactly). REVIEW.md's boundary-representation section calls out WTF-8 for lone surrogates specifically. Exotic trigger, so not blocking; consider storing Box<[u16]> for 16-bit impls (or a WTF-8-preserving encode) as a follow-up.

Extended reasoning...

What the bug is

The new copy_args closure converts each argv/execArgv WTF::StringImpl to owned bytes via WTFStringImplExt::to_utf8(), and the worker later reconstructs a JSString via BunString::borrow_utf8(arg).to_js(). For a 16-bit impl, to_utf8() (src/bun_core/string/wtf.rs:60) calls strings::to_utf8_alloc(self.utf16_slice()) → convert_utf16_to_utf8_append (src/bun_core/lib.rs:1603). When simdutf returns Status::SURROGATE, that falls back to append_wtf8_from_utf16 (line 1590), whose doc comment reads "Unpaired surrogates are replaced with U+FFFD" and whose body calls decode_utf16_with_fffd — returning (0xFFFD, 1) for a lone lead or trail surrogate (lines 1503-1511). The replacement happens before encoding, so the stored bytes are EF BF BD (UTF-8 for U+FFFD), not the WTF-8 ED A0 80 for U+D800. borrow_utf8().to_js() on the worker side then decodes standard UTF-8 and the original code unit is unrecoverable.

Why this is a regression

Before this PR, node_process.rs did BunString::init(wtf).to_js(), which wraps the parent's StringImpl* directly in a worker-heap JSString — the exact code-unit sequence was preserved byte-for-byte. That direct sharing is precisely the atomization bug this PR fixes, so the fidelity loss is a direct consequence of the change, not a pre-existing behavior. Node.js also preserves lone surrogates in worker argv (options are structured-cloned, which round-trips WTF-16 exactly).

Why nothing prevents it

The PR description says "Behavior parity checked … (unicode, empty strings)", but valid Unicode round-trips fine through this path — only unpaired surrogates hit the FFFD replacement branch, and no test in the diff exercises one. REVIEW.md's "Validate representation at every boundary" section explicitly names "WTF-8 helpers for lone surrogates (real Windows paths contain them)" as the class to guard.

Step-by-step proof

  1. Parent runs new Worker(src, { eval: true, argv: ['\uD800foo'] }).
  2. WebWorker::create → copy_args iterates the argv slice; the element is a 16-bit StringImpl with code units [0xD800, 0x66, 0x6F, 0x6F].
  3. to_utf8() sees !is_8bit() → to_utf8_alloc([0xD800, 0x66, 0x6F, 0x6F]).
  4. simdutf returns SURROGATE; append_wtf8_from_utf16 calls decode_utf16_with_fffd([0xD800, ...]) → (0xFFFD, 1), then encode_wtf8_rune writes EF BF BD. Remaining ASCII appends as 66 6F 6F. Stored Box<[u8]> = [0xEF, 0xBF, 0xBD, 0x66, 0x6F, 0x6F].
  5. Worker thread: create_argv → BunString::borrow_utf8(arg).to_js() decodes those bytes as UTF-8 → JS string "\uFFFDfoo".
  6. process.argv[2].charCodeAt(0) yields 0xFFFD; on the previous release it yields 0xD800.

Impact

Narrow: a JS string containing a lone surrogate passed through worker argv/execArgv is silently altered. The realistic case is a Windows path with an unpaired surrogate (real dirents can contain them) forwarded via argv. Exotic enough not to block a fix for a CI-killing memory-safety abort, but it is a genuine Node-compat / data-fidelity regression introduced by this change.

Suggested fix

Either keep 16-bit impls as Box<[u16]> and go through BunString::borrow_utf16() on the worker side, or use a WTF-8-preserving encode (encode the surrogate code point itself, not U+FFFD) paired with a WTF-8-aware to_js path. Both avoid the atomization bug (worker owns the bytes) while preserving code-unit fidelity.

.collect()
};

Check failure on line 553 in src/jsc/web_worker.rs

View check run for this annotation

Claude / Claude Code Review

copy_args dereferences null WTFStringImpl (Symbol() in argv/execArgv crashes the parent) 

`copy_args` does `unsafe { &*s }.to_utf8()` on each `WTF::StringImpl*` with no null check, but a null impl is reachable: `coerceToIsolatedString` returns `String()` for `Symbol()` (no description), so `new Worker('x', {eval:true, argv:[Symbol()]})` (or `execArgv`) segfaults the **parent** thread inside `WebWorker__create`. The pre-PR consumers guarded this exact case (`!arg.is_null()` in `VirtualMachine.rs`, `if arg.is_null() { continue; }` in `jsc_hooks.rs`); both guards were removed and the co
Comment on lines +546 to +553

@claude claude Bot Aug 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 copy_args does unsafe { &*s }.to_utf8() on each WTF::StringImpl* with no null check, but a null impl is reachable: coerceToIsolatedString returns String() for Symbol() (no description), so new Worker('x', {eval:true, argv:[Symbol()]}) (or execArgv) segfaults the parent thread inside WebWorker__create. The pre-PR consumers guarded this exact case (!arg.is_null() in VirtualMachine.rs, if arg.is_null() { continue; } in jsc_hooks.rs); both guards were removed and the copy site that replaces them has none — skip null entries (or push an empty Box<[u8]>) in copy_args.

Extended reasoning...

What the bug is

The new copy_args closure in WebWorker::create iterates the C++ Vector<WTF::String> (received as *const WTFStringImpl, i.e. *const *mut WTFStringImplStruct) and unconditionally dereferences each element:

.map(|&s| {
    // SAFETY: each element is a live `WTF::StringImpl*` (see above).
    unsafe { &*s }.to_utf8().slice().to_vec().into_boxed_slice()
})

WTFStringImpl is pub type WTFStringImpl = *mut WTFStringImplStruct (bun_alloc/lib.rs:984) — a nullable raw pointer. When s is null, unsafe { &*s } is immediate UB, and to_utf8() reads m_hash_and_flags through it → segfault.

How null reaches this code path

JSWorker.cpp:303-305 defines:

auto coerceToIsolatedString = [lexicalGlobalObject](JSValue v) -> String {
    String original = v.isSymbol() ? asSymbol(v)->tryGetDescriptiveString().value_or(String()) : v.toWTFString(lexicalGlobalObject);
    return original.isolatedCopy();
};

For Symbol() (no description), the uid is a null symbol, so tryGetDescriptiveString() returns std::nullopt (every other in-tree caller — BunProcess.cpp, ErrorCode.cpp — branches on has_value() with a fallback string, confirming this). .value_or(String()) yields a null-impl WTF::String, and .isolatedCopy() on a null String stays null. No exception is thrown, so RETURN_IF_EXCEPTION doesn't fire and options.argv.append(str) stores a null-impl entry.

validateArray only checks that the container is an array (min length 0); it does not validate element types. The existing test file even carries // TODO(@190n) get our handling of non-string array elements in line with Node's, acknowledging non-string elements are reachable.

Worker.cpp then reinterpret_casts Vector<WTF::String> to WTF::StringImpl** and passes it as argv_ptr/exec_argv_ptr to WebWorker__create. A WTF::String IS a RefPtr<StringImpl>, so a null-impl String becomes a null StringImpl* array element on the Rust side.

Why this is a regression

The pre-PR code guarded exactly this condition, and this diff removes both guards:

  • VirtualMachine.rs (removed): !arg.is_null() && is_bootstrap_flag(...)
  • jsc_hooks.rs (removed): if arg.is_null() { continue; }

Before this PR, the null was stored on the WebWorker and either skipped by these guarded scans, or only crashed lazily on the worker thread if process.argv was actually read. After this PR, the deref happens eagerly on the parent thread, inside the Worker constructor, before the worker thread is even spawned — strictly worse. Per REVIEW.md: "Delete defensive code only when you can show the condition cannot occur" and "user-reachable failures are recoverable errors, never panics".

Step-by-step proof

  1. User runs new Worker('1', { eval: true, execArgv: [Symbol()] }).
  2. JSWorker.cpp validateArray passes (it's an array of length 1 ≥ 0).
  3. forEachInIterable calls coerceToIsolatedString(Symbol()) → tryGetDescriptiveString() returns nullopt → String() (null impl) → .isolatedCopy() → still null. No exception. execArgv.append(String{null}).
  4. Worker::create calls WebWorker__create(..., reinterpret_cast<StringImpl**>(execArgv.begin()), 1, ...). Element 0 is a null StringImpl*.
  5. Rust copy_args(exec_argv_ptr, 1) builds a slice [null: *mut WTFStringImplStruct] and does unsafe { &*null }.to_utf8() → null-pointer deref → segfault on the parent thread.

Fix

Restore the null skip in copy_args, mirroring the removed guards:

unsafe { bun_core::ffi::slice(ptr, len) }
    .iter()
    .filter_map(|&s| {
        // A `WTF::String` may have a null impl (e.g. `Symbol()` in
        // argv/execArgv via `coerceToIsolatedString`); skip it, matching
        // the pre-existing consumer guards this replaces.
        core::ptr::NonNull::new(s).map(|nn| unsafe { nn.as_ref() }.to_utf8().slice().to_vec().into_boxed_slice())
    })
    .collect()

(or push an empty Box<[u8]> for null if positional alignment matters — for execArgv scanning it doesn't, and Node stringifies Symbol() to "Symbol()", so either is closer than a crash).

let argv = copy_args(argv_ptr, argv_len);
let exec_argv = if inherit_exec_argv {
None
} else {
Some(copy_args(exec_argv_ptr, exec_argv_len))
};

let worker = bun_core::heap::into_raw(Box::new(WebWorker {
cpp_worker,
// `parent` is the calling thread's live VM; non-null by FFI contract.
Expand All @@ -549,11 +566,8 @@
mini,
eval_mode,
store_fd,
argv_ptr,
argv_len,
exec_argv_ptr,
exec_argv_len,
inherit_exec_argv,
argv,
exec_argv,
unresolved_specifier: spec_slice.slice().to_vec().into_boxed_slice(),
preloads,
name: if name_str.is_empty() {
Expand Down Expand Up @@ -861,14 +875,9 @@
// RunCommand param table. The param table lives in
// `bun_runtime::cli` (forward-dep), so dispatch through
// `RuntimeHooks::parse_worker_exec_argv_allow_addons`. Currently
// only honours `--no-addons`; the hook owns the temporary UTF-8
// alloc + clap parse + `args.deinit()`. `None` on parse failure
// only honours `--no-addons`. `None` on parse failure
// (the parent's setting is kept).

// SAFETY: `exec_argv` borrows C++ `WorkerOptions` kept alive by the
// owning `WebCore::Worker` for `self`'s lifetime; the hook only
// reads the slice and owns its own temporary allocations.
let parsed = unsafe { (hooks.parse_worker_exec_argv_allow_addons)(exec_argv) };
let parsed = (hooks.parse_worker_exec_argv_allow_addons)(exec_argv);
if let Some(allow_addons) = parsed {
let parent_allows = transform_options.allow_addons.unwrap_or(true);
transform_options.allow_addons = Some(parent_allows && allow_addons);
Expand Down
22 changes: 5 additions & 17 deletions src/runtime/jsc_hooks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
//! 4. `__bun_get_vm_ctx` / `__bun_stdio_blob_store_new` /
//! `__bun_http_sync_download_*` — low-tier extern impls.

use bun_core::WTFStringImplExt as _;
use bun_options_types::LoaderExt as _;
use core::cell::Cell;
use core::ffi::c_void;
Expand Down Expand Up @@ -1513,27 +1512,16 @@ unsafe fn apply_standalone_runtime_flags(
///
/// Note: the Rust `bun_clap::parse_ex` port currently constrains
/// `ArgIter<'static>` (parsed values are stored by reference), which would
/// force leaking the per-call UTF-8 copies of `exec_argv`. Spec only ever
/// force leaking a copy of `exec_argv`. Spec only ever
/// reads the single `--no-addons` flag from the result (per the in-tree
/// `// TODO: currently this only checks for --no-addons`), so this body scans
/// the converted argv directly with the same `stop_after_positional_at = 1`
/// the argv directly with the same `stop_after_positional_at = 1`
/// short-circuit. Full clap routing can return when `ComptimeClap` grows a
/// borrowed-lifetime variant.
///
/// # Safety
/// Each `WTFStringImpl` in `exec_argv` is a live WTF string (the C++
/// `Worker::create` array, kept alive for the worker's lifetime).
unsafe fn parse_worker_exec_argv_allow_addons(
exec_argv: &[bun_core::WTFStringImpl],
) -> Option<bool> {
fn parse_worker_exec_argv_allow_addons(exec_argv: &[Box<[u8]>]) -> Option<bool> {
let mut no_addons = false;
for &arg in exec_argv {
if arg.is_null() {
continue;
}
// SAFETY: per fn contract — `arg` is a live `WTFStringImpl*`.
let owned = unsafe { &*arg }.to_owned_slice_z();
let bytes = owned.as_bytes();
for arg in exec_argv {
let bytes: &[u8] = arg;
// `stop_after_positional_at = 1` — first non-flag token ends parsing.
if bytes.first() != Some(&b'-') {
break;
Expand Down
8 changes: 4 additions & 4 deletions src/runtime/node/node_process.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,8 @@ mod _impl {
if let Some(worker) = vm.worker_ref() {
// 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)
return JSValue::create_array_from_iter(global_object, exec_argv.iter(), |arg| {
BunString::borrow_utf8(arg).to_js(global_object)
});
}
}
Expand Down Expand Up @@ -402,8 +402,8 @@ mod _impl {
}

if let Some(worker) = worker {
for &arg in worker.argv() {
args_list.push(BunString::init(arg));
for arg in worker.argv() {
args_list.push(BunString::borrow_utf8(arg));
}
} else {
for arg in &vm.argv {
Expand Down
32 changes: 3 additions & 29 deletions src/runtime/shell/interpreter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
//! own data up via `interp.node_mut(this)` / `interp.nodes[this]`.

use bun_collections::VecExt;
use bun_core::WTFStringImplExt as _;
use bun_jsc::JsCell;
use core::cell::Cell;
use core::fmt;
Expand Down Expand Up @@ -308,10 +307,6 @@ pub struct Interpreter {
pub(crate) cleanup_state: Cell<CleanupState>,
pub(crate) estimated_size_for_gc: Cell<usize>,

/// Lazily-populated UTF-8 cache for the JS-side argv (`$@`/`$N` expansion
/// when running under a Worker). See [`Interpreter::get_vm_args_utf8`].
pub(crate) vm_args_utf8: JsCell<Vec<bun_core::ZigStringSlice>>,

/// `bun run` CLI context for `$N` expansion on the mini event loop.
/// Null when constructed from JS (no `ContextData` is reachable).
pub(crate) command_ctx: *mut bun_options_types::context::ContextData,
Expand Down Expand Up @@ -581,7 +576,6 @@ impl Interpreter {
this_jsvalue: Cell::new(crate::jsc::JSValue::ZERO),
cleanup_state: Cell::new(CleanupState::NeedsFullCleanup),
estimated_size_for_gc: Cell::new(0),
vm_args_utf8: JsCell::new(Vec::new()),
command_ctx: ctx,
});
// Wire the interpreter backref into root stdin so async poll
Expand Down Expand Up @@ -621,8 +615,6 @@ impl Interpreter {
// Free buffered IO, env
// maps, cwd fd; do NOT free the struct itself (it's embedded).
self.root_shell.with_mut(|rs| rs.deinit_embedded(true));
// `vm_args_utf8` slices Drop themselves (`ZigStringSlice` has a Drop
// impl that derefs the WTF backing); the Vec frees on box drop.
}

/// Standalone-shell entrypoint for `bun <file>.sh`: parse `src` (already
Expand Down Expand Up @@ -1025,11 +1017,6 @@ impl Interpreter {
size += self.root_shell.get().memory_cost();
size += self.root_io.get().memory_cost();
size += self.jsobjs.len() * core::mem::size_of::<crate::jsc::JSValue>();
let vm_args = self.vm_args_utf8.get();
for arg in vm_args {
size += arg.slice().len();
}
size += vm_args.capacity() * core::mem::size_of::<bun_core::ZigStringSlice>();
size
}

Expand Down Expand Up @@ -1381,9 +1368,7 @@ impl Interpreter {
}

this.keep_alive.with_mut(|k| k.disable());
// `args: Box<ShellArgs>` and `vm_args_utf8: Vec<ZigStringSlice>` drop
// with the box; `ZigStringSlice` has a `Drop` impl that derefs its
// WTF backing.
// `args: Box<ShellArgs>` drops with the box.
}

pub(crate) fn is_running(
Expand Down Expand Up @@ -1470,7 +1455,6 @@ impl Interpreter {
original_int: u8,
event_loop: EventLoopHandle,
command_ctx: *mut bun_options_types::context::ContextData,
vm_args_utf8: &mut Vec<bun_core::ZigStringSlice>,
) {
let mut int = original_int;
match event_loop {
Expand Down Expand Up @@ -1505,19 +1489,9 @@ impl Interpreter {
// SAFETY: `vm.worker` is set in `VirtualMachine::initWorker`
// to a live `*WebWorker` for the worker's lifetime.
let worker = unsafe { &*worker_ptr.cast::<bun_jsc::web_worker::WebWorker>() };
let argv = worker.argv();
if int as usize >= argv.len() {
return;
}
if vm_args_utf8.len() != argv.len() {
vm_args_utf8.reserve(argv.len());
for arg in argv {
// SAFETY: each `WTFStringImpl` in `argv` is a live
// `*WTF::StringImpl` borrowed from `worker.argv`.
vm_args_utf8.push(unsafe { (**arg).to_utf8() });
}
if let Some(arg) = worker.argv().get(int as usize) {
out.extend_from_slice(arg);
}
out.extend_from_slice(vm_args_utf8[int as usize].slice());
return;
}

Expand Down
12 changes: 1 addition & 11 deletions src/runtime/shell/states/Expansion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,15 +118,8 @@ impl Expansion {
/// `child_done` advances `word_idx`.
pub(crate) fn next(interp: &Interpreter, this: NodeId) -> Yield {
loop {
// Split-borrow: `me` from `nodes`, `vm_args_utf8` from its own
// field, so `expand_simple_no_io` can expand `$N` without aliasing.
// R-2: both are `JsCell`-backed; `as_ptr()`/`node_mut()` project
// disjoint `&mut` from `&Interpreter`.
let event_loop = interp.event_loop;
let command_ctx = interp.command_ctx;
// SAFETY: single-JS-thread; `vm_args_utf8` and `nodes` are
// disjoint `JsCell` fields (no aliasing between the two borrows).
let vm_args_utf8 = unsafe { &mut *interp.vm_args_utf8.as_ptr() };
let me = interp.as_expansion_mut(this);
match me.state {
ExpansionState::Idle => {
Expand Down Expand Up @@ -180,7 +173,6 @@ impl Expansion {
true,
event_loop,
command_ctx,
vm_args_utf8,
);
if !is_cmd_subst {
me.word_idx += 1;
Expand Down Expand Up @@ -462,7 +454,6 @@ impl Expansion {
expand_tilde: bool,
event_loop: EventLoopHandle,
command_ctx: *mut bun_options_types::context::ContextData,
vm_args_utf8: &mut Vec<bun_core::ZigStringSlice>,
) -> bool {
use crate::shell::env_str::EnvStr;
match atom {
Expand All @@ -486,8 +477,7 @@ impl Expansion {
}
}
ast::SimpleAtom::VarArgv(int) => {
// SAFETY: `command_ctx` is the live VM ctx; `vm_args_utf8` borrows it.
Interpreter::append_var_argv(out, *int, event_loop, command_ctx, vm_args_utf8);
Interpreter::append_var_argv(out, *int, event_loop, command_ctx);
}
ast::SimpleAtom::Asterisk => {
meta_offsets.push(out.len() as u32);
Expand Down
Loading