From 94b1424ed7cd8a10c8e16f48b2dd7c9bfe223d71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 18 Mar 2026 17:46:42 +0100 Subject: [PATCH 1/5] fix(ext/node): implement timeout and pid for child_process spawnSync Adds timeout support to `op_spawn_sync` so that Node's `spawnSync`, `execSync`, and `execFileSync` correctly kill the child process and return ETIMEDOUT when the timeout expires. Also returns pid from the sync spawn op so `spawnSync` results include the `pid` field. Co-Authored-By: Claude Opus 4.6 (1M context) --- ext/node/polyfills/internal/child_process.ts | 13 +++++ ext/process/40_process.js | 23 +++++++- ext/process/lib.rs | 55 ++++++++++++++++++++ tests/node_compat/config.jsonc | 2 + 4 files changed, 91 insertions(+), 2 deletions(-) diff --git a/ext/node/polyfills/internal/child_process.ts b/ext/node/polyfills/internal/child_process.ts index 66aba55ec586cf..31cdebacabb59f 100644 --- a/ext/node/polyfills/internal/child_process.ts +++ b/ext/node/polyfills/internal/child_process.ts @@ -74,8 +74,10 @@ import { kExtraStdio, kInputOption, kIpc, + kKillSignalOption, kNeedsNpmProcessState, kSerialization, + kTimeoutOption, } from "ext:deno_process/40_process.js"; export function mapValues( @@ -1634,6 +1636,8 @@ export function spawnSync( uid, gid, maxBuffer, + timeout, + killSignal, windowsVerbatimArguments = false, } = options; const [ @@ -1668,6 +1672,8 @@ export function spawnSync( // deno-lint-ignore no-explicit-any [kNeedsNpmProcessState]: (options as any)[kNeedsNpmProcessState] || includeNpmProcessState, + [kTimeoutOption]: timeout, + [kKillSignalOption]: killSignal, }).outputSync(); const status = output.signal ? null : output.code; @@ -1681,11 +1687,18 @@ export function spawnSync( result.error = _createSpawnError("ENOBUFS", command, args, true); } + // deno-lint-ignore no-explicit-any + if ((output as any).killedByTimeout) { + result.error = _createSpawnError("ETIMEDOUT", command, args, true); + } + if (encoding && encoding !== "buffer") { stdout = stdout && stdout.toString(encoding); stderr = stderr && stderr.toString(encoding); } + // deno-lint-ignore no-explicit-any + result.pid = (output as any).pid; result.status = status; result.signal = output.signal; result.stdout = stdout; diff --git a/ext/process/40_process.js b/ext/process/40_process.js index b1a2c5bf85ac50..3c0c161c389082 100644 --- a/ext/process/40_process.js +++ b/ext/process/40_process.js @@ -45,6 +45,10 @@ import { // The key for private `input` option for `Deno.Command` const kInputOption = Symbol("kInputOption"); +// The key for private `timeout` option for `Deno.Command` +const kTimeoutOption = Symbol("kTimeoutOption"); +// The key for private `killSignal` option for `Deno.Command` +const kKillSignalOption = Symbol("kKillSignalOption"); function opKill(pid, signo, apiName) { op_kill(pid, signo, apiName); @@ -471,13 +475,15 @@ function spawnSyncInner(command, { windowsRawArguments = false, [kInputOption]: input, [kNeedsNpmProcessState]: needsNpmProcessState = false, + [kTimeoutOption]: timeout, + [kKillSignalOption]: killSignal, } = { __proto__: null }) { if (stdin === "piped") { throw new TypeError( "Piped stdin is not supported for this function, use 'Deno.Command().spawn()' instead", ); } - const result = op_spawn_sync({ + const spawnArgs = { cmd: pathFromURL(command), args: ArrayPrototypeMap(args, String), cwd: pathFromURL(cwd), @@ -493,11 +499,22 @@ function spawnSyncInner(command, { detached: false, needsNpmProcessState, input, - }); + }; + if (timeout != null && timeout > 0) { + spawnArgs.timeout = timeout; + if (killSignal != null) { + spawnArgs.killSignal = typeof killSignal === "number" + ? String(killSignal) + : killSignal; + } + } + const result = op_spawn_sync(spawnArgs); return { + pid: result.pid, success: result.status.success, code: result.status.code, signal: result.status.signal, + killedByTimeout: result.killedByTimeout, get stdout() { if (result.stdout == null) { throw new TypeError("Cannot get 'stdout': 'stdout' is not piped"); @@ -597,6 +614,8 @@ export { Command, kill, kInputOption, + kKillSignalOption, + kTimeoutOption, Process, run, spawn, diff --git a/ext/process/lib.rs b/ext/process/lib.rs index 2090487b21df47..2ae9a63fc70d01 100644 --- a/ext/process/lib.rs +++ b/ext/process/lib.rs @@ -250,6 +250,11 @@ pub struct SpawnArgs { extra_stdio: Vec, detached: bool, needs_npm_process_state: bool, + + #[serde(default)] + timeout: Option, + #[serde(default)] + kill_signal: Option, } #[derive(Deserialize)] @@ -393,9 +398,11 @@ impl TryFrom for ChildStatus { #[derive(ToV8)] pub struct SpawnOutput { + pid: u32, status: ChildStatus, stdout: Option, stderr: Option, + killed_by_timeout: bool, } type CreateCommand = ( @@ -1110,6 +1117,8 @@ fn op_spawn_sync( let stdout = matches!(args.stdio.stdout, StdioOrRid::Stdio(Stdio::Piped)); let stderr = matches!(args.stdio.stderr, StdioOrRid::Stdio(Stdio::Piped)); let input = args.input.clone(); + let timeout = args.timeout; + let kill_signal_str = args.kill_signal.clone(); let (mut command, _, _, _) = create_command(state, args, "Deno.Command().outputSync()")?; @@ -1117,6 +1126,7 @@ fn op_spawn_sync( command: command.get_program().to_string_lossy().into_owned(), error: Box::new(e.into()), })?; + let pid = child.id(); if let Some(input) = input { let mut stdin = child.stdin.take().ok_or_else(|| { ProcessError::Io(std::io::Error::other("stdin is not available")) @@ -1124,6 +1134,48 @@ fn op_spawn_sync( stdin.write_all(&input)?; stdin.flush()?; } + + // If timeout is specified, spawn a thread that will kill the child + // after the timeout expires. + let killed_by_timeout = + std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); + if let Some(timeout_ms) = timeout { + if timeout_ms > 0 { + let child_id = child.id(); + let killed = killed_by_timeout.clone(); + let kill_signal = kill_signal_str.as_deref().unwrap_or("SIGTERM"); + #[cfg(unix)] + let signal = + deno_signals::signal_str_to_int(kill_signal).unwrap_or(libc::SIGTERM); + std::thread::spawn(move || { + std::thread::sleep(std::time::Duration::from_millis(timeout_ms)); + killed.store(true, std::sync::atomic::Ordering::SeqCst); + #[cfg(unix)] + unsafe { + libc::kill(child_id as i32, signal); + } + #[cfg(windows)] + { + // On Windows, use TerminateProcess via the child handle + // We can't easily signal, so just kill the process + unsafe { + let handle = windows_sys::Win32::System::Threading::OpenProcess( + windows_sys::Win32::System::Threading::PROCESS_TERMINATE, + 0, + child_id, + ); + if handle != 0 { + windows_sys::Win32::System::Threading::TerminateProcess( + handle, 1, + ); + windows_sys::Win32::Foundation::CloseHandle(handle); + } + } + } + }); + } + } + let output = child .wait_with_output() @@ -1131,7 +1183,9 @@ fn op_spawn_sync( command: command.get_program().to_string_lossy().into_owned(), error: Box::new(e.into()), })?; + let timed_out = killed_by_timeout.load(std::sync::atomic::Ordering::SeqCst); Ok(SpawnOutput { + pid, status: output.status.try_into()?, stdout: if stdout { Some(output.stdout.into()) @@ -1143,6 +1197,7 @@ fn op_spawn_sync( } else { None }, + killed_by_timeout: timed_out, }) } diff --git a/tests/node_compat/config.jsonc b/tests/node_compat/config.jsonc index a0edad85054ee6..53790bc99179d1 100644 --- a/tests/node_compat/config.jsonc +++ b/tests/node_compat/config.jsonc @@ -253,6 +253,7 @@ "parallel/test-child-process-spawnsync-env.js": {}, "parallel/test-child-process-spawnsync-input.js": {}, "parallel/test-child-process-spawnsync-maxbuf.js": {}, + "parallel/test-child-process-spawnsync-timeout.js": {}, "parallel/test-child-process-spawnsync-validation-errors.js": {}, "parallel/test-child-process-spawnsync.js": {}, "parallel/test-child-process-stdin-ipc.js": {}, @@ -2524,6 +2525,7 @@ // "pummel/test-heapdump-inspector.js": {}, "pummel/test-string-decoder-large-buffer.js": {}, "sequential/test-buffer-creation-regression.js": {}, + "sequential/test-child-process-execsync.js": {}, "sequential/test-child-process-exit.js": {}, "sequential/test-cli-syntax-bad.js": {}, "sequential/test-cli-syntax-file-not-found.js": { From f10d04276feedb97fd9dde39d9a37bfae03d2c67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Fri, 20 Mar 2026 17:20:30 +0100 Subject: [PATCH 2/5] fix: address review feedback for spawnSync timeout - Hide internal `pid` and `killedByTimeout` fields from public Deno.Command().outputSync() API by prefixing with _ and stripping in outputSync() - Replace fire-and-forget timeout thread with condvar-based cancellation so the thread exits promptly when the child finishes before the deadline - Clean up use statements Co-Authored-By: Claude Opus 4.6 (1M context) --- ext/node/polyfills/internal/child_process.ts | 4 +- ext/process/40_process.js | 12 +++- ext/process/lib.rs | 59 +++++++++++++------- 3 files changed, 49 insertions(+), 26 deletions(-) diff --git a/ext/node/polyfills/internal/child_process.ts b/ext/node/polyfills/internal/child_process.ts index 31cdebacabb59f..a21b47e9944ea7 100644 --- a/ext/node/polyfills/internal/child_process.ts +++ b/ext/node/polyfills/internal/child_process.ts @@ -1688,7 +1688,7 @@ export function spawnSync( } // deno-lint-ignore no-explicit-any - if ((output as any).killedByTimeout) { + if ((output as any)._killedByTimeout) { result.error = _createSpawnError("ETIMEDOUT", command, args, true); } @@ -1698,7 +1698,7 @@ export function spawnSync( } // deno-lint-ignore no-explicit-any - result.pid = (output as any).pid; + result.pid = (output as any)._pid; result.status = status; result.signal = output.signal; result.stdout = stdout; diff --git a/ext/process/40_process.js b/ext/process/40_process.js index 3c0c161c389082..af698249012acb 100644 --- a/ext/process/40_process.js +++ b/ext/process/40_process.js @@ -510,11 +510,13 @@ function spawnSyncInner(command, { } const result = op_spawn_sync(spawnArgs); return { - pid: result.pid, + // Internal fields used by node:child_process but not exposed via Deno API. + // outputSync() below strips these before returning. + _pid: result.pid, + _killedByTimeout: result.killedByTimeout, success: result.status.success, code: result.status.code, signal: result.status.signal, - killedByTimeout: result.killedByTimeout, get stdout() { if (result.stdout == null) { throw new TypeError("Cannot get 'stdout': 'stdout' is not piped"); @@ -554,7 +556,11 @@ class Command { "Piped stdin is not supported for this function, use 'Deno.Command.spawn()' instead", ); } - return spawnSyncInner(this.#command, this.#options); + const output = spawnSyncInner(this.#command, this.#options); + // Strip internal fields that are not part of the Deno API. + delete output._pid; + delete output._killedByTimeout; + return output; } spawn() { diff --git a/ext/process/lib.rs b/ext/process/lib.rs index 2ae9a63fc70d01..f5d06c2bc98147 100644 --- a/ext/process/lib.rs +++ b/ext/process/lib.rs @@ -18,6 +18,11 @@ use std::process::ExitStatus; #[cfg(unix)] use std::process::Stdio as StdStdio; use std::rc::Rc; +use std::sync::atomic::AtomicBool; +use std::sync::atomic::Ordering; +use std::sync::Arc; +use std::sync::Condvar; +use std::sync::Mutex; use deno_core::AsyncMutFuture; use deno_core::AsyncRefCell; @@ -1136,40 +1141,43 @@ fn op_spawn_sync( } // If timeout is specified, spawn a thread that will kill the child - // after the timeout expires. - let killed_by_timeout = - std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); + // after the timeout expires. Uses a condvar so the timer thread can be + // cancelled promptly when the child exits before the deadline. + let killed_by_timeout = Arc::new(AtomicBool::new(false)); + let cancel = Arc::new((Mutex::new(false), Condvar::new())); if let Some(timeout_ms) = timeout { if timeout_ms > 0 { let child_id = child.id(); let killed = killed_by_timeout.clone(); + let cancel2 = cancel.clone(); let kill_signal = kill_signal_str.as_deref().unwrap_or("SIGTERM"); #[cfg(unix)] let signal = deno_signals::signal_str_to_int(kill_signal).unwrap_or(libc::SIGTERM); std::thread::spawn(move || { - std::thread::sleep(std::time::Duration::from_millis(timeout_ms)); - killed.store(true, std::sync::atomic::Ordering::SeqCst); + let (lock, cvar) = &*cancel2; + let guard = lock.lock().unwrap(); + let timeout = std::time::Duration::from_millis(timeout_ms); + let (guard, _) = cvar.wait_timeout(guard, timeout).unwrap(); + // If cancelled, the child already exited — don't kill. + if *guard { + return; + } + killed.store(true, Ordering::SeqCst); #[cfg(unix)] unsafe { libc::kill(child_id as i32, signal); } #[cfg(windows)] - { - // On Windows, use TerminateProcess via the child handle - // We can't easily signal, so just kill the process - unsafe { - let handle = windows_sys::Win32::System::Threading::OpenProcess( - windows_sys::Win32::System::Threading::PROCESS_TERMINATE, - 0, - child_id, - ); - if handle != 0 { - windows_sys::Win32::System::Threading::TerminateProcess( - handle, 1, - ); - windows_sys::Win32::Foundation::CloseHandle(handle); - } + unsafe { + let handle = windows_sys::Win32::System::Threading::OpenProcess( + windows_sys::Win32::System::Threading::PROCESS_TERMINATE, + 0, + child_id, + ); + if handle != 0 { + windows_sys::Win32::System::Threading::TerminateProcess(handle, 1); + windows_sys::Win32::Foundation::CloseHandle(handle); } } }); @@ -1183,7 +1191,16 @@ fn op_spawn_sync( command: command.get_program().to_string_lossy().into_owned(), error: Box::new(e.into()), })?; - let timed_out = killed_by_timeout.load(std::sync::atomic::Ordering::SeqCst); + + // Cancel the timeout thread if it's still waiting. + { + let (lock, cvar) = &*cancel; + let mut cancelled = lock.lock().unwrap(); + *cancelled = true; + cvar.notify_one(); + } + + let timed_out = killed_by_timeout.load(Ordering::SeqCst); Ok(SpawnOutput { pid, status: output.status.try_into()?, From e77a90008b09b04daada95cabf71b0d4a1454831 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Fri, 20 Mar 2026 17:38:52 +0100 Subject: [PATCH 3/5] fix: address Copilot review feedback - Use wait_timeout_while instead of wait_timeout to guard against spurious condvar wakeups that could kill the child prematurely - Support numeric killSignal values (e.g. 9) by falling back to parse::() when signal_str_to_int doesn't recognize the string - Make _pid and _killedByTimeout non-enumerable on the output object instead of deleting them in outputSync(), so the node:child_process polyfill can still access them Co-Authored-By: Claude Opus 4.6 (1M context) --- ext/process/40_process.js | 24 ++++++++++++++---------- ext/process/lib.rs | 14 +++++++++----- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/ext/process/40_process.js b/ext/process/40_process.js index af698249012acb..139760f4f82aba 100644 --- a/ext/process/40_process.js +++ b/ext/process/40_process.js @@ -509,11 +509,7 @@ function spawnSyncInner(command, { } } const result = op_spawn_sync(spawnArgs); - return { - // Internal fields used by node:child_process but not exposed via Deno API. - // outputSync() below strips these before returning. - _pid: result.pid, - _killedByTimeout: result.killedByTimeout, + const output = { success: result.status.success, code: result.status.code, signal: result.status.signal, @@ -530,6 +526,18 @@ function spawnSyncInner(command, { return result.stderr; }, }; + // Internal fields used by node:child_process, hidden from Deno public API. + Object.defineProperty(output, "_pid", { + __proto__: null, + value: result.pid, + enumerable: false, + }); + Object.defineProperty(output, "_killedByTimeout", { + __proto__: null, + value: result.killedByTimeout, + enumerable: false, + }); + return output; } class Command { @@ -556,11 +564,7 @@ class Command { "Piped stdin is not supported for this function, use 'Deno.Command.spawn()' instead", ); } - const output = spawnSyncInner(this.#command, this.#options); - // Strip internal fields that are not part of the Deno API. - delete output._pid; - delete output._killedByTimeout; - return output; + return spawnSyncInner(this.#command, this.#options); } spawn() { diff --git a/ext/process/lib.rs b/ext/process/lib.rs index f5d06c2bc98147..25850069b0c6d7 100644 --- a/ext/process/lib.rs +++ b/ext/process/lib.rs @@ -1152,15 +1152,19 @@ fn op_spawn_sync( let cancel2 = cancel.clone(); let kill_signal = kill_signal_str.as_deref().unwrap_or("SIGTERM"); #[cfg(unix)] - let signal = - deno_signals::signal_str_to_int(kill_signal).unwrap_or(libc::SIGTERM); + let signal = deno_signals::signal_str_to_int(kill_signal) + .ok() + .or_else(|| kill_signal.parse::().ok()) + .unwrap_or(libc::SIGTERM); std::thread::spawn(move || { let (lock, cvar) = &*cancel2; let guard = lock.lock().unwrap(); let timeout = std::time::Duration::from_millis(timeout_ms); - let (guard, _) = cvar.wait_timeout(guard, timeout).unwrap(); - // If cancelled, the child already exited — don't kill. - if *guard { + let (guard, wait_result) = cvar + .wait_timeout_while(guard, timeout, |cancelled| !*cancelled) + .unwrap(); + // If cancelled or woken before the timeout, the child already exited. + if *guard || !wait_result.timed_out() { return; } killed.store(true, Ordering::SeqCst); From 776196844f742a91f28b045aef8f3e9ce8e394d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Fri, 20 Mar 2026 18:00:31 +0100 Subject: [PATCH 4/5] fix: address CI failures for spawnSync timeout PR 1. Fix Windows build: use 0i32 for BOOL arg in OpenProcess, handle HANDLE type correctly (isize, not *mut c_void) 2. Fix child.id() cross-platform: add std_child_id() helper for Windows where Child::id() may return Option 3. Fix import ordering: rustfmt reorders std::sync::atomic imports 4. Fix JS lint: use ObjectDefineProperty instead of Object.defineProperty Co-Authored-By: Claude Opus 4.6 (1M context) --- ext/process/40_process.js | 5 +++-- ext/process/lib.rs | 23 ++++++++++++++++++----- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/ext/process/40_process.js b/ext/process/40_process.js index 139760f4f82aba..d0785774802c01 100644 --- a/ext/process/40_process.js +++ b/ext/process/40_process.js @@ -17,6 +17,7 @@ const { ArrayPrototypeMap, ArrayPrototypeSlice, TypeError, + ObjectDefineProperty, ObjectEntries, SafeArrayIterator, String, @@ -527,12 +528,12 @@ function spawnSyncInner(command, { }, }; // Internal fields used by node:child_process, hidden from Deno public API. - Object.defineProperty(output, "_pid", { + ObjectDefineProperty(output, "_pid", { __proto__: null, value: result.pid, enumerable: false, }); - Object.defineProperty(output, "_killedByTimeout", { + ObjectDefineProperty(output, "_killedByTimeout", { __proto__: null, value: result.killedByTimeout, enumerable: false, diff --git a/ext/process/lib.rs b/ext/process/lib.rs index 25850069b0c6d7..6a621d4cd69f61 100644 --- a/ext/process/lib.rs +++ b/ext/process/lib.rs @@ -18,11 +18,11 @@ use std::process::ExitStatus; #[cfg(unix)] use std::process::Stdio as StdStdio; use std::rc::Rc; -use std::sync::atomic::AtomicBool; -use std::sync::atomic::Ordering; use std::sync::Arc; use std::sync::Condvar; use std::sync::Mutex; +use std::sync::atomic::AtomicBool; +use std::sync::atomic::Ordering; use deno_core::AsyncMutFuture; use deno_core::AsyncRefCell; @@ -1114,6 +1114,19 @@ async fn op_spawn_wait( Ok(result) } +/// Helper to get child process ID as u32. On Windows with some Rust +/// versions, `Child::id()` may return `Option`. +fn std_child_id(child: &std::process::Child) -> u32 { + #[cfg(windows)] + { + child.id().unwrap_or(0) + } + #[cfg(not(windows))] + { + child.id() + } +} + #[op2(stack_trace)] fn op_spawn_sync( state: &mut OpState, @@ -1131,7 +1144,7 @@ fn op_spawn_sync( command: command.get_program().to_string_lossy().into_owned(), error: Box::new(e.into()), })?; - let pid = child.id(); + let pid = std_child_id(&child); if let Some(input) = input { let mut stdin = child.stdin.take().ok_or_else(|| { ProcessError::Io(std::io::Error::other("stdin is not available")) @@ -1147,7 +1160,7 @@ fn op_spawn_sync( let cancel = Arc::new((Mutex::new(false), Condvar::new())); if let Some(timeout_ms) = timeout { if timeout_ms > 0 { - let child_id = child.id(); + let child_id = std_child_id(&child); let killed = killed_by_timeout.clone(); let cancel2 = cancel.clone(); let kill_signal = kill_signal_str.as_deref().unwrap_or("SIGTERM"); @@ -1176,7 +1189,7 @@ fn op_spawn_sync( unsafe { let handle = windows_sys::Win32::System::Threading::OpenProcess( windows_sys::Win32::System::Threading::PROCESS_TERMINATE, - 0, + 0i32, child_id, ); if handle != 0 { From b5cb14ea4b4b4e723b093067ef44ce25df9fea19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Fri, 20 Mar 2026 18:19:54 +0100 Subject: [PATCH 5/5] fix: address remaining CI failures 1. Remove std_child_id helper - on Windows the child type is deno_subprocess_windows::Child, not std::process::Child. Both types return u32 from id() so no helper needed. 2. Fix Windows HANDLE: use !handle.is_null() instead of handle != 0 (HANDLE is *mut c_void in windows-sys 0.59, use false.into() for BOOL parameter) 3. Collapse nested if (clippy collapsible_if) 4. Add SAFETY comments on unsafe blocks (clippy undocumented_unsafe_blocks) Co-Authored-By: Claude Opus 4.6 (1M context) --- ext/process/lib.rs | 97 +++++++++++++++++++++------------------------- 1 file changed, 44 insertions(+), 53 deletions(-) diff --git a/ext/process/lib.rs b/ext/process/lib.rs index 6a621d4cd69f61..d2c69b68e9071a 100644 --- a/ext/process/lib.rs +++ b/ext/process/lib.rs @@ -1114,19 +1114,6 @@ async fn op_spawn_wait( Ok(result) } -/// Helper to get child process ID as u32. On Windows with some Rust -/// versions, `Child::id()` may return `Option`. -fn std_child_id(child: &std::process::Child) -> u32 { - #[cfg(windows)] - { - child.id().unwrap_or(0) - } - #[cfg(not(windows))] - { - child.id() - } -} - #[op2(stack_trace)] fn op_spawn_sync( state: &mut OpState, @@ -1144,7 +1131,7 @@ fn op_spawn_sync( command: command.get_program().to_string_lossy().into_owned(), error: Box::new(e.into()), })?; - let pid = std_child_id(&child); + let pid = child.id(); if let Some(input) = input { let mut stdin = child.stdin.take().ok_or_else(|| { ProcessError::Io(std::io::Error::other("stdin is not available")) @@ -1158,47 +1145,51 @@ fn op_spawn_sync( // cancelled promptly when the child exits before the deadline. let killed_by_timeout = Arc::new(AtomicBool::new(false)); let cancel = Arc::new((Mutex::new(false), Condvar::new())); - if let Some(timeout_ms) = timeout { - if timeout_ms > 0 { - let child_id = std_child_id(&child); - let killed = killed_by_timeout.clone(); - let cancel2 = cancel.clone(); - let kill_signal = kill_signal_str.as_deref().unwrap_or("SIGTERM"); + if let Some(timeout_ms) = timeout + && timeout_ms > 0 + { + let child_id = child.id(); + let killed = killed_by_timeout.clone(); + let cancel2 = cancel.clone(); + let kill_signal = kill_signal_str.as_deref().unwrap_or("SIGTERM"); + #[cfg(unix)] + let signal = deno_signals::signal_str_to_int(kill_signal) + .ok() + .or_else(|| kill_signal.parse::().ok()) + .unwrap_or(libc::SIGTERM); + std::thread::spawn(move || { + let (lock, cvar) = &*cancel2; + let guard = lock.lock().unwrap(); + let timeout = std::time::Duration::from_millis(timeout_ms); + let (guard, wait_result) = cvar + .wait_timeout_while(guard, timeout, |cancelled| !*cancelled) + .unwrap(); + // If cancelled or woken before the timeout, the child already exited. + if *guard || !wait_result.timed_out() { + return; + } + killed.store(true, Ordering::SeqCst); #[cfg(unix)] - let signal = deno_signals::signal_str_to_int(kill_signal) - .ok() - .or_else(|| kill_signal.parse::().ok()) - .unwrap_or(libc::SIGTERM); - std::thread::spawn(move || { - let (lock, cvar) = &*cancel2; - let guard = lock.lock().unwrap(); - let timeout = std::time::Duration::from_millis(timeout_ms); - let (guard, wait_result) = cvar - .wait_timeout_while(guard, timeout, |cancelled| !*cancelled) - .unwrap(); - // If cancelled or woken before the timeout, the child already exited. - if *guard || !wait_result.timed_out() { - return; - } - killed.store(true, Ordering::SeqCst); - #[cfg(unix)] - unsafe { - libc::kill(child_id as i32, signal); - } - #[cfg(windows)] - unsafe { - let handle = windows_sys::Win32::System::Threading::OpenProcess( - windows_sys::Win32::System::Threading::PROCESS_TERMINATE, - 0i32, - child_id, - ); - if handle != 0 { - windows_sys::Win32::System::Threading::TerminateProcess(handle, 1); - windows_sys::Win32::Foundation::CloseHandle(handle); - } + // SAFETY: child_id is a valid PID from the spawned child process. + unsafe { + libc::kill(child_id as i32, signal); + } + #[cfg(windows)] + // SAFETY: child_id is a valid PID from the spawned child process. + // OpenProcess/TerminateProcess/CloseHandle are safe to call with + // valid arguments. + unsafe { + let handle = windows_sys::Win32::System::Threading::OpenProcess( + windows_sys::Win32::System::Threading::PROCESS_TERMINATE, + false.into(), + child_id, + ); + if !handle.is_null() { + windows_sys::Win32::System::Threading::TerminateProcess(handle, 1); + windows_sys::Win32::Foundation::CloseHandle(handle); } - }); - } + } + }); } let output =