fix: ubuntu loop waiting for sub processes

Fixes #1633

Multiple parts but all localised to run.js:

- Refactor the kill logic to be simpler
- Consistently use pstree to determine sub process list
- Pipe stderr through nodemon to squash `sh -c` error warning

Bug was caused by waiting on multiple sub processes and if they all
ended the logic would only subtract one from the count list (rather
than the total number).

I've refactored the code so that it doesn't use the `kill -0 <pid>` as
this was a little confusing to read (it's effectively a no op) and
switched to using pstree to test if any sub processes are still running.

The logic for killing the processes has also been refactored to
simplify. Before it would fork the logic based on whether `ps` existed
on the system.

Now it uses the same logic with the exception of the kill signal sent -
when `ps` isn't on the system, we have to send numeric signals (I can't
remember how I found that out, but I do remember it was a painful
process!).

The last part required due to a side effect of the refactor on kill:
when a kill signial is sent to `sh -c` the shell prints a warning.
Details on how to replicate: https://git.io/Je6d0

To squash this, I track if the process is about to be killed (by
flagging the sub process right before the kill function call) and if
there's an error whilst shutdown is in effect, the error is only printed
to nodemon's detailed output (using nodemon -V).

Author: remy

LICENSE

@@ -1,6 +1,6 @@
 MIT License
 
-Copyright (c) 2019 Remy Sharp, https://remysharp.com <remy@remysharp.com>
+Copyright (c) 2010 - present, Remy Sharp, https://remysharp.com <remy@remysharp.com>
 
 Permission is hereby granted, free of charge, to any person obtaining a copy
 of this software and associated documentation files (the "Software"), to deal

lib/monitor/run.js

@@ -1,4 +1,4 @@
-var debug = require('debug')('nodemon');
+var debug = require('debug')('nodemon:run');
 const statSync = require('fs').statSync;
 var utils = require('../utils');
 var bus = utils.bus;
@@ -10,34 +10,12 @@ var watch = require('./watch').watch;
 var config = require('../config');
 var child = null; // the actual child process we spawn
 var killedAfterChange = false;
-var noop = function () { };
+var noop = () => {};
 var restart = null;
 var psTree = require('pstree.remy');
 var path = require('path');
 var signals = require('./signals');
 
-function waitForSubprocesses(subprocesses, callback) {
-  if (Array.isArray(subprocesses) && subprocesses.length > 0) {
-    // check if all old subprocesses have been terminated
-    exec('kill -0 ' + subprocesses.join(' '), (error) => {
-      const returnCode = error ? error.code : 0;
-      if (returnCode < 126) { // ignore command not found error
-        const stillRunning = subprocesses.length - returnCode;
-        if (stillRunning > 0) {
-          utils.log.status('still waiting for ' + stillRunning +
-            ' subprocess(es) to finish...');
-          setTimeout(waitForSubprocesses.bind(this, subprocesses, callback),
-            100);
-          return;
-        }
-      }
-      callback();
-    });
-  } else {
-    callback();
-  }
-}
-
 function run(options) {
   var cmd = config.command.raw;
 
@@ -55,11 +33,11 @@ function run(options) {
   var stdio = ['pipe', 'pipe', 'pipe'];
 
   if (config.options.stdout) {
-    stdio = ['pipe', process.stdout, process.stderr];
+    stdio = ['pipe', process.stdout, 'pipe'];
   }
 
   if (config.options.stdin === false) {
-    stdio = [process.stdin, process.stdout, process.stderr];
+    stdio = [process.stdin, process.stdout, 'pipe'];
   }
 
   var sh = 'sh';
@@ -161,6 +139,20 @@ function run(options) {
         bus.emit('message', message, sendHandle);
       });
     }
+  } else { // else if not required…
+
+    // this swallows a shell message that happens because we kill the sh -c
+    // more details here: https://git.io/Je6d0
+    child.stderr.on('data', s => {
+      s = s.toString();
+
+      if (child.__nodemonRestart) { // this flag is set right before the kill
+        utils.log.detail('stderr: ' + s);
+        return;
+      }
+
+      process.stderr.write(s);
+    });
   }
 
   bus.emit('start');
@@ -284,6 +276,7 @@ function run(options) {
       /* Now kill the entire subtree of processes belonging to nodemon */
       var oldPid = child.pid;
       if (child) {
+        child.__nodemonRestart = true;
         kill(child, config.signal, function () {
           // this seems to fix the 0.11.x issue with the "rs" restart command,
           // though I'm unsure why. it seems like more data is streamed in to
@@ -340,6 +333,19 @@ function run(options) {
   }
 }
 
+function waitForSubProcesses(pid, callback) {
+  debug('checking ps tree for pids of ' + pid);
+  psTree(pid, (err, pids) => {
+    if (!pids.length) {
+      return callback();
+    }
+
+    utils.log.status(`still waiting for ${pids.length} sub-process${
+      pids.length > 2 ? 'es' : ''} to finish...`);
+    setTimeout(() => waitForSubProcesses(pid, callback), 1000);
+  });
+}
+
 function kill(child, signal, callback) {
   if (!callback) {
     callback = function () { };
@@ -362,26 +368,25 @@ function kill(child, signal, callback) {
     // 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
-    const sig = signal.replace('SIG', '');
-    psTree(child.pid, function (err, subprocesses) {
-      if (psTree.hasPS) {
-        spawn('kill', ['-s', sig].concat(subprocesses))
-          .on('close', waitForSubprocesses.bind(this, subprocesses,
-            function () {
-              spawn('kill', ['-s', sig, child.pid])
-                .on('close', callback);
-            }));
-      } else {
-        // make sure we kill from smallest to largest
-        const pids = subprocesses.slice().sort();
-        pids.forEach(pid => {
-          exec('kill -' + signals[signal] + ' ' + pid, () => { });
-        });
-        waitForSubprocesses(subprocesses, function () {
-          exec('kill -' + signals[signal] + ' ' + child.pid, () => { });
-          callback();
-        })
+    let sig = signal.replace('SIG', '');
+
+    psTree(child.pid, function (err, pids) {
+      // if ps isn't native to the OS, then we need to send the numeric value
+      // for the signal during the kill, `signals` is a lookup table for that.
+      if (!psTree.hasPS) {
+        sig = signals[signal];
       }
+
+      // the sub processes need to be killed from smallest to largest
+      debug('sending kill signal to ' + pids.join(', '));
+
+      pids.sort().forEach(pid => exec(`kill -${sig} ${pid}`, noop));
+
+      waitForSubProcesses(child.pid, () => {
+        // finally kill the main user process
+        exec(`kill -${sig} ${child.pid}`, callback);
+      });
+
     });
 
   }