From 6dade3c3a4f457202f5edaf16504efea27352814 Mon Sep 17 00:00:00 2001 From: Chakradhar Palaparthi Date: Fri, 7 Aug 2020 13:24:07 +0530 Subject: [PATCH] fix: corrected #1742 Moved run.kill outside the run function Moved up restart bind, before runCmd so that we can use it in run.kill If command is not to be run the we just need to watch files and not fork/spawn a child process Binding options with instance of run, so that we can use it in run.kill (https://travis-ci.org/github/remy/nodemon/builds/714612398) Updated run options inside if condition (https://travis-ci.org/github/remy/nodemon/builds/714620936) --- lib/monitor/run.js | 121 +++++++++++++++++++++++++-------------------- 1 file changed, 67 insertions(+), 54 deletions(-) diff --git a/lib/monitor/run.js b/lib/monitor/run.js index dce4638a..bd500ec0 100644 --- a/lib/monitor/run.js +++ b/lib/monitor/run.js @@ -18,16 +18,31 @@ var signals = require('./signals'); function run(options) { var cmd = config.command.raw; + // moved up + // we need restart function below in the global scope for run.kill + /*jshint validthis:true*/ + restart = run.bind(this, options); + run.restart = restart; + + // binding options with instance of run + // so that we can use it in run.kill + run.options = options; var runCmd = !options.runOnChangeOnly || config.lastStarted !== 0; if (runCmd) { utils.log.status('starting `' + config.command.string + '`'); + } else { + // should just watch file if command is not to be run + // had another alternate approach + // to stop process being forked/spawned in the below code + // but this approach does early exit and makes code cleaner + debug('start watch on: %s', config.options.watch); + if (config.options.watch !== false) { + watch(); + return; + } } - /*jshint validthis:true*/ - restart = run.bind(this, options); - run.restart = restart; - config.lastStarted = Date.now(); var stdio = ['pipe', 'pipe', 'pipe']; @@ -235,53 +250,9 @@ function run(options) { } }); - run.kill = function (noRestart, callback) { - // I hate code like this :( - Remy (author of said code) - if (typeof noRestart === 'function') { - callback = noRestart; - noRestart = false; - } - - if (!callback) { - callback = noop; - } - - if (child !== null) { - // if the stdin piping is on, we need to unpipe, but also close stdin on - // the child, otherwise linux can throw EPIPE or ECONNRESET errors. - if (options.stdin) { - process.stdin.unpipe(child.stdin); - } - - // For the on('exit', ...) handler above the following looks like a - // crash, so we set the killedAfterChange flag if a restart is planned - if (!noRestart) { - killedAfterChange = true; - } - - /* Now kill the entire subtree of processes belonging to nodemon */ - var oldPid = child.pid; - if (child) { - 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 - // stdin after we close. - if (child && options.stdin && child.stdin && oldPid === child.pid) { - child.stdin.end(); - } - callback(); - }); - } - } else if (!noRestart) { - // if there's no child, then we need to manually start the process - // this is because as there was no child, the child.on('exit') event - // handler doesn't exist which would normally trigger the restart. - bus.once('start', callback); - restart(); - } else { - callback(); - } - }; + // moved the run.kill outside to handle both the cases + // intial start + // no start // connect stdin to the child process (options.stdin is on by default) if (options.stdin) { @@ -379,12 +350,54 @@ function kill(child, signal, callback) { } } -// stubbed out for now, filled in during run -run.kill = function (flag, callback) { - if (callback) { +run.kill = function (noRestart, callback) { + // I hate code like this :( - Remy (author of said code) + if (typeof noRestart === 'function') { + callback = noRestart; + noRestart = false; + } + + if (!callback) { + callback = noop; + } + + if (child !== null) { + // if the stdin piping is on, we need to unpipe, but also close stdin on + // the child, otherwise linux can throw EPIPE or ECONNRESET errors. + if (run.options.stdin) { + process.stdin.unpipe(child.stdin); + } + + // For the on('exit', ...) handler above the following looks like a + // crash, so we set the killedAfterChange flag if a restart is planned + if (!noRestart) { + killedAfterChange = true; + } + + /* Now kill the entire subtree of processes belonging to nodemon */ + var oldPid = child.pid; + if (child) { + 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 + // stdin after we close. + if (child && run.options.stdin && child.stdin && oldPid === child.pid) { + child.stdin.end(); + } + callback(); + }); + } + } else if (!noRestart) { + // if there's no child, then we need to manually start the process + // this is because as there was no child, the child.on('exit') event + // handler doesn't exist which would normally trigger the restart. + bus.once('start', callback); + run.restart(); + } else { callback(); } }; + run.restart = noop; bus.on('quit', function onQuit(code) {