From a9b34118a9338323532c3b6b2e0336c343a0e834 Mon Sep 17 00:00:00 2001
From: Yoshiya Hinosawa <stibium121@gmail.com>
Date: Tue, 26 Oct 2021 12:03:38 +0900
Subject: [PATCH] feat(runtime): add Deno.addSignalListener API (#12512)

---
 cli/dts/lib.deno.unstable.d.ts |  67 ++++-------
 cli/tests/unit/signal_test.ts  | 198 ++++++++++++++-------------------
 runtime/js/40_signals.js       | 115 ++++++++-----------
 runtime/js/90_deno_ns.js       |   5 +-
 4 files changed, 157 insertions(+), 228 deletions(-)

diff --git a/cli/dts/lib.deno.unstable.d.ts b/cli/dts/lib.deno.unstable.d.ts
index fd33e1a74f..a84883574c 100644
--- a/cli/dts/lib.deno.unstable.d.ts
+++ b/cli/dts/lib.deno.unstable.d.ts
@@ -566,56 +566,37 @@ declare namespace Deno {
 
   /** **UNSTABLE**: new API, yet to be vetted.
    *
-   * Represents the stream of signals, implements both `AsyncIterator` and
-   * `PromiseLike`. */
-  export class SignalStream
-    implements AsyncIterableIterator<void>, PromiseLike<void> {
-    constructor(signal: Signal);
-    then<T, S>(
-      f: (v: void) => T | Promise<T>,
-      g?: (v: void) => S | Promise<S>,
-    ): Promise<T | S>;
-    next(): Promise<IteratorResult<void>>;
-    [Symbol.asyncIterator](): AsyncIterableIterator<void>;
-    dispose(): void;
-  }
-
-  /** **UNSTABLE**: new API, yet to be vetted.
-   *
-   * Returns the stream of the given signal number. You can use it as an async
-   * iterator.
+   * Registers the given function as a listener of the given signal event.
    *
    * ```ts
-   * for await (const _ of Deno.signal("SIGTERM")) {
-   *   console.log("got SIGTERM!");
-   * }
-   * ```
-   *
-   * You can also use it as a promise. In this case you can only receive the
-   * first one.
-   *
-   * ```ts
-   * await Deno.signal("SIGTERM");
-   * console.log("SIGTERM received!")
-   * ```
-   *
-   * If you want to stop receiving the signals, you can use `.dispose()` method
-   * of the signal stream object.
-   *
-   * ```ts
-   * const sig = Deno.signal("SIGTERM");
-   * setTimeout(() => { sig.dispose(); }, 5000);
-   * for await (const _ of sig) {
+   * Deno.addSignalListener("SIGTERM", () => {
    *   console.log("SIGTERM!")
-   * }
+   * });
    * ```
    *
-   * The above for-await loop exits after 5 seconds when `sig.dispose()` is
-   * called.
-   *
    * NOTE: This functionality is not yet implemented on Windows.
    */
-  export function signal(sig: Signal): SignalStream;
+  export function addSignalListener(signal: Signal, handler: () => void): void;
+
+  /** **UNSTABLE**: new API, yet to be vetted.
+   *
+   * Removes the given signal listener that has been registered with
+   * Deno.addSignalListener.
+   *
+   * ```ts
+   * const listener = () => {
+   *   console.log("SIGTERM!")
+   * };
+   * Deno.addSignalListener("SIGTERM", listener);
+   * Deno.removeSignalListener("SIGTERM", listener);
+   * ```
+   *
+   * NOTE: This functionality is not yet implemented on Windows.
+   */
+  export function removeSignalListener(
+    signal: Signal,
+    handler: () => void,
+  ): void;
 
   export type SetRawOptions = {
     cbreak: boolean;
diff --git a/cli/tests/unit/signal_test.ts b/cli/tests/unit/signal_test.ts
index 1f099a0c4e..d567b69fc8 100644
--- a/cli/tests/unit/signal_test.ts
+++ b/cli/tests/unit/signal_test.ts
@@ -1,6 +1,5 @@
 // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.
 import {
-  assert,
   assertEquals,
   assertThrows,
   deferred,
@@ -13,84 +12,84 @@ unitTest(
   function signalsNotImplemented() {
     assertThrows(
       () => {
-        Deno.signal("SIGINT");
+        Deno.addSignalListener("SIGINT", () => {});
       },
       Error,
       "not implemented",
     );
     assertThrows(
       () => {
-        Deno.signal("SIGALRM");
+        Deno.addSignalListener("SIGALRM", () => {});
       },
       Error,
       "not implemented",
     );
     assertThrows(
       () => {
-        Deno.signal("SIGCHLD");
+        Deno.addSignalListener("SIGCHLD", () => {});
       },
       Error,
       "not implemented",
     );
     assertThrows(
       () => {
-        Deno.signal("SIGHUP");
+        Deno.addSignalListener("SIGHUP", () => {});
       },
       Error,
       "not implemented",
     );
     assertThrows(
       () => {
-        Deno.signal("SIGINT");
+        Deno.addSignalListener("SIGINT", () => {});
       },
       Error,
       "not implemented",
     );
     assertThrows(
       () => {
-        Deno.signal("SIGIO");
+        Deno.addSignalListener("SIGIO", () => {});
       },
       Error,
       "not implemented",
     );
     assertThrows(
       () => {
-        Deno.signal("SIGPIPE");
+        Deno.addSignalListener("SIGPIPE", () => {});
       },
       Error,
       "not implemented",
     );
     assertThrows(
       () => {
-        Deno.signal("SIGQUIT");
+        Deno.addSignalListener("SIGQUIT", () => {});
       },
       Error,
       "not implemented",
     );
     assertThrows(
       () => {
-        Deno.signal("SIGTERM");
+        Deno.addSignalListener("SIGTERM", () => {});
       },
       Error,
       "not implemented",
     );
     assertThrows(
       () => {
-        Deno.signal("SIGUSR1");
+        Deno.addSignalListener("SIGUSR1", () => {});
       },
       Error,
       "not implemented",
     );
     assertThrows(
       () => {
-        Deno.signal("SIGUSR2");
+        Deno.addSignalListener("SIGUSR2", () => {});
       },
       Error,
       "not implemented",
     );
     assertThrows(
       () => {
-        Deno.signal("SIGWINCH");
+        Deno.addSignalListener("SIGWINCH", () => {});
       },
       Error,
       "not implemented",
@@ -101,34 +100,75 @@ unitTest(
 unitTest(
   {
     ignore: Deno.build.os === "windows",
-    permissions: { run: true, net: true },
+    permissions: { run: true },
   },
-  async function signalStreamTest() {
+  async function signalListenerTest() {
     const resolvable = deferred();
-    // This prevents the program from exiting.
-    const t = setInterval(() => {}, 1000);
-
     let c = 0;
-    const sig = Deno.signal("SIGUSR1");
+    const listener = () => {
+      c += 1;
+    };
+    Deno.addSignalListener("SIGUSR1", listener);
     setTimeout(async () => {
-      await delay(20);
+      // Sends SIGUSR1 3 times.
       for (const _ of Array(3)) {
-        // Sends SIGUSR1 3 times.
-        Deno.kill(Deno.pid, "SIGUSR1");
         await delay(20);
+        Deno.kill(Deno.pid, "SIGUSR1");
       }
-      sig.dispose();
+      await delay(20);
+      Deno.removeSignalListener("SIGUSR1", listener);
       resolvable.resolve();
     });
 
-    for await (const _ of sig) {
-      c += 1;
-    }
-
-    assertEquals(c, 3);
-
-    clearInterval(t);
     await resolvable;
+    assertEquals(c, 3);
+  },
+);
+
+unitTest(
+  {
+    ignore: Deno.build.os === "windows",
+    permissions: { run: true },
+  },
+  async function multipleSignalListenerTest() {
+    const resolvable = deferred();
+    let c = "";
+    const listener0 = () => {
+      c += "0";
+    };
+    const listener1 = () => {
+      c += "1";
+    };
+    Deno.addSignalListener("SIGUSR2", listener0);
+    Deno.addSignalListener("SIGUSR2", listener1);
+    setTimeout(async () => {
+      // Sends SIGUSR2 3 times.
+      for (const _ of Array(3)) {
+        await delay(20);
+        Deno.kill(Deno.pid, "SIGUSR2");
+      }
+      await delay(20);
+      Deno.removeSignalListener("SIGUSR2", listener1);
+      // Sends SIGUSR2 3 times.
+      for (const _ of Array(3)) {
+        await delay(20);
+        Deno.kill(Deno.pid, "SIGUSR2");
+      }
+      await delay(20);
+      // Sends SIGUSR1 (irrelevant signal) 3 times.
+      for (const _ of Array(3)) {
+        await delay(20);
+        Deno.kill(Deno.pid, "SIGUSR1");
+      }
+      await delay(20);
+      Deno.removeSignalListener("SIGUSR2", listener0);
+      resolvable.resolve();
+    });
+
+    await resolvable;
+    // The first 3 events are handled by both handlers
+    // The last 3 events are handled only by handler0
+    assertEquals(c, "010101000");
   },
 );
 
@@ -138,13 +178,13 @@ unitTest(
     ignore: Deno.build.os === "windows",
     permissions: { run: true, read: true },
   },
-  async function signalStreamExitTest() {
+  async function canExitWhileListeningToSignal() {
     const p = Deno.run({
       cmd: [
         Deno.execPath(),
         "eval",
         "--unstable",
-        "(async () => { for await (const _ of Deno.signal('SIGIO')) {} })()",
+        "Deno.addSignalListener('SIGIO', () => {})",
       ],
     });
     const res = await p.status();
@@ -154,90 +194,18 @@ unitTest(
 );
 
 unitTest(
-  { ignore: Deno.build.os === "windows", permissions: { run: true } },
-  async function signalPromiseTest() {
-    const resolvable = deferred();
-    // This prevents the program from exiting.
-    const t = setInterval(() => {}, 1000);
-
-    const sig = Deno.signal("SIGUSR1");
-    setTimeout(() => {
-      Deno.kill(Deno.pid, "SIGUSR1");
-      resolvable.resolve();
-    }, 20);
-    await sig;
-    sig.dispose();
-
-    clearInterval(t);
-    await resolvable;
+  {
+    ignore: Deno.build.os === "windows",
+    permissions: { run: true },
   },
-);
-
-// https://github.com/denoland/deno/issues/9806
-unitTest(
-  { ignore: Deno.build.os === "windows", permissions: { run: true } },
-  async function signalPromiseTest2() {
-    const resolvable = deferred();
-    // This prevents the program from exiting.
-    const t = setInterval(() => {}, 1000);
-
-    let called = false;
-    const sig = Deno.signal("SIGUSR1");
-    sig.then(() => {
-      called = true;
+  function signalInvalidHandlerTest() {
+    assertThrows(() => {
+      // deno-lint-ignore no-explicit-any
+      Deno.addSignalListener("SIGINT", "handler" as any);
+    });
+    assertThrows(() => {
+      // deno-lint-ignore no-explicit-any
+      Deno.removeSignalListener("SIGINT", "handler" as any);
     });
-    setTimeout(() => {
-      sig.dispose();
-      setTimeout(() => {
-        resolvable.resolve();
-      }, 10);
-    }, 10);
-
-    clearInterval(t);
-    await resolvable;
-
-    // Promise callback is not called because it didn't get
-    // the corresponding signal.
-    assert(!called);
-  },
-);
-
-unitTest(
-  { ignore: Deno.build.os === "windows", permissions: { run: true } },
-  function signalShorthandsTest() {
-    let s: Deno.SignalStream;
-    s = Deno.signal("SIGALRM");
-    assert(s instanceof Deno.SignalStream);
-    s.dispose();
-    s = Deno.signal("SIGCHLD");
-    assert(s instanceof Deno.SignalStream);
-    s.dispose();
-    s = Deno.signal("SIGHUP");
-    assert(s instanceof Deno.SignalStream);
-    s.dispose();
-    s = Deno.signal("SIGINT");
-    assert(s instanceof Deno.SignalStream);
-    s.dispose();
-    s = Deno.signal("SIGIO");
-    assert(s instanceof Deno.SignalStream);
-    s.dispose();
-    s = Deno.signal("SIGPIPE");
-    assert(s instanceof Deno.SignalStream);
-    s.dispose();
-    s = Deno.signal("SIGQUIT");
-    assert(s instanceof Deno.SignalStream);
-    s.dispose();
-    s = Deno.signal("SIGTERM");
-    assert(s instanceof Deno.SignalStream);
-    s.dispose();
-    s = Deno.signal("SIGUSR1");
-    assert(s instanceof Deno.SignalStream);
-    s.dispose();
-    s = Deno.signal("SIGUSR2");
-    assert(s instanceof Deno.SignalStream);
-    s.dispose();
-    s = Deno.signal("SIGWINCH");
-    assert(s instanceof Deno.SignalStream);
-    s.dispose();
   },
 );
diff --git a/runtime/js/40_signals.js b/runtime/js/40_signals.js
index 2de58e74cd..a4f3a6ccd4 100644
--- a/runtime/js/40_signals.js
+++ b/runtime/js/40_signals.js
@@ -3,14 +3,9 @@
 
 ((window) => {
   const core = window.Deno.core;
-  const { build } = window.__bootstrap.build;
-  const { errors } = window.__bootstrap.errors;
   const {
-    Error,
-    Promise,
-    PromisePrototypeThen,
-    PromiseResolve,
-    SymbolAsyncIterator,
+    Set,
+    TypeError,
   } = window.__bootstrap.primordials;
 
   function bindSignal(signo) {
@@ -25,77 +20,63 @@
     core.opSync("op_signal_unbind", rid);
   }
 
-  function signal(signo) {
-    if (build.os === "windows") {
-      throw new Error("Signal API is not implemented for Windows");
-    }
-    return new SignalStream(signo);
+  // Stores signal listeners and resource data. This has type of
+  // `Record<string, { rid: number | undefined, listeners: Set<() => void> }`
+  const signalData = {};
+
+  /** Gets the signal handlers and resource data of the given signal */
+  function getSignalData(signo) {
+    return signalData[signo] ??
+      (signalData[signo] = { rid: undefined, listeners: new Set() });
   }
 
-  class SignalStream {
-    #disposed = false;
-    #pollingPromise = PromiseResolve(false);
-    #rid = 0;
-
-    constructor(signo) {
-      this.#rid = bindSignal(signo);
-      this.#loop();
+  function checkSignalListenerType(listener) {
+    if (typeof listener !== "function") {
+      throw new TypeError(
+        `Signal listener must be a function. "${typeof listener}" is given.`,
+      );
     }
+  }
 
-    #pollSignal = async () => {
-      let done;
-      try {
-        done = await pollSignal(this.#rid);
-      } catch (error) {
-        if (error instanceof errors.BadResource) {
-          return true;
-        }
-        throw error;
-      }
-      return done;
-    };
+  function addSignalListener(signo, listener) {
+    checkSignalListenerType(listener);
 
-    #loop = async () => {
-      do {
-        this.#pollingPromise = this.#pollSignal();
-      } while (!(await this.#pollingPromise) && !this.#disposed);
-    };
+    const sigData = getSignalData(signo);
+    sigData.listeners.add(listener);
 
-    then(
-      f,
-      g,
-    ) {
-      const p = PromisePrototypeThen(this.#pollingPromise, (done) => {
-        if (done) {
-          // If pollingPromise returns true, then
-          // this signal stream is finished and the promise API
-          // should never be resolved.
-          return new Promise(() => {});
-        }
+    if (!sigData.rid) {
+      // If signal resource doesn't exist, create it.
+      // The program starts listening to the signal
+      sigData.rid = bindSignal(signo);
+      loop(sigData);
+    }
+  }
+
+  function removeSignalListener(signo, listener) {
+    checkSignalListenerType(listener);
+
+    const sigData = getSignalData(signo);
+    sigData.listeners.delete(listener);
+
+    if (sigData.listeners.size === 0 && sigData.rid) {
+      unbindSignal(sigData.rid);
+      sigData.rid = undefined;
+    }
+  }
+
+  async function loop(sigData) {
+    while (sigData.rid) {
+      if (await pollSignal(sigData.rid)) {
         return;
-      });
-      return PromisePrototypeThen(p, f, g);
-    }
-
-    async next() {
-      return { done: await this.#pollingPromise, value: undefined };
-    }
-
-    [SymbolAsyncIterator]() {
-      return this;
-    }
-
-    dispose() {
-      if (this.#disposed) {
-        throw new Error("The stream has already been disposed.");
       }
-      this.#disposed = true;
-      unbindSignal(this.#rid);
+      for (const listener of sigData.listeners) {
+        listener();
+      }
     }
   }
 
   window.__bootstrap.signals = {
-    signal,
-    SignalStream,
+    addSignalListener,
+    removeSignalListener,
   };
 })(this);
diff --git a/runtime/js/90_deno_ns.js b/runtime/js/90_deno_ns.js
index 897705f842..40c744fc17 100644
--- a/runtime/js/90_deno_ns.js
+++ b/runtime/js/90_deno_ns.js
@@ -109,9 +109,8 @@
   };
 
   __bootstrap.denoNsUnstable = {
-    signal: __bootstrap.signals.signal,
-    Signal: __bootstrap.signals.Signal,
-    SignalStream: __bootstrap.signals.SignalStream,
+    addSignalListener: __bootstrap.signals.addSignalListener,
+    removeSignalListener: __bootstrap.signals.removeSignalListener,
     emit: __bootstrap.compilerApi.emit,
     setRaw: __bootstrap.tty.setRaw,
     consoleSize: __bootstrap.tty.consoleSize,