Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nodemon does not wait for process to exit before starting a new one #1508

Closed
dobesv opened this issue Jan 18, 2019 · 28 comments
Closed

Nodemon does not wait for process to exit before starting a new one #1508

dobesv opened this issue Jan 18, 2019 · 28 comments
Labels
has PR help wanted needs more info not enough information in issue to debug not-stale Tell stalebot to ignore this issue

Comments

@dobesv
Copy link

dobesv commented Jan 18, 2019

System information:

$ nodemon -v
1.18.9
$ node -v
v11.6.0
$ uname -a
Linux formative 4.13.0-43-generic #48~16.04.1-Ubuntu SMP Thu May 17 12:56:46 UTC 2018 x86_64 
x86_64 x86_64 GNU/Linux

Expected behaviour

When I change a file, nodemon signals the process but does not start a new process unless the existing process exits.

Actual behaviour

nodemon starts a new process while the old process continues to run. If the old process was listening on a port, the new process might get an "address in use" error.

Steps to reproduce

Check out this git repo and follow the instructions:

https://github.com/dobesv/nodemon-repro

Alternatively, just run any process that uses the spawn method of launching (or maybe pass --spawn ?) and does not exit when it receives SIGUSR2.

Dump

$ nodemon -r ./register index.js --dump
[nodemon] 1.18.9
[nodemon] to restart at any time, enter `rs`
[nodemon] watching: *.*
--------------
node: v11.6.0
nodemon: 1.18.9
command: /home/ubuntu/.nvm/versions/node/v11.6.0/bin/node /home/ubuntu/nodemon-repro/node_modules/.bin/nodemon -r ./register index.js --dump
cwd: /home/ubuntu/nodemon-repro
OS: linux x64
--------------
{ run: false,
  system: { cwd: '/home/ubuntu/nodemon-repro' },
  required: false,
  dirs: [ '/home/ubuntu/nodemon-repro' ],
  timeout: 1000,
  options:
   { dump: true,
     ignore:
      [ '**/.git/**',
        '**/.nyc_output/**',
        '**/.sass-cache/**',
        '**/bower_components/**',
        '**/coverage/**',
        '**/node_modules/**',
        re: /.*.*\/\.git\/.*.*|.*.*\/\.nyc_output\/.*.*|.*.*\/\.sass\-cache\/.*.*|.*.*\/bower_components\/.*.*|.*.*\/coverage\/.*.*|.*.*\/node_modules\/.*.*/ ],
     watch: [ '*.*', re: /.*\..*/ ],
     ignoreRoot:
      [ '**/.git/**',
        '**/.nyc_output/**',
        '**/.sass-cache/**',
        '**/bower_components/**',
        '**/coverage/**',
        '**/node_modules/**' ],
     restartable: 'rs',
     colours: true,
     execMap: { py: 'python', rb: 'ruby' },
     stdin: true,
     runOnChangeOnly: false,
     verbose: false,
     signal: 'SIGUSR2',
     stdout: true,
     watchOptions: {},
     execOptions:
      { script: 'index.js',
        exec: 'node',
        args: [ '-r', './register' ],
        scriptPosition: 2,
        nodeArgs: undefined,
        execArgs: [],
        ext: 'js,mjs,json',
        env: {} },
     monitor:
      [ '*.*',
        '!**/.git/**',
        '!**/.nyc_output/**',
        '!**/.sass-cache/**',
        '!**/bower_components/**',
        '!**/coverage/**',
        '!**/node_modules/**' ] },
  load: [Function],
  reset: [Function: reset],
  lastStarted: 0,
  loaded: [],
  watchInterval: null,
  signal: 'SIGUSR2',
  command:
   { raw:
      { executable: 'node', args: [ '-r', './register', 'index.js' ] },
     string: 'node -r ./register index.js' } }
--------------
Done in 0.23s.

Related issues

@bke-daniel
Copy link

Hi everyone,

I guess #1501 is also related.

Cheers

@bke-daniel
Copy link

Little update from here:
Was going through my package.json and reinstalled everything, still not restarting.
But I say a warning / suggestion from babel that they suggest not tu use babel-preset-es2015 anymore. Replaced es2015 as suggested with preset env and everything's working as expected.
See here: https://babeljs.io/docs/en/env/

