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 Fails to Wait for Process Exit when using Docker + Inspector #1476

Closed
darkobits opened this issue Dec 1, 2018 · 44 comments
Closed
Labels
docker help wanted not-stale Tell stalebot to ignore this issue

Comments

@darkobits
Copy link

darkobits commented Dec 1, 2018

Created per @remy's request in this issue.

It seems that when restarting a process, Nodemon isn't waiting for the old process to completely exit before starting a new one, but:

  • only when in a Docker container and
  • only when the inspector/debug mode is active.

This issue can be intermittent in applications without a shutdown handler. But, having an application that implements a listener for Nodemon's SIGUSR2 signal that takes even a second or so to complete can be enough to cause this issue to happen reliably.

The presence of --inspect does cause the new process to immediately crash because its trying to open a debug server on the same port that is still in use by the previous process' debug server, but this merely a side-effect of this issue.

If running Nodemon in a Docker container without --inspect, Nodemon correctly waits for the old process to exit, but when --inspect is used inside the container, Nodemon does not wait for the process to exit, leading to an immediate crash.

This can be reproduced with Nodemon 1.18.7 and the following:

$ node -v
v10.13.0

$ docker -v
Docker version 18.09.0, build 4d60db4

$ docker-compose -v
docker-compose version 1.23.1, build b02f1306

Here is a repo that replicates this issue: https://github.com/darkobits/nodemon-restart-issue.

@ImTheDeveloper
Copy link

ImTheDeveloper commented Dec 2, 2018

Just started to see this issue when I upgraded to latest versions of nodemon too.

Ubuntu 18
nodemon 1.18.6 global
node 10.7.0
docker 18.06.1-ce build e68fc7a

Running this with vscode debugging. Worked perfectly fine previously until I did some package upgrades after the event-stream vulnerability.

NODE_ENV=development nodemon --legacy-watch --delay 2000ms --inspect-brk=0.0.0.0:9222 --nolazy index.js",

@daniele-orlando
Copy link

daniele-orlando commented Dec 3, 2018

Same issue, but only on Linux. No problem on Mac with Docker For Mac.
On my Linux machine the port is used by docker-proxy, an executable running as root user.

@remy
Copy link
Owner

remy commented Dec 10, 2018

I think this might be solved with the next release going up at the moment - assuming it passes, it'll land in nodemon@1.18.8.

@darkobits can you test this once it's up?

@remy remy added the docker label Dec 10, 2018
@denic
Copy link

denic commented Dec 11, 2018

@remy Works for me. Thanks!

@remy remy closed this as completed Dec 11, 2018
@ImTheDeveloper
Copy link

Great thank you for this fix!

@bighuggies
Copy link

I'm still seeing the issue with 1.18.8

@daniele-orlando
Copy link

v1.18.8

Same here, less frequent but still there.

@darkobits
Copy link
Author

I've updated the demo repo referenced in the original post to nodemon 1.18.9, and this issue is still occurring.

@darkobits
Copy link
Author

@remy, thoughts regarding leaving this closed vs. re-opening?

@justinpage
Copy link

justinpage commented Jan 6, 2019

I'm seeing intermittent results. Sometimes when I save a file change it restarts correctly.
Other times it errors out the following message:

crumb_1     | Starting inspector on 0.0.0.0:9229 failed: address already in use
crumb_1     | [nodemon] app crashed - waiting for file changes before starting...
crumb_1     | [nodemon] restarting due to changes...
crumb_1     | [nodemon] starting `node --inspect=0.0.0.0:9229 ./index.js`

What I'm forced to do, as shown above, is save again which triggers nodemon to restart.
The second time around works correctly.

  • version: 1.18.9
  • command ./node_modules/.bin/nodemon --inspect=0.0.0.0:9229 ./index.js

@stale
Copy link

stale bot commented Jan 20, 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 Jan 20, 2019
@justinpage
Copy link

Still applicable.

@stale stale bot removed the stale no activity for 2 weeks label Jan 20, 2019
@stale
Copy link

stale bot commented Feb 10, 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 10, 2019
@daniele-orlando
Copy link

daniele-orlando commented Feb 11, 2019

Still persists

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

stale bot commented Feb 25, 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 25, 2019
@anodynos
Copy link

I still get this error with nodemon@1.18.10 & latest node@10

