fix: remove windows-only panic when calling `Deno.kill` (#11948)

3 files changed, 90 insertions, 91 deletions

diff --git a/cli/tests/unit/process_test.ts b/cli/tests/unit/process_test.ts
index 73df3fa91..371521e33 100644
--- a/cli/tests/unit/process_test.ts
+++ b/cli/tests/unit/process_test.ts
@@ -448,11 +448,9 @@ unitTest(
 
     assert(
       error instanceof Deno.errors.NotFound ||
-        // 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.
+        // This is not yet implemented on Windows
         (Deno.build.os === "windows" &&
-          error instanceof Deno.errors.PermissionDenied),
+          error instanceof Error && error.message === "not implemented"),
     );
 
     p.close();
@@ -476,19 +474,23 @@ unitTest(
       cmd: [Deno.execPath(), "eval", "setTimeout(() => {}, 10000)"],
     });
 
-    Deno.kill(p.pid, "SIGINT");
-    const status = await p.status();
-
-    assertEquals(status.success, false);
     try {
-      assertEquals(status.signal, "SIGINT");
-    } catch {
-      // TODO(nayeemrmn): On Windows sometimes the following values are given
-      // instead. Investigate and remove this catch when fixed.
-      assertEquals(status.code, 130);
-      assertEquals(status.signal, 2);
+      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();
+
+        assertEquals(status.success, false);
+        assertEquals(status.code, 130);
+        assertEquals(status.signal, 2);
+      }
+    } finally {
+      p.close();
     }
-    p.close();
   },
 );
 
@@ -499,10 +501,11 @@ unitTest({ perms: { 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");
-  }, TypeError);
+  }, Deno.build.os === "windows" ? Error : TypeError);
 
   p.close();
 });
diff --git a/runtime/ops/process.rs b/runtime/ops/process.rs
index b1d5e80ca..b47492e58 100644
--- a/runtime/ops/process.rs
+++ b/runtime/ops/process.rs
@@ -295,7 +295,7 @@ fn op_kill(state: &mut OpState, args: KillArgs, _: ()) -> Result<(), AnyError> {
   super::check_unstable(state, "Deno.kill");
   state.borrow_mut::<Permissions>().run.check_all()?;
 
-  let signo = super::signal::signal_str_to_int_unwrap(&args.signo)?;
+  let signo = super::signal::signal_str_to_int(&args.signo)?;
   kill(args.pid, signo)?;
   Ok(())
 }
diff --git a/runtime/ops/signal.rs b/runtime/ops/signal.rs
index 84beaf3c8..cdc61b2e8 100644
--- a/runtime/ops/signal.rs
+++ b/runtime/ops/signal.rs
@@ -1,6 +1,7 @@
 // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.
 #[cfg(not(unix))]
 use deno_core::error::generic_error;
+#[cfg(not(target_os = "windows"))]
 use deno_core::error::type_error;
 use deno_core::error::AnyError;
 use deno_core::op_async_unref;
@@ -57,89 +58,84 @@ impl Resource for SignalStreamResource {
 }
 
 #[cfg(target_os = "linux")]
