Kill the whole process group when shutting down, instead of just the …

…one child process

This fixes a bug where when wds is orchestrating a process that starts
its own child processes, those child processes can be come zombies if we
just SIGTERM/SIGKILL the main orchestrated process. What we need to do
is kill the whole process group, which you can do by sending the
negative pid of the parent to the kernel.

See https://azimi.me/2014/12/31/kill-child_process-node-js.html

package.json

@@ -55,7 +55,7 @@
     "@types/find-root": "^1.1.2",
     "@types/jest": "^27.4.0",
     "@types/lodash": "^4.14.194",
-    "@types/node": "^14.14.14",
+    "@types/node": "^18.11.9",
     "@types/yargs": "^15.0.14",
     "eslint": "^8.40.0",
     "eslint-plugin-jest": "^27.2.1",

spec/Supervisor.spec.ts

@@ -106,4 +106,4 @@ test("it doesn't have any stdin if wds is started with terminal commands", async
   await childExit(child);
 
   expect(output).toEqual("");
-});
+});

src/Project.ts

@@ -17,7 +17,7 @@ export class Project {
   supervisor!: Supervisor;
   watcher?: FSWatcher;
 
-  constructor(readonly workspaceRoot: string, readonly config: ProjectConfig, readonly compiler: Compiler) {}
+  constructor(readonly workspaceRoot: string, readonly config: ProjectConfig, readonly compiler: Compiler) { }
 
   addShutdownCleanup(cleanup: () => void) {
     this.cleanups.push(cleanup);
@@ -58,8 +58,8 @@ export class Project {
     this.supervisor.restart();
   }
 
-  shutdown(code = 0) {
-    this.supervisor.stop();
+  async shutdown(code = 0) {
+    await this.supervisor.stop();
     for (const cleanup of this.cleanups) {
       cleanup();
     }

src/Supervisor.ts

@@ -1,6 +1,7 @@
+import { setTimeout } from "timers/promises";
 import type { ChildProcess, StdioOptions } from "child_process";
 import { spawn } from "child_process";
-import { EventEmitter } from "events";
+import { EventEmitter, once } from "events";
 import type { RunOptions } from "./Options";
 import type { Project } from "./Project";
 import { log } from "./utils";
@@ -12,28 +13,41 @@ export class Supervisor extends EventEmitter {
     super();
   }
 
-  stop() {
-    if (this.process) {
-      this.process.kill("SIGTERM");
+  /** 
+   * Stop the process with a graceful SIGTERM, then SIGKILL after a timeout
+   * Kills the whole process group so that any subprocesses of the process are also killed
+   * See https://azimi.me/2014/12/31/kill-child_process-node-js.html for more information
+   */
+  async stop() {
+    if (!this.process || !this.process.pid) return;
+
+    const ref = this.process;
+    const exit = once(ref, 'exit')
+    this.kill("SIGTERM")
+
+    await Promise.race([exit, setTimeout(3000)]);
+    if (!ref.killed) {
+      this.kill("SIGKILL", ref.pid)
     }
-    const process = this.process;
-    setTimeout(() => {
-      if (!process.killed) {
-        process.kill("SIGKILL");
-      }
-    }, 5000);
   }
 
-  kill() {
-    if (this.process) {
-      this.process.kill("SIGKILL");
+  kill(signal = "SIGKILL", pid = this.process?.pid) {
+    if (pid) {
+      try {
+        process.kill(-pid, signal);
+      } catch (error: any) {
+        if (error.code == "ESRCH") {
+          // process can't be found, is already dead
+          return
+        } else {
+          throw error
+        }
+      }
     }
   }
 
   restart() {
-    if (this.process) {
-      this.process.kill("SIGKILL");
-    }
+    this.kill()
 
     const stdio: StdioOptions = [null, "inherit", "inherit"];
     if (!this.options.terminalCommands) {
@@ -51,6 +65,7 @@ export class Supervisor extends EventEmitter {
         WDS_EXTENSIONS: this.project.config.extensions.join(","),
       },
       stdio: stdio,
+      detached: true
     });
 
     if (this.options.terminalCommands) {