@stale stale bot removed the stale no activity for 2 weeks label Feb 26, 2019
@d-wagner
Copy link

I also have this problem with nodemon@1.18.9 and 1.8.10

@aaronup
Copy link

aaronup commented Mar 7, 2019

We are also having this exact issue with nodemon@1.18.10

@stale
Copy link

stale bot commented Mar 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

@marcelkottmann
Copy link
Contributor

Hello, today I worked on this and I think I have understood what is happening here.

First of all it seems, that this is only a Linux related issue (from the comments I see docker and Ubuntu). I had this problem since several weeks with Ubuntu 18.

The problem is, that even if you think nodemon starts only one child process, nodemon actually starts two processes: one sh-process AND one node-process (run.js:97). The child.on('exit')-callback in run.js only gets events for the sh-Process (because it's the parent of the node process).
The node documentation explicitly says, that on Linux platform terminating the shell command does not necessarily kill all the shell child commands:

On Linux, child processes of child processes will not be terminated when attempting to kill their parent. This is likely to happen when running a new process in a shell or with the use of the shell option of ChildProcess.

(see https://nodejs.org/api/child_process.html#child_process_subprocess_kill_signal)
Although here the native kill command is executed in line run.js:368, it seems it behaves the same way.

Explicitly waiting for all other kids to be terminated before restarting the application solves the problem.

See my PR #1579

@daniele-orlando
Copy link

daniele-orlando commented Jun 11, 2019

@PePe79 Thank you very much for having worked on the issue and the extensive explanation. Very interesting and I've learned something new about Node.

@pavles6
Copy link

pavles6 commented Jul 24, 2019

Still persits, running on docker:

[nodemon] Internal watch failed: ENOSPC: System limit for number of file watchers reached, watch '/sys/kernel/slab/:A-0000040/cgroup/pde_opener(667:console-setup.service)/cpu_partial'

@aymather
Copy link

I've been having this issue for a while and my workaround might provide some information.

Right before I run npm run dev I run the following command: lsof -ti tcp:5000 | xargs kill which basically kills whatever processes are on the port.

However, even when I manually kill the port, I sometimes still get the same :::5000 address in use error. In that case I just kill the port again and it will usually work on the second try.

I'm just confused why even when I manually kill the port, I still get the error, and makes me think that maybe nodemon is the one blocking the port?

I'm running node v8.11.3 on MacOS 10.14.3.

@Remzi1993
Copy link

Remzi1993 commented Oct 21, 2019

I'm unable to replicate with OS X - see screenshot.

@nomcopter I'm using

  • node@v10.14.1
  • nodemon@latest
  • OS X @ 10.13.6 (High Sierra)

Screen Shot 2019-05-08 at 14 21 34

Because the issue is on Linux! I hope that delaying it could fix this problem. It seems like @PePe79 has fixed this issue, see PR #1579

@remy I hope you could take a look at his repo/commit.

marcelkottmann added a commit to marcelkottmann/nodemon that referenced this issue Oct 24, 2019
marcelkottmann added a commit to marcelkottmann/nodemon that referenced this issue Oct 24, 2019
remy added a commit that referenced this issue Nov 20, 2019
[skip ci]

* 'master' of https://github.com/pepe79/nodemon:
  fix: wait for all subprocesses to terminate (fixes issue #1476)
remy added a commit that referenced this issue Nov 20, 2019
* pepe79-master:
  fix: wait for all subprocesses to terminate (fixes issue #1476)
@towerofnix
Copy link

towerofnix commented Nov 25, 2019

now that #1632 (which contains 0e6ba3c) has been merged & nodemon v2 has been released, should this issue be closed?

@remy
Copy link
Owner

remy commented Nov 26, 2019

👍

@remy remy closed this as completed Nov 26, 2019
@marcelkottmann
Copy link
Contributor

Hello @remy , sorry to say that, but this issue is back again since version 2.0.2. It was fixed in 2.0.0 and 2.0.1. I just had the chance to upgrade to version 2.0.2 and this issue is back again. I looked at 47dfb8b and I guess the new line
child.kill(signal);
should only be called after all sub-processes have already been terminated. Moving this line behind the waitForSubProcesses-function call, should fix this.
Otherwise the exit signal of child will restart the process tooo early (again).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docker help wanted not-stale Tell stalebot to ignore this issue
Projects
None yet
Development

No branches or pull requests