From 55737ce3682638edd61ff490c139d9ebb150114b Mon Sep 17 00:00:00 2001 From: jdalton Date: Fri, 31 Jul 2026 22:07:42 -0400 Subject: [PATCH] fix(child_process): spawn via posix_spawn to avoid a macOS fork/dyld deadlock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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. --- .../7157-macos-child-process-posix-spawn.md | 1 + .../perry-runtime/src/child_process/exec.rs | 23 ++- crates/perry-runtime/src/child_process/mod.rs | 6 +- .../src/child_process/options.rs | 169 +++++++++++++++++- .../src/child_process/registry.rs | 67 +++++-- 5 files changed, 243 insertions(+), 23 deletions(-) create mode 100644 changelog.d/7157-macos-child-process-posix-spawn.md diff --git a/changelog.d/7157-macos-child-process-posix-spawn.md b/changelog.d/7157-macos-child-process-posix-spawn.md new file mode 100644 index 0000000000..005d71a89e --- /dev/null +++ b/changelog.d/7157-macos-child-process-posix-spawn.md @@ -0,0 +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. diff --git a/crates/perry-runtime/src/child_process/exec.rs b/crates/perry-runtime/src/child_process/exec.rs index 28c88dd2c7..eee0347fac 100644 --- a/crates/perry-runtime/src/child_process/exec.rs +++ b/crates/perry-runtime/src/child_process/exec.rs @@ -46,7 +46,10 @@ pub extern "C" fn js_child_process_exec_sync( // Execute the command using the shell, honoring `cwd`/`env` options. #[cfg(unix)] let mut command = { - let mut c = Command::new("sh"); + // Absolute path (Node's `exec` shell) keeps std on `posix_spawn` + // instead of the `fork`+`exec` fallback a bare "sh" + `env` triggers + // (the macOS fork/dyld deadlock fix — see `cp_command_for_program`). + let mut c = Command::new("/bin/sh"); c.arg("-c").arg(&cmd_str); c }; @@ -280,7 +283,10 @@ pub extern "C" fn js_child_process_exec(cmd_ptr: *const StringHeader, arg1: f64, // `env` from the options are applied here. #[cfg(unix)] let mut command = { - let mut c = Command::new("sh"); + // Absolute path (Node's `exec` shell) keeps std on `posix_spawn` + // instead of the `fork`+`exec` fallback a bare "sh" + `env` triggers + // (the macOS fork/dyld deadlock fix — see `cp_command_for_program`). + let mut c = Command::new("/bin/sh"); c.arg("-c").arg(&cmd_str); c }; @@ -356,7 +362,7 @@ pub extern "C" fn js_child_process_exec_file( // `cwd`/`env` come from the options slot; when `opts_val` is the callback // (`execFile(file, args, cb)`) it's a closure, so the helper no-ops. - let mut command = Command::new(&file_str); + let mut command = cp_command_for_program(&file_str, opts_val); command.args(&arg_strs); cp_apply_options(&mut command, opts_val); let run_options = cp_read_async_run_options(opts_val); @@ -395,7 +401,7 @@ pub extern "C" fn js_child_process_exec_file_sync( return cp_box_output(b"", &mode); } let arg_strs = cp_args_from_value(args_val); - let mut command = Command::new(&file_str); + let mut command = cp_command_for_program(&file_str, opts_val); command.args(&arg_strs); cp_apply_argv0(&mut command, opts_val); cp_apply_options(&mut command, opts_val); @@ -476,7 +482,10 @@ extern "C" fn cp_promisified_exec(_closure: *const ClosureHeader, cmd_val: f64, let cmd = cp_value_to_string(cmd_val).unwrap_or_default(); #[cfg(unix)] let mut command = { - let mut c = Command::new("sh"); + // Absolute path (Node's `exec` shell) keeps std on `posix_spawn` + // instead of the `fork`+`exec` fallback a bare "sh" + `env` triggers + // (the macOS fork/dyld deadlock fix — see `cp_command_for_program`). + let mut c = Command::new("/bin/sh"); c.arg("-c").arg(&cmd); c }; @@ -497,7 +506,9 @@ extern "C" fn cp_promisified_exec_file( ) -> f64 { let file = cp_value_to_string(file_val).unwrap_or_default(); let arg_strs = cp_args_from_value(args_val); - let mut command = Command::new(&file); + // The 2-arg promisify(execFile) wrapper has no options slot; resolve a bare + // program against the parent PATH to keep std on `posix_spawn`. + let mut command = cp_command_for_program(&file, cp_undefined()); command.args(&arg_strs); // The 2-arg promisify(execFile) wrapper has no options slot. cp_promisified_run( diff --git a/crates/perry-runtime/src/child_process/mod.rs b/crates/perry-runtime/src/child_process/mod.rs index 71416bb783..0bc73daf1e 100644 --- a/crates/perry-runtime/src/child_process/mod.rs +++ b/crates/perry-runtime/src/child_process/mod.rs @@ -94,10 +94,12 @@ pub(crate) use builder::{ }; // options.rs — command option application (cwd/env/uid/gid/argv0/detached/stdio). +#[cfg(unix)] +pub(crate) use options::cp_resolve_program_path; pub(crate) use options::{ cp_abort_signal_is_aborted, cp_apply_argv0, cp_apply_detached, cp_apply_live_stdio, - cp_apply_options, cp_build_command, cp_read_abort_signal, cp_read_stdio, cp_spawnargs_argv0, - cp_stdio_from_fd, cp_stdio_js_value, CpStdio, + cp_apply_options, cp_build_command, cp_command_for_program, cp_read_abort_signal, + cp_read_stdio, cp_spawnargs_argv0, cp_stdio_from_fd, cp_stdio_js_value, CpStdio, }; // output.rs — output encoding, error shape, exit decoding. diff --git a/crates/perry-runtime/src/child_process/options.rs b/crates/perry-runtime/src/child_process/options.rs index 192549960a..163f070a74 100644 --- a/crates/perry-runtime/src/child_process/options.rs +++ b/crates/perry-runtime/src/child_process/options.rs @@ -140,6 +140,12 @@ pub(crate) fn cp_apply_detached(command: &mut Command, opts_val: f64) { #[cfg(unix)] { use std::os::unix::process::CommandExt; + // NOTE: a `pre_exec` closure forces std onto the `fork`+`exec` path + // (posix_spawn cannot run arbitrary code), so `detached` children do not + // benefit from the posix_spawn fork/dyld-deadlock fix. `setsid` is not + // expressible through std's posix_spawn wrapper (no `POSIX_SPAWN_SETSID` + // knob), and `detached` is a rare, deliberate full-session-detach — + // unlike the common `exec`/`spawn` paths, it is not converted here. unsafe { command.pre_exec(|| { if libc::setsid() < 0 { @@ -294,6 +300,98 @@ fn cp_default_shell() -> String { } } +/// Fallback search path when a child's environment carries no `PATH` — the same +/// default `execvp(3)` uses (`_PATH_DEFPATH`). +#[cfg(unix)] +const CP_DEFAULT_PATH: &str = "/usr/bin:/bin:/usr/sbin:/sbin"; + +/// The child's effective `PATH` for resolving a bare command name. When an `env` +/// option is present the child's environment *replaces* the parent's (Node +/// semantics), so its `PATH` — not the parent's — governs the lookup; a missing +/// `PATH` falls back to the `execvp` default. With no `env` option the parent's +/// `PATH` applies. +#[cfg(unix)] +fn cp_effective_path(opts_val: f64) -> String { + if cp_object_ptr(opts_val).is_some() { + let env_val = cp_get_field(opts_val, b"env"); + if cp_object_ptr(env_val).is_some() { + if let Some(p) = cp_value_to_string(cp_get_field(env_val, b"PATH")) { + if !p.is_empty() { + return p; + } + } + return CP_DEFAULT_PATH.to_string(); + } + } + std::env::var("PATH").unwrap_or_else(|_| CP_DEFAULT_PATH.to_string()) +} + +/// Whether `path` names an executable regular file (following symlinks). +#[cfg(unix)] +fn cp_is_executable(path: &std::path::Path) -> bool { + use std::os::unix::fs::PermissionsExt; + std::fs::metadata(path) + .map(|m| m.is_file() && (m.permissions().mode() & 0o111 != 0)) + .unwrap_or(false) +} + +/// Resolve a bare command name to an absolute path by walking `path` (a +/// colon-separated `PATH`), returning the first executable match. An empty +/// `PATH` element means the current directory (POSIX `execvp` semantics). +#[cfg(unix)] +pub(crate) fn cp_resolve_program_path(program: &str, path: &str) -> Option { + for dir in path.split(':') { + let base = if dir.is_empty() { "." } else { dir }; + let candidate = std::path::Path::new(base).join(program); + if cp_is_executable(&candidate) { + return candidate.into_os_string().into_string().ok(); + } + } + None +} + +/// Build a `Command` for `program`, resolving a bare command name to its +/// absolute path in the child's effective `PATH`. +/// +/// This is the macOS fork/dyld deadlock fix. `std::process::Command` uses +/// `posix_spawn` only 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 triggers `env_clear()`) drops it onto the `fork`+`exec` +/// fallback (see `library/std/src/sys/pal/unix/process/process_unix.rs`, +/// `env_saw_path() && !program_is_path()`). On macOS a `fork` from Perry's +/// multithreaded runtime (async reactor + GC/worker threads) can deadlock the +/// child post-`exec` in dyld (`RemoteNotificationResponder:: +/// blockOnSynchronousEvent`) when the process is being observed by a Mach +/// notification port (telemetry, a crash reporter, a debugger): the child +/// inherits locks/Mach state from parent threads that don't exist after +/// `fork`. Resolving the name to an absolute path here keeps std on the +/// `posix_spawn` fast path. +/// +/// The original name is preserved as `argv[0]` (`arg0`) so the child sees the +/// same `process.argv[0]` it would have gotten from the bare name. When nothing +/// resolves we fall back to the bare name unchanged — a genuine `ENOENT` never +/// `exec`s a real image, so it cannot hit the dyld hang, and the error surface +/// stays identical. +pub(crate) fn cp_command_for_program(program: &str, opts_val: f64) -> Command { + #[cfg(unix)] + { + if !program.is_empty() && !program.contains('/') { + let path = cp_effective_path(opts_val); + if let Some(abs) = cp_resolve_program_path(program, &path) { + use std::os::unix::process::CommandExt; + let mut command = Command::new(abs); + command.arg0(program); + return command; + } + } + } + #[cfg(not(unix))] + { + let _ = opts_val; + } + Command::new(program) +} + /// Whether a self-launch uses a Node CLI mode that evaluates source text. fn cp_should_use_node_interpreter(cmd: &str, args: &[String]) -> bool { let is_self = std::env::args().next().as_deref() == Some(cmd) @@ -347,14 +445,16 @@ pub(crate) fn cp_build_command(cmd: &str, args: &[String], opts_val: f64) -> Com line.push(' '); line.push_str(a); } - let mut c = Command::new(shell_bin); + // Resolve a bare shell name to an absolute path so std stays on + // `posix_spawn` (see `cp_command_for_program`). + let mut c = cp_command_for_program(&shell_bin, opts_val); #[cfg(windows)] c.arg("/d").arg("/s").arg("/c").arg(line); #[cfg(not(windows))] c.arg("-c").arg(line); c } else { - let mut c = Command::new(program); + let mut c = cp_command_for_program(&program, opts_val); c.args(args); c }; @@ -365,6 +465,71 @@ pub(crate) fn cp_build_command(cmd: &str, args: &[String], opts_val: f64) -> Com command } +#[cfg(all(test, unix))] +mod posix_spawn_tests { + use super::{cp_command_for_program, cp_resolve_program_path}; + use crate::child_process::cp_undefined; + + /// A bare command name in a real `PATH` resolves to an executable absolute + /// path — the precondition std needs to pick `posix_spawn` over `fork`. + #[test] + fn resolves_bare_name_to_absolute_executable() { + let resolved = cp_resolve_program_path("sh", "/nonexistent:/bin:/usr/bin") + .expect("sh should resolve on a POSIX system"); + assert!( + resolved.starts_with('/'), + "expected an absolute path, got {resolved}" + ); + assert!(resolved.ends_with("/sh")); + assert!(std::path::Path::new(&resolved).exists()); + } + + /// A name that cannot be found returns `None` (caller falls back to the bare + /// name, which then fails ENOENT before exec'ing any real image). + #[test] + fn missing_program_does_not_resolve() { + assert!(cp_resolve_program_path("perry-definitely-missing-xyz", "/bin:/usr/bin").is_none()); + } + + /// `cp_command_for_program` rewrites a bare name to an absolute path so std + /// stays on `posix_spawn`; an already-absolute program is passed through + /// unchanged. + #[test] + fn command_program_is_absolute_for_bare_name() { + let cmd = cp_command_for_program("sh", cp_undefined()); + let program = cmd.get_program().to_string_lossy().into_owned(); + assert!( + program.starts_with('/') && program.ends_with("/sh"), + "bare name should resolve to an absolute path; got {program}" + ); + + let passthrough = cp_command_for_program("/bin/sh", cp_undefined()); + assert_eq!(passthrough.get_program().to_string_lossy(), "/bin/sh"); + } + + /// End-to-end: the resolved (absolute-path, no-`pre_exec`) command spawns via + /// std's `posix_spawn` path and captures output correctly. This is exactly + /// the shape that deadlocked in dyld when std took the `fork`+`exec` fallback + /// on macOS. (Full GC-stress N/N verification uses the compiled repro under + /// `PERRY_GC_FORCE_EVACUATE=1`; a raw unit test cannot drive Perry's + /// thread-local GC without runtime init.) + #[test] + fn resolved_command_runs_and_captures_output() { + let mut cmd = cp_command_for_program("sh", cp_undefined()); + assert!( + cmd.get_program().to_string_lossy().starts_with('/'), + "resolved program must be an absolute path to keep std on posix_spawn" + ); + let out = cmd + .arg("-c") + .arg("printf ok-%s 42") + .output() + .expect("spawn resolved sh"); + assert!(out.status.success()); + assert_eq!(out.stdout, b"ok-42"); + } +} + #[cfg(test)] mod tests { use super::cp_should_use_node_interpreter; diff --git a/crates/perry-runtime/src/child_process/registry.rs b/crates/perry-runtime/src/child_process/registry.rs index 274e1cd4f5..dd4c121915 100644 --- a/crates/perry-runtime/src/child_process/registry.rs +++ b/crates/perry-runtime/src/child_process/registry.rs @@ -85,7 +85,55 @@ pub extern "C" fn js_child_process_spawn_background( None => return std::ptr::null_mut(), }; - let mut command = Command::new(&cmd_str); + // Parse the env JSON up front so we can read its `PATH` for command + // resolution below (an env override with a `PATH` key is what pushes + // std onto the `fork`+`exec` fallback for a bare command name). + let env_map = { + let env_bits = env_json_val.to_bits(); + if env_bits != TAG_NULL_BITS && env_bits != TAG_UNDEFINED_BITS { + extract_string_from_nanboxed(env_json_val).and_then(|env_json| { + serde_json::from_str::>(&env_json) + .ok() + }) + } else { + None + } + }; + + // Resolve a bare command name to an absolute path so std uses + // `posix_spawn` instead of the `fork`+`exec` fallback that an env + // override triggers — the macOS fork/dyld deadlock fix (see + // `options::cp_command_for_program`). `arg0` preserves argv[0]. + let mut command = { + #[cfg(unix)] + { + let resolved = if cmd_str.contains('/') { + None + } else { + let path = env_map + .as_ref() + .and_then(|m| m.get("PATH")) + .and_then(|v| v.as_str()) + .map(str::to_string) + .or_else(|| std::env::var("PATH").ok()) + .unwrap_or_default(); + super::cp_resolve_program_path(&cmd_str, &path) + }; + match resolved { + Some(abs) => { + use std::os::unix::process::CommandExt; + let mut c = Command::new(abs); + c.arg0(&cmd_str); + c + } + None => Command::new(&cmd_str), + } + } + #[cfg(not(unix))] + { + Command::new(&cmd_str) + } + }; // Add arguments if provided if args_ptr != 0 { @@ -102,18 +150,11 @@ pub extern "C" fn js_child_process_spawn_background( } } - // Parse env JSON if provided (not null/undefined) - let env_bits = env_json_val.to_bits(); - if env_bits != TAG_NULL_BITS && env_bits != TAG_UNDEFINED_BITS { - if let Some(env_json) = extract_string_from_nanboxed(env_json_val) { - if let Ok(map) = - serde_json::from_str::>(&env_json) - { - for (k, v) in map { - if let Some(val_str) = v.as_str() { - command.env(k, val_str); - } - } + // Apply the parsed env (string values only), matching prior behavior. + if let Some(map) = env_map { + for (k, v) in map { + if let Some(val_str) = v.as_str() { + command.env(k, val_str); } } }