From bf9b7a6876610bfe9031fa27f7fdba26b44e649c Mon Sep 17 00:00:00 2001 From: Remy Sharp Date: Fri, 15 Dec 2017 13:56:36 +0000 Subject: [PATCH] fix: if no ps, walk /proc to kill child fully Originally used ps-tree which relied on `ps` on *nix. But if the system didn't have `ps` at all, we'd try to kill the child process, but alas this would not always work, as we're spawning `sh` and _then_ node, so the kill would only kill the `sh` process, and not the running node process. The new @remy/pstree lib sniffs for `ps` and defers to ps-tree, otherwise it will walk /proc and map the PPID to the child process allowing nodemon to fully clean up. --- lib/monitor/run.js | 38 ++++++++++++-------------------------- package-lock.json | 20 ++++++++++++++------ package.json | 2 +- 3 files changed, 27 insertions(+), 33 deletions(-) diff --git a/lib/monitor/run.js b/lib/monitor/run.js index 058512b4..ab890ec9 100644 --- a/lib/monitor/run.js +++ b/lib/monitor/run.js @@ -11,17 +11,9 @@ var child = null; // the actual child process we spawn var killedAfterChange = false; var noop = function () { }; var restart = null; -var psTree = require('ps-tree'); -var hasPS = true; +var psTree = require('@remy/pstree'); var path = require('path'); -// discover if the OS has `ps`, and therefore can use psTree -exec('ps', function (error) { - if (error) { - hasPS = false; - } -}); - function run(options) { var cmd = config.command.raw; @@ -291,23 +283,17 @@ function kill(child, signal, callback) { exec('taskkill /pid ' + child.pid + ' /T /F'); callback(); } else { - if (hasPS) { - // we use psTree to kill the full subtree of nodemon, because when - // spawning processes like `coffee` under the `--debug` flag, it'll spawn - // it's own child, and that can't be killed by nodemon, so psTree gives us - // an array of PIDs that have spawned under nodemon, and we send each the - // configured signal (defaul: SIGUSR2) signal, which fixes #335 - psTree(child.pid, function (err, kids) { - spawn('kill', ['-s', signal, child.pid].concat(kids.map(function (p) { - return p.PID; - }))).on('close', callback); - }); - } else { - exec('kill -s ' + signal + ' ' + child.pid, function () { - // ignore if the process has been killed already - callback(); - }); - } + // we use psTree to kill the full subtree of nodemon, because when + // spawning processes like `coffee` under the `--debug` flag, it'll spawn + // it's own child, and that can't be killed by nodemon, so psTree gives us + // an array of PIDs that have spawned under nodemon, and we send each the + // configured signal (default: SIGUSR2) signal, which fixes #335 + // note that psTree also works if `ps` is missing by looking in /proc + psTree(child.pid, function (err, kids) { + spawn('kill', ['-s', signal, child.pid].concat(kids.map(function (p) { + return p.PID; + }))).on('close', callback); + }); } } diff --git a/package-lock.json b/package-lock.json index 68dc552f..5ecfed74 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7,7 +7,7 @@ "@commitlint/cli": { "version": "3.2.0", "resolved": "https://registry.npmjs.org/@commitlint/cli/-/cli-3.2.0.tgz", - "integrity": "sha512-vqedqIEih0DUMz7myT+Fw4SLB03hF2U90dSHDBTPtiI3yHsm/yz2ruchRs2RuOTtwtcLrgtTdbFvRWzNkArBWw==", + "integrity": "sha1-vokC8vqFn5Npek4gQwXWtHZICTE=", "dev": true, "requires": { "@commitlint/core": "3.2.0", @@ -47,6 +47,14 @@ "semver": "5.4.1" } }, + "@remy/pstree": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/@remy/pstree/-/pstree-1.0.0.tgz", + "integrity": "sha512-QKtg6GZb7xMuAA/JeibdFRir+IkDfueG6HoIXNNAZQGvSHUG6TFsxKMqyaxjC9PvPDxob6G3eEbAcO5/nyYo4g==", + "requires": { + "ps-tree": "1.1.0" + } + }, "@semantic-release/commit-analyzer": { "version": "3.0.7", "resolved": "https://registry.npmjs.org/@semantic-release/commit-analyzer/-/commit-analyzer-3.0.7.tgz", @@ -909,7 +917,7 @@ "debug": { "version": "2.6.9", "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.9.tgz", - "integrity": "sha512-bC7ElrdJaJnPbAP+1EotYvqZsb3ecl5wi6Bfi6BJTUcNowp6cvspg0jXznRTKDjm/E7AdgFBVeAPVMNcKGsHMA==", + "integrity": "sha1-XRKFFd8TT/Mn6QpMk/Tgd6U2NB8=", "requires": { "ms": "2.0.0" } @@ -2624,7 +2632,7 @@ "husky": { "version": "0.14.3", "resolved": "https://registry.npmjs.org/husky/-/husky-0.14.3.tgz", - "integrity": "sha512-e21wivqHpstpoiWA/Yi8eFti8E+sQDSS53cpJsPptPs295QTOQR0ZwnHo2TXy1XOpZFD9rPOd3NpmqTK6uMLJA==", + "integrity": "sha1-xp7XTi0neXaaF7qDmbVM4LY8EsM=", "dev": true, "requires": { "is-ci": "1.0.10", @@ -3536,7 +3544,7 @@ "minimatch": { "version": "3.0.4", "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.0.4.tgz", - "integrity": "sha512-yJHVQEhyqPLUTgt9B83PXu6W3rx4MvvHvSUvToogpwoGDOUQ+yDrR0HRot+yOCdCO7u4hX3pWft6kWBBcqh0UA==", + "integrity": "sha1-UWbihkV/AzBgZL5Ul+jbsMPTIIM=", "requires": { "brace-expansion": "1.1.8" } @@ -4537,7 +4545,7 @@ "semantic-release": { "version": "8.2.0", "resolved": "https://registry.npmjs.org/semantic-release/-/semantic-release-8.2.0.tgz", - "integrity": "sha512-b86H8268ublYLVM2ytHozVv5J4vBaRFli3q7h7wIYku7/my6mg1tWtINvapcWBfgEF2L+HKXySoLTG2B4hMc7g==", + "integrity": "sha1-lyqjpyRgZdikBZkQBaIQ5GmV1LY=", "dev": true, "requires": { "@semantic-release/commit-analyzer": "3.0.7", @@ -4929,7 +4937,7 @@ "touch": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/touch/-/touch-3.1.0.tgz", - "integrity": "sha512-WBx8Uy5TLtOSRtIq+M03/sKDrXCLHxwDcquSP2c43Le03/9serjQBIztjRz6FkJez9D/hleyAXTBGLwwZUw9lA==", + "integrity": "sha1-/jZfX3XsntTlaCXgu3bSSrdK+Ds=", "requires": { "nopt": "1.0.10" } diff --git a/package.json b/package.json index 660ed7f2..79ca72fb 100644 --- a/package.json +++ b/package.json @@ -52,13 +52,13 @@ "should": "~4.0.0" }, "dependencies": { + "@remy/pstree": "^1.0.0", "chokidar": "^1.7.0", "debug": "^2.6.8", "es6-promise": "^3.3.1", "ignore-by-default": "^1.0.1", "lodash.defaults": "^3.1.2", "minimatch": "^3.0.4", - "ps-tree": "^1.1.0", "touch": "^3.1.0", "undefsafe": "0.0.3", "update-notifier": "^2.3.0"