Fix: use ps-tree to kill processes (fixes #14)

package.json

@@ -25,6 +25,7 @@
   "dependencies": {
     "babel-runtime": "^5.8.29",
     "minimatch": "^3.0.0",
+    "ps-tree": "^1.0.1",
     "shell-quote": "^1.4.3",
     "which": "^1.2.0"
   },

src/lib/spawn-posix.js

@@ -3,44 +3,37 @@
  * @copyright 2015 Toru Nagashima. All rights reserved.
  * See LICENSE file in root directory for full license.
  */
+/* eslint no-param-reassign: 0 */
 import cp from "child_process";
-
-// List of child processes that are running currently.
-const children = [];
-
-/**
- * Removes this process from the children pool.
- * @this ChildProcess
- */
-function removeFromPool() {
-    const index = children.indexOf(this);
-    if (index !== -1) {
-        children.splice(index, 1);
-    }
-}
+import getDescendentProcessInfo from "ps-tree";
 
 /**
- * Kills this process and sub processes with the process group ID.
+ * Kills the new process and its sub processes.
  * @this ChildProcess
  */
 function kill() {
-    try {
-        process.kill(-this.pid);
-    }
-    catch (err) {
-        // ignore.
-    }
+    getDescendentProcessInfo(this.pid, (err, descendent) => {
+        if (err) {
+            return;
+        }
+
+        for (const {PID: pid} of descendent) {
+            try {
+                process.kill(pid);
+            }
+            catch (err2) {
+                // ignore.
+            }
+        }
+    });
 }
 
 /**
  * Launches a new process with the given command.
  * This is almost same as `child_process.spawn`.
  *
- * This detaches the new process to make new process group.
- * And if this process exited before the new process exits, this kills the new process.
- *
  * This returns a `ChildProcess` instance.
- * `kill` method of the instance kills the new process and its sub processes with the process group ID.
+ * `kill` method of the instance kills the new process and its sub processes.
  *
  * @param {string} command - The command to run.
  * @param {string[]} args - List of string arguments.
@@ -49,22 +42,8 @@ function kill() {
  * @private
  */
 export default function spawn(command, args, options) {
-    options.detached = true; // eslint-disable-line no-param-reassign
-
     const child = cp.spawn(command, args, options);
-    child.on("exit", removeFromPool);
-    child.on("error", removeFromPool);
     child.kill = kill;
 
-    // Add to the pool to kill on exit.
-    children.push(child);
-
     return child;
 }
-
-// Kill all child processes on exit.
-process.on("exit", () => {
-    for (const child of children) {
-        child.kill();
-    }
-});

test-workspace/package.json

@@ -4,6 +4,7 @@
   "private": true,
   "description": "",
   "config": {
+    "DEST": "build",
     "test": "OK"
   },
   "scripts": {
@@ -18,7 +19,11 @@
     "test-task:append:b": "node tasks/append.js b",
     "test-task:append2": "node tasks/append2.js",
     "test-task:error": "node tasks/error.js",
-    "test-task:stdio": "node tasks/stdio.js"
+    "test-task:stdout": "node tasks/stdout.js > test.txt",
+    "test-task:stderr": "node tasks/stderr.js 2> test.txt",
+    "test-task:stdin": "echo STDIN | node tasks/stdin.js",
+    "test-task:issue14:win32": "..\\node_modules\\.bin\\rimraf build && mkdir %npm_package_config_DEST% && cd build",
+    "test-task:issue14:posix": "../node_modules/.bin/rimraf build && mkdir $npm_package_config_DEST && cd build"
   },
   "repository": {
     "type": "git",

test-workspace/tasks/stderr.js

@@ -0,0 +1,3 @@
+"use strict";
+
+process.stderr.write("STDERR");

test-workspace/tasks/stdin.js

@@ -0,0 +1,9 @@
+"use strict";
+
+var appendResult = require("../../test/lib/util").appendResult;
+
+process.stdin.on("data", function(chunk) {
+    appendResult(chunk.toString());
+    process.exit(0);
+});
+setTimeout(function() { process.exit(1); }, 5000);

test-workspace/tasks/stdout.js

@@ -0,0 +1,3 @@
+"use strict";
+
+process.stdout.write("STDOUT");

test/common.js

@@ -1,4 +1,3 @@
-import {PassThrough} from "stream";
 import assert from "power-assert";
 import {result, removeResult} from "./lib/util";
 import BufferStream from "./lib/buffer-stream";
@@ -47,40 +46,40 @@ describe("[common] npm-run-all", () => {
         );
     });
 
-    it("stdin option should pipe to task.", () => {
-        const stream = new PassThrough();
-        const promise = runAll("test-task:stdio -- --wait-input", {stdin: stream})
-            .then(() => {
-                assert(result() === "STDIN");
-            });
-
-        stream.write("STDIN");
+    describe("stdin can be used in tasks:", () => {
+        it("lib version", () =>
+            runAll("test-task:stdin")
+                .then(() => assert(result().trim() === "STDIN"))
+        );
 
-        return promise;
+        it("command version", () =>
+            command(["test-task:stdin"])
+                .then(() => assert(result().trim() === "STDIN"))
+        );
     });
 
-    it("stdout option should pipe from task.", (done) => {
-        const stream = new PassThrough();
-        stream.setEncoding("utf8");
-        runAll("test-task:stdio", {stdout: stream})
-            .then(() => {
-                stream.on("readable", () => {
-                    assert(stream.read().indexOf("STDOUT") >= 0);
-                    done();
-                });
-            });
+    describe("stdout can be used in tasks:", () => {
+        it("lib version", () =>
+            runAll("test-task:stdout")
+                .then(() => assert(result() === "STDOUT"))
+        );
+
+        it("command version", () =>
+            command(["test-task:stdout"])
+                .then(() => assert(result() === "STDOUT"))
+        );
     });
 
-    it("stderr option should pipe from task.", (done) => {
-        const stream = new PassThrough();
-        stream.setEncoding("utf8");
-        runAll("test-task:stdio", {stderr: stream})
-            .then(() => {
-                stream.on("readable", () => {
-                    assert(stream.read() === "STDERR");
-                    done();
-                });
-            });
+    describe("stderr can be used in tasks:", () => {
+        it("lib version", () =>
+            runAll("test-task:stderr")
+                .then(() => assert(result() === "STDERR"))
+        );
+
+        it("command version", () =>
+            command(["test-task:stderr"])
+                .then(() => assert(result() === "STDERR"))
+        );
     });
 
     describe("should be able to use `restart` built-in task:", () => {
@@ -129,4 +128,17 @@ describe("[common] npm-run-all", () => {
                 })
         );
     });
+
+    if (process.platform === "win32") {
+        describe("issue14", () => {
+            it("lib version", () => runAll("test-task:issue14:win32"));
+            it("command version", () => command(["test-task:issue14:win32"]));
+        });
+    }
+    else {
+        describe("issue14", () => {
+            it("lib version", () => runAll("test-task:issue14:posix"));
+            it("command version", () => command(["test-task:issue14:posix"]));
+        });
+    }
 });