Maybe it helps someone. Not sure if it fixed my environment, it did at least for now.

@Cyral
Copy link

Cyral commented Jan 27, 2019

Rolling back to [email protected] fixed the issue for me. I remember this issue was happening before, was resolved, and appears to have reoccurred in the latest version.

@remy
Copy link
Owner

remy commented Jan 29, 2019

Can anyone else confirm downgrading to [email protected] also works?

I don't have whatever flavour of linux uname -a spat out, so I can't directly test @dobesv's original issue.

If .7 does indeed revert the issue, the number of commits are small enough to dig in to work out where it's going wrong:

It's either in a pstree.remy release in 1.18.8 - but pstree.remy has been released from 1.1.3 to 1.1.6 since that release, but since the version isn't pinned in [email protected], I'm guessing @Cyral has got [email protected] installed (could @Cyral confirm via npm ls pstree.remy - that would be useful).

Otherwise it's in the 1.18.9 release which is effectively a single line.

I think @dobesv could comment that single line out in their local version and re-test.

Also to note: the issues that have been tag as related are only related in that they have similar or the same symptoms. The cause is very different - and thus not related to these releases.

@remy remy added needs more info not enough information in issue to debug help wanted labels Jan 29, 2019
@dobesv
Copy link
Author

dobesv commented Jan 29, 2019

I tried downgrading to 1.18.7 in that reproduction I linked above and the symptoms are the same:

12:01:17 ~/nodemon-repro (master)*
$ yarn add [email protected]
yarn add v1.12.3
[1/4] Resolving packages...
[2/4] Fetching packages...
info [email protected]: The platform "linux" is incompatible with this module.
info "[email protected]" is an optional dependency and failed compatibility check. Excluding it from installation.
[3/4] Linking dependencies...
[4/4] Building fresh packages...
success Saved lockfile.
warning "nodemon" is already in "devDependencies". Please remove existing entry first before adding it to "dependencies".
success Saved 3 new dependencies.
info Direct dependencies
└─ [email protected]
info All dependencies
├─ [email protected]
├─ [email protected]
└─ [email protected]
Done in 1.12s.
12:01:39 ~/nodemon-repro (master)*
$ yarn start
yarn run v1.12.3
$ nodemon -r ./register index.js
[nodemon] 1.18.7
[nodemon] to restart at any time, enter `rs`
[nodemon] watching: *.*
[nodemon] starting `node -r ./register index.js`
24774 'started up!'
24774 'still running'
[nodemon] restarting due to changes...
24774 'SIGUSR2 received, ignoring it!'
[nodemon] starting `node -r ./register index.js`
24813 'started up!'
24774 'still running'
24813 'still running'
[nodemon] restarting due to changes...
24813 'SIGUSR2 received, ignoring it!'
[nodemon] starting `node -r ./register index.js`
24836 'started up!'
24774 'still running'
24813 'still running'
^C

@dobesv
Copy link
Author

dobesv commented Jan 29, 2019

I also tried downgrading [email protected] and the issue remains in my test case:

$ yarn add [email protected]
yarn add v1.12.3
[1/4] Resolving packages...
[2/4] Fetching packages...
info [email protected]: The platform "linux" is incompatible with this module.
info "[email protected]" is an optional dependency and failed compatibility check. Excluding it from installation.
[3/4] Linking dependencies...
[4/4] Building fresh packages...
success Saved lockfile.
success Saved 9 new dependencies.
info Direct dependencies
└─ [email protected]
info All dependencies
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
└─ [email protected]
Done in 1.00s.
12:03:18 ~/nodemon-repro (master)*
$ yarn start
yarn run v1.12.3
$ nodemon -r ./register index.js
[nodemon] 1.18.7
[nodemon] to restart at any time, enter `rs`
[nodemon] watching: *.*
[nodemon] starting `node -r ./register index.js`
24947 'started up!'
24947 'still running'
24947 'still running'
[nodemon] restarting due to changes...
24947 'SIGUSR2 received, ignoring it!'
[nodemon] starting `node -r ./register index.js`
25006 'started up!'
24947 'still running'
25006 'still running'
24947 'still running'
^C

@dobesv
Copy link
Author

dobesv commented Jan 29, 2019

Downgrading to node v8.11.4 also has no effect.

@dobesv
Copy link
Author

dobesv commented Jan 29, 2019

