Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: remy/nodemon
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: v2.0.0
Choose a base ref
...
head repository: remy/nodemon
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: v2.0.1
Choose a head ref
  • 1 commit
  • 2 files changed
  • 1 contributor

Commits on Nov 22, 2019

  1. 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).
    remy committed Nov 22, 2019

    Verified

    This commit was created on GitHub.com and signed with GitHub’s verified signature.
    Copy the full SHA
    ed91703 View commit details
Showing with 51 additions and 46 deletions.
  1. +1 −1 LICENSE
  2. +50 −45 lib/monitor/run.js
2 changes: 1 addition & 1 deletion LICENSE
Original file line number Diff line number Diff line change
@@ -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
95 changes: 50 additions & 45 deletions lib/monitor/run.js
Original file line number Diff line number Diff line change
@@ -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);
});

});

}