From 79342901afccd0f3d54bd2e9364f108414c4bf26 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Sat, 21 Nov 2015 22:54:26 +0900 Subject: [PATCH] Fix: use `ps-tree` to kill processes (fixes #14) --- package.json | 1 + src/lib/spawn-posix.js | 57 +++++++++------------------ test-workspace/package.json | 7 +++- test-workspace/tasks/stderr.js | 3 ++ test-workspace/tasks/stdin.js | 9 +++++ test-workspace/tasks/stdio.js | 14 ------- test-workspace/tasks/stdout.js | 3 ++ test/common.js | 72 ++++++++++++++++++++-------------- 8 files changed, 82 insertions(+), 84 deletions(-) create mode 100644 test-workspace/tasks/stderr.js create mode 100644 test-workspace/tasks/stdin.js delete mode 100644 test-workspace/tasks/stdio.js create mode 100644 test-workspace/tasks/stdout.js diff --git a/package.json b/package.json index 8d7c34d..7de11b8 100644 --- a/package.json +++ b/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" }, diff --git a/src/lib/spawn-posix.js b/src/lib/spawn-posix.js index c04abc1..7ab52e0 100644 --- a/src/lib/spawn-posix.js +++ b/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(); - } -}); diff --git a/test-workspace/package.json b/test-workspace/package.json index fabe020..e92d130 100644 --- a/test-workspace/package.json +++ b/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", diff --git a/test-workspace/tasks/stderr.js b/test-workspace/tasks/stderr.js new file mode 100644 index 0000000..3367acb --- /dev/null +++ b/test-workspace/tasks/stderr.js @@ -0,0 +1,3 @@ +"use strict"; + +process.stderr.write("STDERR"); diff --git a/test-workspace/tasks/stdin.js b/test-workspace/tasks/stdin.js new file mode 100644 index 0000000..4ce5a27 --- /dev/null +++ b/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); diff --git a/test-workspace/tasks/stdio.js b/test-workspace/tasks/stdio.js deleted file mode 100644 index 2d2e2e6..0000000 --- a/test-workspace/tasks/stdio.js +++ /dev/null @@ -1,14 +0,0 @@ -"use strict"; - -var appendResult = require("../../test/lib/util").appendResult; - -if (process.argv[2] === "--wait-input") { - process.stdin.on("data", function(chunk) { - appendResult(chunk.toString()); - process.exit(0); - }); - setTimeout(function() { process.exit(1); }, 10000); -} - -process.stdout.write("STDOUT"); -process.stderr.write("STDERR"); diff --git a/test-workspace/tasks/stdout.js b/test-workspace/tasks/stdout.js new file mode 100644 index 0000000..42a3926 --- /dev/null +++ b/test-workspace/tasks/stdout.js @@ -0,0 +1,3 @@ +"use strict"; + +process.stdout.write("STDOUT"); diff --git a/test/common.js b/test/common.js index d514cb6..3346ec8 100644 --- a/test/common.js +++ b/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"])); + }); + } });