What appears to the be issue here is that when the process is launched using spawn, it runs a shell, and the shell runs the command.

When nodemon detects a file change, it sends the SIGUSR2 signal to both sh and node.

When sh exits, nodemon gets the exit event and launches the process again. However, node has not exited.

The bug here, I suppose, is that nodemon has to wait not only for its direct child to exit, but also any child process spawned by that.

Alternatively, perhaps there's a way to run this through the shell that doesn't create an extra intermediate shell process, e.g. by using exec.

@Cyral
Copy link

Cyral commented Jan 29, 2019

@remy

# npm ls pstree.remy
`-- [email protected]
  `-- [email protected]

@dobesv
Copy link
Author

dobesv commented Feb 7, 2019

To workaround this issue you have to avoid using the spawn launch method. What I found solved the issue for my particular case is to use NODE_OPTIONS to pass options to node, instead of passing them to nodemon as command line argument. For example NODE_OPTIONS="-r require.js --max_old_space_size=1000" nodemon script.js. This way it only creates one process with no intermediate sh process, so it correctly waits for the script to exit before restarting.

@stale
Copy link

stale bot commented Feb 21, 2019

This issue has been automatically marked as idle and stale because it hasn't had any recent activity. It will be automtically closed if no further activity occurs. If you think this is wrong, or the problem still persists, just pop a reply in the comments and @remy will (try!) to follow up.
Thank you for contributing <3

@stale stale bot added the stale no activity for 2 weeks label Feb 21, 2019
@dobesv
Copy link
Author

dobesv commented Feb 21, 2019

This is still an issue for processes run by nodemon using the "spawn" launch method.

@stale stale bot removed the stale no activity for 2 weeks label Feb 21, 2019
@remy
Copy link
Owner

remy commented Feb 22, 2019

I meant to remove the stale label last night. Anyway, I investigated this issue in my weekly twitch session and found some interesting things. (If you're interested it's here, though a little overly verbose - I suggest sped up!).

Short version:

  1. Doesn't happen on mac (even though it's linux based)
  2. I can replicate inside of a docker
  3. Spawn is definitely the issue
  4. The SIGUSR2 triggered by nodemon, passed down to the sub process but it appears that there's two sub processes - this is the issue.

In linux (I think) spawn is one process id (which nodemon refers to as the child and listens to signals on), but the running script is a second process.

The SIGUSR2 is sent by nodemon to the child which correctly passes to the running process (your process). Your process catches the signal and keeps running. The "middle" process, the child however doesn't keep running and triggers an exit which is why nodemon starts a new sub process.

I don't understand how to tell from the child process that it's sub process caught the exit signal.

This feels like a node issue over nodemon, but I'd need to pare down the issue even more.

Importantly: does anyone have any experience understanding unix processes, node and kill signals?

@dobesv
Copy link
Author

dobesv commented Feb 22, 2019

I believe I saw somewhere that one way to wait for all the subprocesses to exit is to wait for them to close their stdin / stdout / stderr handles. Then you as the parent will get an EOF (e.g. "close" event) on those handles. You can wait until all three handles are closed and the consider the job to have exited. This will ignore processes that explicitly close these handles themselves, but I think it's OK to assume those processes wanted to escape being controlled anyway. This article mentions this detail about the stdio streams closing when all the child processes close them or exit: https://medium.freecodecamp.org/node-js-child-processes-everything-you-need-to-know-e69498fe970a

Another way is when you try to kill the process, get a list of all the subprocess ids and start monitoring them to wait for them to close, only considering the exit complete when you can no longer detect any running subprocesses. This is more work but adheres to the more strict rule of really waiting for all subprocesses to exit, including ones that close all their stdio streams.

@stale
Copy link

stale bot commented Mar 8, 2019

This issue has been automatically marked as idle and stale because it hasn't had any recent activity. It will be automtically closed if no further activity occurs. If you think this is wrong, or the problem still persists, just pop a reply in the comments and @remy will (try!) to follow up.
Thank you for contributing <3

@stale stale bot added the stale no activity for 2 weeks label Mar 8, 2019
@stale stale bot closed this as completed Mar 15, 2019
@mdobroggg
Copy link

This is still an issue

@panzelva
Copy link

panzelva commented Jun 4, 2019