-fn signal_str_to_int(s: &str) -> Option<libc::c_int> {
+pub fn signal_str_to_int(s: &str) -> Result<libc::c_int, AnyError> {
   match s {
-    "SIGHUP" => Some(1),
-    "SIGINT" => Some(2),
-    "SIGQUIT" => Some(3),
-    "SIGILL" => Some(4),
-    "SIGTRAP" => Some(5),
-    "SIGABRT" => Some(6),
-    "SIGBUS" => Some(7),
-    "SIGFPE" => Some(8),
-    "SIGKILL" => Some(9),
-    "SIGUSR1" => Some(10),
-    "SIGSEGV" => Some(11),
-    "SIGUSR2" => Some(12),
-    "SIGPIPE" => Some(13),
-    "SIGALRM" => Some(14),
-    "SIGTERM" => Some(15),
-    "SIGSTKFLT" => Some(16),
-    "SIGCHLD" => Some(17),
-    "SIGCONT" => Some(18),
-    "SIGSTOP" => Some(19),
-    "SIGTSTP" => Some(20),
-    "SIGTTIN" => Some(21),
-    "SIGTTOU" => Some(22),
-    "SIGURG" => Some(23),
-    "SIGXCPU" => Some(24),
-    "SIGXFSZ" => Some(25),
-    "SIGVTALRM" => Some(26),
-    "SIGPROF" => Some(27),
-    "SIGWINCH" => Some(28),
-    "SIGIO" => Some(29),
-    "SIGPWR" => Some(30),
-    "SIGSYS" => Some(31),
-    _ => None,
+    "SIGHUP" => Ok(1),
+    "SIGINT" => Ok(2),
+    "SIGQUIT" => Ok(3),
+    "SIGILL" => Ok(4),
+    "SIGTRAP" => Ok(5),
+    "SIGABRT" => Ok(6),
+    "SIGBUS" => Ok(7),
+    "SIGFPE" => Ok(8),
+    "SIGKILL" => Ok(9),
+    "SIGUSR1" => Ok(10),
+    "SIGSEGV" => Ok(11),
+    "SIGUSR2" => Ok(12),
+    "SIGPIPE" => Ok(13),
+    "SIGALRM" => Ok(14),
+    "SIGTERM" => Ok(15),
+    "SIGSTKFLT" => Ok(16),
+    "SIGCHLD" => Ok(17),
+    "SIGCONT" => Ok(18),
+    "SIGSTOP" => Ok(19),
+    "SIGTSTP" => Ok(20),
+    "SIGTTIN" => Ok(21),
+    "SIGTTOU" => Ok(22),
+    "SIGURG" => Ok(23),
+    "SIGXCPU" => Ok(24),
+    "SIGXFSZ" => Ok(25),
+    "SIGVTALRM" => Ok(26),
+    "SIGPROF" => Ok(27),
+    "SIGWINCH" => Ok(28),
+    "SIGIO" => Ok(29),
+    "SIGPWR" => Ok(30),
+    "SIGSYS" => Ok(31),
+    _ => Err(type_error(format!("Invalid signal : {}", s))),
   }
 }
 
 #[cfg(target_os = "macos")]
-fn signal_str_to_int(s: &str) -> Option<libc::c_int> {
+pub fn signal_str_to_int(s: &str) -> Result<libc::c_int, AnyError> {
   match s {
-    "SIGHUP" => Some(1),
-    "SIGINT" => Some(2),
-    "SIGQUIT" => Some(3),
-    "SIGILL" => Some(4),
-    "SIGTRAP" => Some(5),
-    "SIGABRT" => Some(6),
-    "SIGEMT" => Some(7),
-    "SIGFPE" => Some(8),
-    "SIGKILL" => Some(9),
-    "SIGBUS" => Some(10),
-    "SIGSEGV" => Some(11),
-    "SIGSYS" => Some(12),
-    "SIGPIPE" => Some(13),
-    "SIGALRM" => Some(14),
-    "SIGTERM" => Some(15),
-    "SIGURG" => Some(16),
-    "SIGSTOP" => Some(17),
-    "SIGTSTP" => Some(18),
-    "SIGCONT" => Some(19),
-    "SIGCHLD" => Some(20),
-    "SIGTTIN" => Some(21),
-    "SIGTTOU" => Some(22),
-    "SIGIO" => Some(23),
-    "SIGXCPU" => Some(24),
-    "SIGXFSZ" => Some(25),
-    "SIGVTALRM" => Some(26),
-    "SIGPROF" => Some(27),
-    "SIGWINCH" => Some(28),
-    "SIGINFO" => Some(29),
-    "SIGUSR1" => Some(30),
-    "SIGUSR2" => Some(31),
-    _ => None,
+    "SIGHUP" => Ok(1),
+    "SIGINT" => Ok(2),
+    "SIGQUIT" => Ok(3),
+    "SIGILL" => Ok(4),
+    "SIGTRAP" => Ok(5),
+    "SIGABRT" => Ok(6),
+    "SIGEMT" => Ok(7),
+    "SIGFPE" => Ok(8),
+    "SIGKILL" => Ok(9),
+    "SIGBUS" => Ok(10),
+    "SIGSEGV" => Ok(11),
+    "SIGSYS" => Ok(12),
+    "SIGPIPE" => Ok(13),
+    "SIGALRM" => Ok(14),
+    "SIGTERM" => Ok(15),
+    "SIGURG" => Ok(16),
+    "SIGSTOP" => Ok(17),
+    "SIGTSTP" => Ok(18),
+    "SIGCONT" => Ok(19),
+    "SIGCHLD" => Ok(20),
+    "SIGTTIN" => Ok(21),
+    "SIGTTOU" => Ok(22),
+    "SIGIO" => Ok(23),
+    "SIGXCPU" => Ok(24),
+    "SIGXFSZ" => Ok(25),
+    "SIGVTALRM" => Ok(26),
+    "SIGPROF" => Ok(27),
+    "SIGWINCH" => Ok(28),
+    "SIGINFO" => Ok(29),
+    "SIGUSR1" => Ok(30),
+    "SIGUSR2" => Ok(31),
+    _ => Err(type_error(format!("Invalid signal : {}", s))),
   }
 }
 
 #[cfg(target_os = "windows")]
-fn signal_str_to_int(_s: &str) -> Option<libc::c_int> {
-  unimplemented!()
-}
-
-pub fn signal_str_to_int_unwrap(s: &str) -> Result<libc::c_int, AnyError> {
-  signal_str_to_int(s)
-    .ok_or_else(|| type_error(format!("Invalid signal : {}", s)))
+pub fn signal_str_to_int(_s: &str) -> Result<libc::c_int, AnyError> {
+  Err(generic_error("not implemented"))
 }
 
 #[cfg(unix)]
@@ -149,7 +145,7 @@ fn op_signal_bind(
   _: (),
 ) -> Result<ResourceId, AnyError> {
   super::check_unstable(state, "Deno.signal");
-  let signo = signal_str_to_int_unwrap(&sig)?;
+  let signo = signal_str_to_int(&sig)?;
   let resource = SignalStreamResource {
     signal: AsyncRefCell::new(signal(SignalKind::from_raw(signo)).unwrap()),
     cancel: Default::default(),