Skip to content

Commit 61725ba

Browse files
jdaltonRalph Küpper
andauthored
fix(child_process): spawn via posix_spawn to avoid a macOS fork/dyld deadlock (#7157)
`std::process::Command` only uses `posix_spawn` when the program is given as a path and no `pre_exec`/uid/gid closures are set; a bare command name combined with an `env` option (which calls `env_clear()`) drops it onto the `fork`+`exec` fallback. On macOS a `fork` from Perry's multithreaded runtime (async reactor + GC/worker threads) can deadlock the child post-`exec` in dyld (`RemoteNotificationResponder::blockOnSynchronousEvent`): the child inherits locks/Mach state from parent threads that no longer exist after `fork`. The reader/waiter threads then block forever in `read()`/`wait4()` and the main loop idles in `js_wait_for_event`. Resolve a bare command name to its absolute path in the child's effective PATH before building the `Command`, so std stays on the `posix_spawn` fast path even when an `env`/`cwd` option is present; `argv[0]` is preserved via `arg0`. The `exec`/`execSync`/promisify(exec) shell now uses the absolute `/bin/sh` (Node's shell), and `spawn`/`spawnSync`/`execFile`/`execFileSync`/spawn_background all resolve their program. Verified with a dyld interposer: `exec`/`spawnSync` with an `env` option go from `fork()` to `posix_spawn` while stdout/exit-code capture and argv are unchanged. `detached` (setsid), `fork()`'s IPC dup2, and uid/gid necessarily keep std's fork path — `posix_spawn` can't express them via std — and are documented as such; they are outside the reported exec/spawn impact. Co-authored-by: Ralph Küpper <ralph@skelpo.com>
1 parent 173d62b commit 61725ba

5 files changed

Lines changed: 244 additions & 23 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
**macOS `child_process` fork/dyld deadlock:** `exec`/`spawn`/`execFile` (and their `Sync` forms) could deadlock a child on macOS. `std::process::Command` falls back from `posix_spawn` to `fork`+`exec` whenever a bare command name is combined with an `env` option (`env_clear()` sets `env_saw_path()`), and `fork` from Perry's multithreaded runtime (async reactor + GC/worker threads) leaves the child holding locks/Mach state from parent threads that no longer exist — so a fast child like `sh -c "echo hi"` hangs post-`exec` in dyld (`RemoteNotificationResponder::blockOnSynchronousEvent`), the reader/waiter threads block in `read()`/`wait4()`, and the main loop idles in `js_wait_for_event`. Perry now resolves a bare command name to its absolute path in the child's effective PATH before building the `Command`, keeping std on the `posix_spawn` fast path (`argv[0]` preserved via `arg0`); the `exec` shell uses the absolute `/bin/sh`. Verified with a dyld interposer: `env`-carrying `exec`/`spawnSync` go from `fork()` to `posix_spawn` with output/exit-code/argv capture unchanged. `detached` (setsid), `fork()`'s IPC `dup2`, and uid/gid necessarily keep std's fork path (not expressible through std's `posix_spawn`) and are documented inline. Linux behavior is unchanged.

crates/perry-runtime/src/child_process/exec.rs

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,10 @@ pub extern "C" fn js_child_process_exec_sync(
4646
// Execute the command using the shell, honoring `cwd`/`env` options.
4747
#[cfg(unix)]
4848
let mut command = {
49-
let mut c = Command::new("sh");
49+
// Absolute path (Node's `exec` shell) keeps std on `posix_spawn`
50+
// instead of the `fork`+`exec` fallback a bare "sh" + `env` triggers
51+
// (the macOS fork/dyld deadlock fix — see `cp_command_for_program`).
52+
let mut c = Command::new("/bin/sh");
5053
c.arg("-c").arg(&cmd_str);
5154
c
5255
};
@@ -277,7 +280,10 @@ pub extern "C" fn js_child_process_exec(cmd_ptr: *const StringHeader, arg1: f64,
277280
// `env` from the options are applied here.
278281
#[cfg(unix)]
279282
let mut command = {
280-
let mut c = Command::new("sh");
283+
// Absolute path (Node's `exec` shell) keeps std on `posix_spawn`
284+
// instead of the `fork`+`exec` fallback a bare "sh" + `env` triggers
285+
// (the macOS fork/dyld deadlock fix — see `cp_command_for_program`).
286+
let mut c = Command::new("/bin/sh");
281287
c.arg("-c").arg(&cmd_str);
282288
c
283289
};
@@ -354,7 +360,7 @@ pub extern "C" fn js_child_process_exec_file(
354360

355361
// `cwd`/`env` come from the options slot; when `opts_val` is the callback
356362
// (`execFile(file, args, cb)`) it's a closure, so the helper no-ops.
357-
let mut command = Command::new(&file_str);
363+
let mut command = cp_command_for_program(&file_str, opts_val);
358364
command.args(&arg_strs);
359365
cp_apply_options(&mut command, opts_val);
360366
let run_options = cp_read_async_run_options(opts_val);
@@ -393,7 +399,7 @@ pub extern "C" fn js_child_process_exec_file_sync(
393399
return cp_box_output(b"", &mode);
394400
}
395401
let arg_strs = cp_args_from_value(args_val);
396-
let mut command = Command::new(&file_str);
402+
let mut command = cp_command_for_program(&file_str, opts_val);
397403
command.args(&arg_strs);
398404
cp_apply_argv0(&mut command, opts_val);
399405
cp_apply_options(&mut command, opts_val);
@@ -480,7 +486,10 @@ extern "C" fn cp_promisified_exec(_closure: *const ClosureHeader, cmd_val: f64,
480486
let cmd = cp_value_to_string(cmd_val).unwrap_or_default();
481487
#[cfg(unix)]
482488
let mut command = {
483-
let mut c = Command::new("sh");
489+
// Absolute path (Node's `exec` shell) keeps std on `posix_spawn`
490+
// instead of the `fork`+`exec` fallback a bare "sh" + `env` triggers
491+
// (the macOS fork/dyld deadlock fix — see `cp_command_for_program`).
492+
let mut c = Command::new("/bin/sh");
484493
c.arg("-c").arg(&cmd);
485494
c
486495
};
@@ -501,7 +510,9 @@ extern "C" fn cp_promisified_exec_file(
501510
) -> f64 {
502511
let file = cp_value_to_string(file_val).unwrap_or_default();
503512
let arg_strs = cp_args_from_value(args_val);
504-
let mut command = Command::new(&file);
513+
// The 2-arg promisify(execFile) wrapper has no options slot; resolve a bare
514+
// program against the parent PATH to keep std on `posix_spawn`.
515+
let mut command = cp_command_for_program(&file, cp_undefined());
505516
command.args(&arg_strs);
506517
// The 2-arg promisify(execFile) wrapper has no options slot.
507518
cp_promisified_run(

crates/perry-runtime/src/child_process/mod.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,13 @@ pub(crate) use builder::{
100100
};
101101

102102
// options.rs — command option application (cwd/env/uid/gid/argv0/detached/stdio).
103+
#[cfg(unix)]
104+
pub(crate) use options::cp_resolve_program_path;
103105
pub(crate) use options::{
104106
cp_abort_signal_is_aborted, cp_apply_argv0, cp_apply_detached, cp_apply_live_stdio,
105-
cp_apply_options, cp_build_command, cp_read_abort_signal, cp_read_stdio, cp_spawnargs_argv0,
106-
cp_stdio_from_fd, cp_stdio_js_value, cp_stdio_stream_fd, CpStdio,
107+
cp_apply_options, cp_build_command, cp_command_for_program, cp_read_abort_signal,
108+
cp_read_stdio, cp_spawnargs_argv0, cp_stdio_from_fd, cp_stdio_js_value, cp_stdio_stream_fd,
109+
CpStdio,
107110
};
108111

109112
// output.rs — output encoding, error shape, exit decoding.

crates/perry-runtime/src/child_process/options.rs

Lines changed: 167 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,12 @@ pub(crate) fn cp_apply_detached(command: &mut Command, opts_val: f64) {
140140
#[cfg(unix)]
141141
{
142142
use std::os::unix::process::CommandExt;
143+
// NOTE: a `pre_exec` closure forces std onto the `fork`+`exec` path
144+
// (posix_spawn cannot run arbitrary code), so `detached` children do not
145+
// benefit from the posix_spawn fork/dyld-deadlock fix. `setsid` is not
146+
// expressible through std's posix_spawn wrapper (no `POSIX_SPAWN_SETSID`
147+
// knob), and `detached` is a rare, deliberate full-session-detach —
148+
// unlike the common `exec`/`spawn` paths, it is not converted here.
143149
unsafe {
144150
command.pre_exec(|| {
145151
if libc::setsid() < 0 {
@@ -370,6 +376,98 @@ fn cp_default_shell() -> String {
370376
}
371377
}
372378

379+
/// Fallback search path when a child's environment carries no `PATH` — the same
380+
/// default `execvp(3)` uses (`_PATH_DEFPATH`).
381+
#[cfg(unix)]
382+
const CP_DEFAULT_PATH: &str = "/usr/bin:/bin:/usr/sbin:/sbin";
383+
384+
/// The child's effective `PATH` for resolving a bare command name. When an `env`
385+
/// option is present the child's environment *replaces* the parent's (Node
386+
/// semantics), so its `PATH` — not the parent's — governs the lookup; a missing
387+
/// `PATH` falls back to the `execvp` default. With no `env` option the parent's
388+
/// `PATH` applies.
389+
#[cfg(unix)]
390+
fn cp_effective_path(opts_val: f64) -> String {
391+
if cp_object_ptr(opts_val).is_some() {
392+
let env_val = cp_get_field(opts_val, b"env");
393+
if cp_object_ptr(env_val).is_some() {
394+
if let Some(p) = cp_value_to_string(cp_get_field(env_val, b"PATH")) {
395+
if !p.is_empty() {
396+
return p;
397+
}
398+
}
399+
return CP_DEFAULT_PATH.to_string();
400+
}
401+
}
402+
std::env::var("PATH").unwrap_or_else(|_| CP_DEFAULT_PATH.to_string())
403+
}
404+
405+
/// Whether `path` names an executable regular file (following symlinks).
406+
#[cfg(unix)]
407+
fn cp_is_executable(path: &std::path::Path) -> bool {
408+
use std::os::unix::fs::PermissionsExt;
409+
std::fs::metadata(path)
410+
.map(|m| m.is_file() && (m.permissions().mode() & 0o111 != 0))
411+
.unwrap_or(false)
412+
}
413+
414+
/// Resolve a bare command name to an absolute path by walking `path` (a
415+
/// colon-separated `PATH`), returning the first executable match. An empty
416+
/// `PATH` element means the current directory (POSIX `execvp` semantics).
417+
#[cfg(unix)]
418+
pub(crate) fn cp_resolve_program_path(program: &str, path: &str) -> Option<String> {
419+
for dir in path.split(':') {
420+
let base = if dir.is_empty() { "." } else { dir };
421+
let candidate = std::path::Path::new(base).join(program);
422+
if cp_is_executable(&candidate) {
423+
return candidate.into_os_string().into_string().ok();
424+
}
425+
}
426+
None
427+
}
428+
429+
/// Build a `Command` for `program`, resolving a bare command name to its
430+
/// absolute path in the child's effective `PATH`.
431+
///
432+
/// This is the macOS fork/dyld deadlock fix. `std::process::Command` uses
433+
/// `posix_spawn` only when the program is given as a path *and* no
434+
/// `pre_exec`/uid/gid closures are set; a bare command name combined with an
435+
/// `env` option (which triggers `env_clear()`) drops it onto the `fork`+`exec`
436+
/// fallback (see `library/std/src/sys/pal/unix/process/process_unix.rs`,
437+
/// `env_saw_path() && !program_is_path()`). On macOS a `fork` from Perry's
438+
/// multithreaded runtime (async reactor + GC/worker threads) can deadlock the
439+
/// child post-`exec` in dyld (`RemoteNotificationResponder::
440+
/// blockOnSynchronousEvent`) when the process is being observed by a Mach
441+
/// notification port (telemetry, a crash reporter, a debugger): the child
442+
/// inherits locks/Mach state from parent threads that don't exist after
443+
/// `fork`. Resolving the name to an absolute path here keeps std on the
444+
/// `posix_spawn` fast path.
445+
///
446+
/// The original name is preserved as `argv[0]` (`arg0`) so the child sees the
447+
/// same `process.argv[0]` it would have gotten from the bare name. When nothing
448+
/// resolves we fall back to the bare name unchanged — a genuine `ENOENT` never
449+
/// `exec`s a real image, so it cannot hit the dyld hang, and the error surface
450+
/// stays identical.
451+
pub(crate) fn cp_command_for_program(program: &str, opts_val: f64) -> Command {
452+
#[cfg(unix)]
453+
{
454+
if !program.is_empty() && !program.contains('/') {
455+
let path = cp_effective_path(opts_val);
456+
if let Some(abs) = cp_resolve_program_path(program, &path) {
457+
use std::os::unix::process::CommandExt;
458+
let mut command = Command::new(abs);
459+
command.arg0(program);
460+
return command;
461+
}
462+
}
463+
}
464+
#[cfg(not(unix))]
465+
{
466+
let _ = opts_val;
467+
}
468+
Command::new(program)
469+
}
470+
373471
/// Whether a self-launch uses a Node CLI mode that evaluates source text.
374472
fn cp_should_use_node_interpreter(cmd: &str, args: &[String]) -> bool {
375473
let is_self = std::env::args().next().as_deref() == Some(cmd)
@@ -423,14 +521,16 @@ pub(crate) fn cp_build_command(cmd: &str, args: &[String], opts_val: f64) -> Com
423521
line.push(' ');
424522
line.push_str(a);
425523
}
426-
let mut c = Command::new(shell_bin);
524+
// Resolve a bare shell name to an absolute path so std stays on
525+
// `posix_spawn` (see `cp_command_for_program`).
526+
let mut c = cp_command_for_program(&shell_bin, opts_val);
427527
#[cfg(windows)]
428528
c.arg("/d").arg("/s").arg("/c").arg(line);
429529
#[cfg(not(windows))]
430530
c.arg("-c").arg(line);
431531
c
432532
} else {
433-
let mut c = Command::new(program);
533+
let mut c = cp_command_for_program(&program, opts_val);
434534
c.args(args);
435535
c
436536
};
@@ -441,6 +541,71 @@ pub(crate) fn cp_build_command(cmd: &str, args: &[String], opts_val: f64) -> Com
441541
command
442542
}
443543

544+
#[cfg(all(test, unix))]
545+
mod posix_spawn_tests {
546+
use super::{cp_command_for_program, cp_resolve_program_path};
547+
use crate::child_process::cp_undefined;
548+
549+
/// A bare command name in a real `PATH` resolves to an executable absolute
550+
/// path — the precondition std needs to pick `posix_spawn` over `fork`.
551+
#[test]
552+
fn resolves_bare_name_to_absolute_executable() {
553+
let resolved = cp_resolve_program_path("sh", "/nonexistent:/bin:/usr/bin")
554+
.expect("sh should resolve on a POSIX system");
555+
assert!(
556+
resolved.starts_with('/'),
557+
"expected an absolute path, got {resolved}"
558+
);
559+
assert!(resolved.ends_with("/sh"));
560+
assert!(std::path::Path::new(&resolved).exists());
561+
}
562+
563+
/// A name that cannot be found returns `None` (caller falls back to the bare
564+
/// name, which then fails ENOENT before exec'ing any real image).
565+
#[test]
566+
fn missing_program_does_not_resolve() {
567+
assert!(cp_resolve_program_path("perry-definitely-missing-xyz", "/bin:/usr/bin").is_none());
568+
}
569+
570+
/// `cp_command_for_program` rewrites a bare name to an absolute path so std
571+
/// stays on `posix_spawn`; an already-absolute program is passed through
572+
/// unchanged.
573+
#[test]
574+
fn command_program_is_absolute_for_bare_name() {
575+
let cmd = cp_command_for_program("sh", cp_undefined());
576+
let program = cmd.get_program().to_string_lossy().into_owned();
577+
assert!(
578+
program.starts_with('/') && program.ends_with("/sh"),
579+
"bare name should resolve to an absolute path; got {program}"
580+
);
581+
582+
let passthrough = cp_command_for_program("/bin/sh", cp_undefined());
583+
assert_eq!(passthrough.get_program().to_string_lossy(), "/bin/sh");
584+
}
585+
586+
/// End-to-end: the resolved (absolute-path, no-`pre_exec`) command spawns via
587+
/// std's `posix_spawn` path and captures output correctly. This is exactly
588+
/// the shape that deadlocked in dyld when std took the `fork`+`exec` fallback
589+
/// on macOS. (Full GC-stress N/N verification uses the compiled repro under
590+
/// `PERRY_GC_FORCE_EVACUATE=1`; a raw unit test cannot drive Perry's
591+
/// thread-local GC without runtime init.)
592+
#[test]
593+
fn resolved_command_runs_and_captures_output() {
594+
let mut cmd = cp_command_for_program("sh", cp_undefined());
595+
assert!(
596+
cmd.get_program().to_string_lossy().starts_with('/'),
597+
"resolved program must be an absolute path to keep std on posix_spawn"
598+
);
599+
let out = cmd
600+
.arg("-c")
601+
.arg("printf ok-%s 42")
602+
.output()
603+
.expect("spawn resolved sh");
604+
assert!(out.status.success());
605+
assert_eq!(out.stdout, b"ok-42");
606+
}
607+
}
608+
444609
#[cfg(test)]
445610
mod tests {
446611
use super::cp_should_use_node_interpreter;

crates/perry-runtime/src/child_process/registry.rs

Lines changed: 54 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,55 @@ pub extern "C" fn js_child_process_spawn_background(
8585
None => return std::ptr::null_mut(),
8686
};
8787

88-
let mut command = Command::new(&cmd_str);
88+
// Parse the env JSON up front so we can read its `PATH` for command
89+
// resolution below (an env override with a `PATH` key is what pushes
90+
// std onto the `fork`+`exec` fallback for a bare command name).
91+
let env_map = {
92+
let env_bits = env_json_val.to_bits();
93+
if env_bits != TAG_NULL_BITS && env_bits != TAG_UNDEFINED_BITS {
94+
extract_string_from_nanboxed(env_json_val).and_then(|env_json| {
95+
serde_json::from_str::<serde_json::Map<String, serde_json::Value>>(&env_json)
96+
.ok()
97+
})
98+
} else {
99+
None
100+
}
101+
};
102+
103+
// Resolve a bare command name to an absolute path so std uses
104+
// `posix_spawn` instead of the `fork`+`exec` fallback that an env
105+
// override triggers — the macOS fork/dyld deadlock fix (see
106+
// `options::cp_command_for_program`). `arg0` preserves argv[0].
107+
let mut command = {
108+
#[cfg(unix)]
109+
{
110+
let resolved = if cmd_str.contains('/') {
111+
None
112+
} else {
113+
let path = env_map
114+
.as_ref()
115+
.and_then(|m| m.get("PATH"))
116+
.and_then(|v| v.as_str())
117+
.map(str::to_string)
118+
.or_else(|| std::env::var("PATH").ok())
119+
.unwrap_or_default();
120+
super::cp_resolve_program_path(&cmd_str, &path)
121+
};
122+
match resolved {
123+
Some(abs) => {
124+
use std::os::unix::process::CommandExt;
125+
let mut c = Command::new(abs);
126+
c.arg0(&cmd_str);
127+
c
128+
}
129+
None => Command::new(&cmd_str),
130+
}
131+
}
132+
#[cfg(not(unix))]
133+
{
134+
Command::new(&cmd_str)
135+
}
136+
};
89137

90138
// Add arguments if provided
91139
if args_ptr != 0 {
@@ -102,18 +150,11 @@ pub extern "C" fn js_child_process_spawn_background(
102150
}
103151
}
104152

105-
// Parse env JSON if provided (not null/undefined)
106-
let env_bits = env_json_val.to_bits();
107-
if env_bits != TAG_NULL_BITS && env_bits != TAG_UNDEFINED_BITS {
108-
if let Some(env_json) = extract_string_from_nanboxed(env_json_val) {
109-
if let Ok(map) =
110-
serde_json::from_str::<serde_json::Map<String, serde_json::Value>>(&env_json)
111-
{
112-
for (k, v) in map {
113-
if let Some(val_str) = v.as_str() {
114-
command.env(k, val_str);
115-
}
116-
}
153+
// Apply the parsed env (string values only), matching prior behavior.
154+
if let Some(map) = env_map {
155+
for (k, v) in map {
156+
if let Some(val_str) = v.as_str() {
157+
command.env(k, val_str);
117158
}
118159
}
119160
}

0 commit comments

Comments
 (0)