Also having this issue on Ubuntu 18 LTS

@cruhl
Copy link

cruhl commented Jun 18, 2019

This is an issue for me too on Mac OSX

@remy remy reopened this Jun 18, 2019
@stale stale bot removed the stale no activity for 2 weeks label Jun 18, 2019
@remy remy added the not-stale Tell stalebot to ignore this issue label Jun 18, 2019
@remy
Copy link
Owner

remy commented Jun 18, 2019

Adding label to tell stalebot to back off (I should have done this ages ago). Happy for anyone who detailed info on how to replicate to take a crack at a PR 👍

@marcelkottmann
Copy link
Contributor

I actually think, that this is a duplicate of #1476 and should also be fixed by the PR #1579 . Can someone confirm this?

@Remzi1993
Copy link

@PePe79 I also think these issues are related.

@ghost
Copy link

ghost commented Oct 30, 2019

This issue also occurs on MacOSX.
#1579 seems to fix it

@aodr3w
Copy link

aodr3w commented Nov 18, 2019

Found a solution for this.
Create a nodemon.json file in you're project root directory.
add the following to it:
{
"events":{
"restart":"fuser -k [port number] /tcp "
}
}
This will ensure that everytime nodemon is restarted , it kills the process running on it's assigned port and then restarts you're server.
Alternatively you can install npm package kill-port to do the same thing as fuser

@remy remy added the has PR label Nov 20, 2019
@remy
Copy link
Owner

remy commented Nov 21, 2019

PR live in [email protected]

@AndrewOdiit this is a nice solution, I'm going to add this to the faq too 👍

@remy remy closed this as completed Nov 21, 2019
@rweads0520
Copy link

rweads0520 commented Feb 4, 2020

This is what it took to get nodemon gracefully shutting down for me, a combination of a few things. I'm sure it could be done without node-cleanup, manually coding a lot of what that's doing, but it's a lot easier. Also myAsyncShutdown() doesn't need to return a promise, it could just as well be any function with a callback, e.g. myShutdown(cb => { ... }). However, it's the async case that was causing me problems - if I just tried handling the "SIGUSR2" as in the nodemon docs, it would bail immediately as soon as I hit an await (or a promise that did something asynchronous and required a continuation).

const nodeCleanup = require("node-cleanup");

...

let stopping = false;
nodeCleanup((exitCode, signal) => {
  // Don't call nodeCleanup.uninstall(), it needs to continue catching and ignoring signals
  if (!stopping) {
    stopping = true;
    console.log("Application shutting down...");
    myAsyncShutdown().then(() => {
      console.log("Application shutdown complete.");
      process.kill(process.pid, signal);
    });
  }
  return false;
});

process.on("SIGUSR2", () => process.kill(process.pid, "SIGHUP"));

@Remzi1993
Copy link

This is what it took to get nodemon gracefully shutting down for me, a combination of a few things. I'm sure it could be done without node-cleanup, manually coding a lot of what that's doing, but it's a lot easier. Also myAsyncShutdown() doesn't need to return a promise, it could just as well be any function with a callback, e.g. myShutdown(cb => { ... }). However, it's the async case that was causing me problems - if I just tried handling the "SIGUSR2" as in the nodemon docs, it would bail immediately as soon as I hit an await (or a promise that did something asynchronous and required a continuation).

const nodeCleanup = require("node-cleanup");

...

let stopping = false;
nodeCleanup((exitCode, signal) => {
  // Don't call nodeCleanup.uninstall(), it needs to continue catching and ignoring signals
  if (!stopping) {
    stopping = true;
    console.log("Application shutting down...");
    myAsyncShutdown().then(() => {
      console.log("Application shutdown complete.");
      process.kill(process.pid, signal);
    });
  }
  return false;
});

process.on("SIGUSR2", () => process.kill(process.pid, "SIGHUP"));

@remy Could you take a look at this?

@Clindbergh
Copy link

I am still having this issue and I'm not sure how to workaround.

@remy
Copy link
Owner

remy commented May 18, 2020

@Clindbergh you'll need to create an issue with pared down code if you think it can be fixed (thanks).

Repository owner locked and limited conversation to collaborators May 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
has PR help wanted needs more info not enough information in issue to debug not-stale Tell stalebot to ignore this issue
Projects
None yet
Development

No branches or pull requests