diff --git a/changelog.d/9926-child-process-parity.md b/changelog.d/9926-child-process-parity.md new file mode 100644 index 0000000000..1396899def --- /dev/null +++ b/changelog.d/9926-child-process-parity.md @@ -0,0 +1,5 @@ +### Fixed + +- Match Node's `child_process` metadata and synchronous buffer-limit behavior by + preserving the caller's `execFile` spelling and signaling a process when its + output crosses `maxBuffer` only while it is still running. diff --git a/crates/perry-runtime/src/child_process/exec.rs b/crates/perry-runtime/src/child_process/exec.rs index adfc1ac7bc..7ed352e59e 100644 --- a/crates/perry-runtime/src/child_process/exec.rs +++ b/crates/perry-runtime/src/child_process/exec.rs @@ -62,8 +62,7 @@ pub extern "C" fn js_child_process_exec_sync( }; cp_apply_options(&mut command, opts_val); - let mut run_options = cp_read_sync_stdio_run_options(opts_val); - run_options.mark_shell_command(); + let run_options = cp_read_sync_stdio_run_options(opts_val); let run = cp_run_to_completion(command, &run_options); let stdout_box = cp_box_run_output(&run.stdout, run.stdout_piped, &mode); if run.success() { @@ -298,8 +297,7 @@ pub extern "C" fn js_child_process_exec(cmd_ptr: *const StringHeader, arg1: f64, c }; cp_apply_options(&mut command, arg1); - let mut run_options = cp_read_async_run_options(arg1); - run_options.mark_shell_command(); + let run_options = cp_read_async_run_options(arg1); if cb.is_null() { // Legacy no-callback shape — run synchronously and return stdout @@ -312,7 +310,7 @@ pub extern "C" fn js_child_process_exec(cmd_ptr: *const StringHeader, arg1: f64, // With a callback, run asynchronously: off the main thread, with the // callback fired on a later event-loop tick (#4912). - reactor::cp_exec_async(command, cmd_str, cb_val, run_options, mode) + reactor::cp_exec_async(command, cmd_str, None, cb_val, run_options, mode) } /// `child_process.execFile(file[, args][, options][, callback])` — like `exec` @@ -383,6 +381,7 @@ pub extern "C" fn js_child_process_exec_file( reactor::cp_exec_async( command, cp_file_cmd_display(&file_str, &arg_strs), + Some(file_str), cb_nanbox, run_options, mode, @@ -471,7 +470,12 @@ extern "C" fn cp_promise_settle_cb( /// `command` through the async exec reactor (#4912). Returns the NaN-boxed /// pending promise. The settle closure (and through it the promise) is kept /// alive by the reactor's exec-callback GC root. -fn cp_promisified_run(command: Command, cmd_str: String, opts: f64) -> f64 { +fn cp_promisified_run( + command: Command, + cmd_str: String, + public_spawnfile: Option, + opts: f64, +) -> f64 { let run_options = cp_read_async_run_options(opts); // promisify(exec)/promisify(execFile) yield string stdout/stderr (utf8). let mode = cp_read_output_mode(opts, true); @@ -480,7 +484,14 @@ fn cp_promisified_run(command: Command, cmd_str: String, opts: f64) -> f64 { let cb = js_closure_alloc(cp_promise_settle_cb as *const u8, 1); js_closure_set_capture_ptr(cb, 0, cp_box_ptr(promise as *const u8).to_bits() as i64); let cb_val = crate::value::js_nanbox_pointer(cb as i64); - let child = reactor::cp_exec_async(command, cmd_str, cb_val, run_options, mode); + let child = reactor::cp_exec_async( + command, + cmd_str, + public_spawnfile, + cb_val, + run_options, + mode, + ); crate::object::exotic_expando::value_store( crate::object::exotic_expando::ExoticKind::Promise, promise as usize, @@ -509,7 +520,7 @@ extern "C" fn cp_promisified_exec(_closure: *const ClosureHeader, cmd_val: f64, c }; cp_apply_options(&mut command, opts); - cp_promisified_run(command, cmd, opts) + cp_promisified_run(command, cmd, None, opts) } extern "C" fn cp_promisified_exec_file( @@ -529,6 +540,7 @@ extern "C" fn cp_promisified_exec_file( cp_promisified_run( command, cp_file_cmd_display(&file, &arg_strs), + Some(file), f64::from_bits(TAG_UNDEFINED_BITS), ) } diff --git a/crates/perry-runtime/src/child_process/reactor.rs b/crates/perry-runtime/src/child_process/reactor.rs index be104b70c7..0b7653c2d6 100644 --- a/crates/perry-runtime/src/child_process/reactor.rs +++ b/crates/perry-runtime/src/child_process/reactor.rs @@ -1223,6 +1223,7 @@ pub(super) fn cp_register_reactor_arities() { pub(super) fn cp_exec_async( mut command: Command, cmd_str: String, + public_spawnfile: Option, cb_val: f64, run_options: CpRunOptions, mode: CpOutput, @@ -1283,7 +1284,16 @@ pub(super) fn cp_exec_async( cp_set_field(cp, b"signalCode", TAG_NULL_F64); cp_set_field(cp, b"killed", TAG_FALSE_F64); cp_set_field(cp, b"connected", TAG_FALSE_F64); - cp_set_field(cp, b"spawnfile", cp_box_string(&file)); + // `cp_command_for_program` may resolve a bare executable against PATH to + // keep macOS on `posix_spawn`, but Node exposes the caller's original + // `execFile` spelling through `ChildProcess.spawnfile`. Keep the resolved + // path above for launch-error metadata and publish the explicit spelling + // when that API supplied one. `exec` continues to expose its real shell. + cp_set_field( + cp, + b"spawnfile", + cp_box_string(public_spawnfile.as_deref().unwrap_or(&file)), + ); match command.spawn() { Ok(mut child) => { diff --git a/crates/perry-runtime/src/child_process/sync_run.rs b/crates/perry-runtime/src/child_process/sync_run.rs index 403721df20..a790b78bce 100644 --- a/crates/perry-runtime/src/child_process/sync_run.rs +++ b/crates/perry-runtime/src/child_process/sync_run.rs @@ -1,5 +1,7 @@ -use std::io::Write; -use std::process::{Command, Stdio}; +use std::io::{Read, Write}; +use std::process::{Child, Command, ExitStatus, Stdio}; +use std::sync::mpsc::{self, Sender}; +use std::thread::JoinHandle; use std::time::{Duration, Instant}; use crate::value::JSValue; @@ -16,7 +18,6 @@ pub(crate) struct CpRunOptions { input: Option>, timeout: Option, kill_signal: i32, - shell_command: bool, pub(super) max_buffer: usize, stdio: [CpStdio; 3], } @@ -33,10 +34,6 @@ impl CpRunOptions { pub(super) fn timeout(&self) -> Option { self.timeout } - - pub(super) fn mark_shell_command(&mut self) { - self.shell_command = true; - } } impl Default for CpRunOptions { @@ -45,7 +42,6 @@ impl Default for CpRunOptions { input: None, timeout: None, kill_signal: CP_SIGTERM, - shell_command: false, max_buffer: CP_DEFAULT_MAX_BUFFER, stdio: [CpStdio::Pipe; 3], } @@ -129,7 +125,6 @@ pub(super) fn cp_read_spawn_sync_run_options(opts_val: f64) -> CpRunOptions { stdio.get(1).copied().unwrap_or(CpStdio::Pipe), stdio.get(2).copied().unwrap_or(CpStdio::Pipe), ]; - options.shell_command = crate::value::js_is_truthy(cp_get_field(opts_val, b"shell")) != 0; options } @@ -218,10 +213,6 @@ impl CpRun { /// Piped stdin without input is closed so children that read stdin see EOF /// instead of blocking. Used by synchronous + buffered-callback entry points. pub(super) fn cp_run_to_completion(mut command: Command, options: &CpRunOptions) -> CpRun { - // A shell that has already completed its short command reports its real - // exit status with ENOBUFS; a direct child is still terminable at the - // buffer threshold and reports the configured signal. - let shell_command = options.shell_command; let stdin_piped = matches!(options.stdio[0], CpStdio::Pipe) && options.input.is_some(); let stdout_piped = matches!(options.stdio[1], CpStdio::Pipe); let stderr_piped = matches!(options.stdio[2], CpStdio::Pipe); @@ -246,33 +237,37 @@ pub(super) fn cp_run_to_completion(mut command: Command, options: &CpRunOptions) match command.spawn() { Ok(mut child) => { let pid = child.id(); + let (limit_tx, limit_rx) = mpsc::channel(); + let stdout_reader = + cp_read_piped_output(child.stdout.take(), options, pid, limit_tx.clone()); + let stderr_reader = cp_read_piped_output(child.stderr.take(), options, pid, limit_tx); if stdin_piped { if let (Some(input), Some(mut stdin)) = (&options.input, child.stdin.take()) { let _ = stdin.write_all(input); } } - let mut run_error = - cp_wait_for_timeout(&mut child, options.timeout, options.kill_signal); - match child.wait_with_output() { - Ok(o) => { - let CpExit { code, signal } = cp_decode_status(&o.status); + drop(child.stdin.take()); + + let mut run_error = None; + match cp_wait_for_buffered_child(&mut child, options, &limit_rx, &mut run_error) { + Ok(status) => { + let stdout = cp_join_piped_output(stdout_reader); + let stderr = cp_join_piped_output(stderr_reader); + let CpExit { code, signal } = cp_decode_status(&status); if run_error.is_none() && options.max_buffer > 0 - && ((stdout_piped && o.stdout.len() > options.max_buffer) - || (stderr_piped && o.stderr.len() > options.max_buffer)) + && ((stdout_piped && stdout.len() > options.max_buffer) + || (stderr_piped && stderr.len() > options.max_buffer)) { run_error = Some(CpRunError::MaxBuffer); } let (code, signal) = match run_error { Some(CpRunError::Timeout) => (None, Some(options.kill_signal)), - Some(CpRunError::MaxBuffer) if !shell_command => { - (None, Some(options.kill_signal)) - } _ => (code, signal), }; CpRun { - stdout: o.stdout, - stderr: o.stderr, + stdout, + stderr, stdout_piped, stderr_piped, code, @@ -309,25 +304,81 @@ pub(super) fn cp_run_to_completion(mut command: Command, options: &CpRunOptions) } } -fn cp_wait_for_timeout( - child: &mut std::process::Child, - timeout: Option, - kill_signal: i32, -) -> Option { - let timeout = timeout?; - let deadline = Instant::now() + timeout; +fn cp_read_piped_output( + reader: Option, + options: &CpRunOptions, + _pid: u32, + limit_tx: Sender<()>, +) -> Option>> { + let max_buffer = options.max_buffer; + #[cfg(unix)] + let kill_signal = options.kill_signal; + reader.map(|mut reader| { + std::thread::spawn(move || { + let mut output = Vec::new(); + let mut chunk = [0_u8; 8192]; + let mut reported_limit = false; + loop { + match reader.read(&mut chunk) { + Ok(0) | Err(_) => break, + Ok(read) => { + output.extend_from_slice(&chunk[..read]); + if !reported_limit && max_buffer > 0 && output.len() > max_buffer { + // Signal at the read that crosses the limit. Going + // through the parent's polling loop here leaves a + // fast child enough time to exit before the signal. + #[cfg(unix)] + unsafe { + let _ = libc::kill(_pid as i32, kill_signal); + } + let _ = limit_tx.send(()); + reported_limit = true; + } + } + } + } + output + }) + }) +} + +fn cp_join_piped_output(reader: Option>>) -> Vec { + reader + .and_then(|reader| reader.join().ok()) + .unwrap_or_default() +} + +fn cp_wait_for_buffered_child( + child: &mut Child, + options: &CpRunOptions, + limit_rx: &mpsc::Receiver<()>, + run_error: &mut Option, +) -> std::io::Result { + let deadline = options.timeout.map(|timeout| Instant::now() + timeout); loop { + if run_error.is_none() && limit_rx.try_recv().is_ok() { + *run_error = Some(CpRunError::MaxBuffer); + // A short child can exit between filling the pipe and this + // notification. Preserve that real exit status, as Node does, + // and terminate only a child that is still running. + if let Some(status) = child.try_wait()? { + return Ok(status); + } + cp_terminate_child(child, options.kill_signal); + } + match child.try_wait() { - Ok(Some(_)) => return None, + Ok(Some(status)) => return Ok(status), Ok(None) => { - if Instant::now() >= deadline { - cp_terminate_child(child, kill_signal); - return Some(CpRunError::Timeout); + if run_error.is_none() + && deadline.is_some_and(|deadline| Instant::now() >= deadline) + { + *run_error = Some(CpRunError::Timeout); + cp_terminate_child(child, options.kill_signal); } - let remaining = deadline.saturating_duration_since(Instant::now()); - std::thread::sleep(remaining.min(Duration::from_millis(5))); + std::thread::sleep(Duration::from_millis(1)); } - Err(_) => return None, + Err(error) => return Err(error), } } }