diff --git a/cli/tests/unit/process_test.ts b/cli/tests/unit/process_test.ts index 9012274c38..12946656de 100644 --- a/cli/tests/unit/process_test.ts +++ b/cli/tests/unit/process_test.ts @@ -114,8 +114,6 @@ unitTest( unitTest( { - // No signals on windows. - ignore: Deno.build.os === "windows", permissions: { run: true, read: true }, }, async function runCommandFailedWithSignal() { @@ -129,8 +127,13 @@ unitTest( }); const status = await p.status(); assertEquals(status.success, false); - assertEquals(status.code, 128 + 9); - assertEquals(status.signal, 9); + if (Deno.build.os === "windows") { + assertEquals(status.code, 1); + assertEquals(status.signal, undefined); + } else { + assertEquals(status.code, 128 + 9); + assertEquals(status.signal, 9); + } p.close(); }, ); @@ -448,9 +451,11 @@ unitTest( assert( error instanceof Deno.errors.NotFound || - // This is not yet implemented on Windows + // On Windows, the underlying Windows API may return + // `ERROR_ACCESS_DENIED` when the process has exited, but hasn't been + // completely cleaned up yet and its `pid` is still valid. (Deno.build.os === "windows" && - error instanceof Error && error.message === "not implemented"), + error instanceof Deno.errors.PermissionDenied), ); p.close(); @@ -467,6 +472,24 @@ unitTest(function killPermissions() { }, Deno.errors.PermissionDenied); }); +unitTest( + { ignore: Deno.build.os !== "windows", permissions: { run: true } }, + function negativePidInvalidWindows() { + assertThrows(() => { + Deno.kill(-1, "SIGINT"); + }, TypeError); + }, +); + +unitTest( + { ignore: Deno.build.os !== "windows", permissions: { run: true } }, + function invalidSignalNameWindows() { + assertThrows(() => { + Deno.kill(Deno.pid, "SIGUSR1"); + }, TypeError); + }, +); + unitTest( { permissions: { run: true, read: true } }, async function killSuccess() { @@ -475,16 +498,14 @@ unitTest( }); try { - if (Deno.build.os === "windows") { - // currently not implemented - assertThrows(() => { - Deno.kill(p.pid, "SIGINT"); - }, Error); - } else { - Deno.kill(p.pid, "SIGINT"); - const status = await p.status(); + Deno.kill(p.pid, "SIGINT"); + const status = await p.status(); - assertEquals(status.success, false); + assertEquals(status.success, false); + if (Deno.build.os === "windows") { + assertEquals(status.code, 1); + assertEquals(status.signal, undefined); + } else { assertEquals(status.code, 130); assertEquals(status.signal, 2); } @@ -501,11 +522,10 @@ unitTest({ permissions: { run: true, read: true } }, function killFailed() { assert(!p.stdin); assert(!p.stdout); - // windows is currently not implemented so it throws a regular Error saying so assertThrows(() => { // @ts-expect-error testing runtime error of bad signal Deno.kill(p.pid, "foobar"); - }, Deno.build.os === "windows" ? Error : TypeError); + }, TypeError); p.close(); }); diff --git a/runtime/ops/process.rs b/runtime/ops/process.rs index 1f7280d48f..7b01916b4d 100644 --- a/runtime/ops/process.rs +++ b/runtime/ops/process.rs @@ -257,7 +257,8 @@ async fn op_run_status( } #[cfg(unix)] -pub fn kill(pid: i32, signo: i32) -> Result<(), AnyError> { +pub fn kill(pid: i32, signal: &str) -> Result<(), AnyError> { + let signo = super::signal::signal_str_to_int(signal)?; use nix::sys::signal::{kill as unix_kill, Signal}; use nix::unistd::Pid; use std::convert::TryFrom; @@ -266,7 +267,7 @@ pub fn kill(pid: i32, signo: i32) -> Result<(), AnyError> { } #[cfg(not(unix))] -pub fn kill(pid: i32, signal: i32) -> Result<(), AnyError> { +pub fn kill(pid: i32, signal: &str) -> Result<(), AnyError> { use std::io::Error; use std::io::ErrorKind::NotFound; use winapi::shared::minwindef::DWORD; @@ -279,14 +280,10 @@ pub fn kill(pid: i32, signal: i32) -> Result<(), AnyError> { use winapi::um::processthreadsapi::TerminateProcess; use winapi::um::winnt::PROCESS_TERMINATE; - const SIGINT: i32 = 2; - const SIGKILL: i32 = 9; - const SIGTERM: i32 = 15; - - if !matches!(signal, SIGINT | SIGKILL | SIGTERM) { - Err(type_error("unsupported signal")) + if !matches!(signal, "SIGINT" | "SIGKILL" | "SIGTERM") { + Err(type_error(format!("Invalid signal: {}", signal))) } else if pid <= 0 { - Err(type_error("unsupported pid")) + Err(type_error("Invalid pid")) } else { let handle = unsafe { OpenProcess(PROCESS_TERMINATE, FALSE, pid as DWORD) }; if handle.is_null() { @@ -310,12 +307,10 @@ pub fn kill(pid: i32, signal: i32) -> Result<(), AnyError> { fn op_kill( state: &mut OpState, pid: i32, - signo: String, + signal: String, ) -> Result<(), AnyError> { super::check_unstable(state, "Deno.kill"); state.borrow_mut::().run.check_all()?; - - let signo = super::signal::signal_str_to_int(&signo)?; - kill(pid, signo)?; + kill(pid, &signal)?; Ok(()) } diff --git a/runtime/ops/signal.rs b/runtime/ops/signal.rs index 60d7070279..eef72f5112 100644 --- a/runtime/ops/signal.rs +++ b/runtime/ops/signal.rs @@ -169,15 +169,10 @@ pub fn signal_str_to_int(s: &str) -> Result { "SIGINFO" => Ok(29), "SIGUSR1" => Ok(30), "SIGUSR2" => Ok(31), - _ => Err(type_error(format!("Invalid signal : {}", s))), + _ => Err(type_error(format!("Invalid signal: {}", s))), } } -#[cfg(target_os = "windows")] -pub fn signal_str_to_int(_s: &str) -> Result { - Err(generic_error("not implemented")) -} - #[cfg(unix)] fn op_signal_bind( state: &mut OpState,