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 is not able to gracefully restart applications in Windows environment #1720

Closed
chriswoodle opened this issue May 14, 2020 · 50 comments
Closed

Comments

@chriswoodle
Copy link

chriswoodle commented May 14, 2020

Possibly related to #1567

  • nodemon -v: 6.13.4

  • node -v: v12.16.1

  • Operating system/terminal environment:

    • Windows Command Prompt, Windows Git Bash, Windows Terminal WSL
         Edition: Windows 10 Pro 
         Version: 1909
    Installed on: 5/22/2019
        OS Build: 18363.836 
    
  • Command ran: nodemon --signal SIGHUP index.js

Expected behaviour

On Windows, Nodemon should wait for the process to complete if it handles a shut down signal.

Notice the express server closed message

WSL:
wsl

Actual behaviour

The application is killed and restarted right away. This behavior does not exist in Linux, Mac and WSL.

CMD:
windows

Steps to reproduce

I have created a sample Express app here: https://github.com/chriswoodle/nodemon-1567 that shuts down the express server once the application is signaled to stop ['SIGTERM', 'SIGINT', 'SIGHUP']

git clone https://github.com/chriswoodle/nodemon-1567.git
cd nodemon-1567
npm install
npm start
touch index.js

Let me know if I can provide any more information or test changes, thanks.


@remy remy added the windows label May 16, 2020
@remy
Copy link
Owner

remy commented May 16, 2020

If I get time, I'll look into this, but I wanted to stop and say:

Thank you for filing an issue with a good level of detail and a good example of pared down code to replicate the issue. I'll be pointing people to this issue as an example of what makes the world of difference in debugging. 🙏

@nicholas-ochoa
Copy link

I came here with the same exact problem and I'm glad I found this issue. On *nix this works perfectly, on Windows it's almost like the signals are ignored and a SIGKILL is used instead? I'm seeing the same behavior regardless during restarts for file changes (or typing 'rs') - the application is IMMEDIATELY restarted and my shutdown handlers are never called.

@stale
Copy link

stale bot commented Jun 3, 2020

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 Jun 3, 2020
@nicholas-ochoa
Copy link

Still happening..

@stale stale bot removed the stale no activity for 2 weeks label Jun 4, 2020
@remy remy added help wanted not-stale Tell stalebot to ignore this issue labels Jun 4, 2020
@JulianNicholls
Copy link

JulianNicholls commented Jun 16, 2020

Hi Remy.

Just a suggestion

I hadn't seen this problem on Windows via WSL until I enabled a subscription in my GraphQL server.
GraphQL Subscriptions use websockets, which may be relevant.

This seems to be consistent, activating a subscription will cause nodemon to not terminate the server before attempting to restart. It usually leaves more than one instance running.

I can't offer any solutions, but I'd be happy to help you debug it.

@countzero
Copy link
Contributor

Hi there,

I dug a little bit deeper into this rabbit hole and like to share my findings:

Problem: taskkill does not send signals

If nodemon is running in a Windows context it is using the taskkill command to terminate the node process:

exec('taskkill /pid ' + child.pid + ' /T /F');

Sadly taskkill does not send any signals prior to reaping the process. Therefore no process event listeners within the Node.js application will trigger.

Solution: use windows-kill

There is actually a way to programmatically send the following signals to a process in Windows:

  • SIGINT ( CTRL + C )
  • SIGBREAK ( CTRL + Break )

There is a project called windows-kill which implements this:

.\windows-kill.exe -SIGINT 16312 
The node process (16312) received a SIGINT POSIX signal, starting graceful shutdown...

I used the windows-kill_x64_1.1.4_lib_release.zip binary.

Next steps

@remy I think the fix would be a drop in replacement of taskkill with windows-kill. But I have also seen the big comments above the taskkill invocation and do not know all edge-cases you have already tested for. And I am not sure, if it is possible to simply add the windows-kill.exe binary to this project (yet another dependency, license issues etc.). What do you think?

@koustavmandal95
Copy link

Facing issue nodemon stuck at restarting the server on windows 10 platform . Any trick around ?

@usm076
Copy link

usm076 commented Oct 7, 2020

Still isn't resolved

@sridharjatla
Copy link

i have some issue with nodemon when i use command nodemon app.js -e js,hbs after making some changes in hbs files nodemon is not restarting only for the first time its working later i have to shutdown the app.js and rerun the command

@remy
Copy link
Owner

remy commented Oct 19, 2020

Finally getting around to this - given @chriswoodle's code example, my primary question is: how does this work without nodemon?

@countzero I'm not sure this is the right solution - my reading is that this problem is a failing of the code running in a system that doesn't have POSSIX signals, and therefore the source code shouldn't be listening for these. Now, I could be entirely wrong, but that fact it works in WSL suggests that the windows-kill project is actually monkey patching in signals. Nodemon definitely does not want to do this because it'll change the running environment without nodemon (and one of the core principles that I first built nodemon with is: not to change the behaviour of runtime - i.e. no code changes required, etc).

I don't often have a windows machine to hand test this, but there's a good number of you on this thread that could try @chriswoodle's example git repo that replicates the problem.

The tasks is: run without nodemon, does the application behave as expected?

@remy remy removed the not-stale Tell stalebot to ignore this issue label Oct 19, 2020
@chriswoodle
Copy link
Author

Finally getting around to this - given @chriswoodle's code example, my primary question is: how does this work without nodemon?

@countzero I'm not sure this is the right solution - my reading is that this problem is a failing of the code running in a system that doesn't have POSSIX signals, and therefore the source code shouldn't be listening for these. Now, I could be entirely wrong, but that fact it works in WSL suggests that the windows-kill project is actually monkey patching in signals. Nodemon definitely does not want to do this because it'll change the running environment without nodemon (and one of the core principles that I first built nodemon with is: not to change the behaviour of runtime - i.e. no code changes required, etc).

I don't often have a windows machine to hand test this, but there's a good number of you on this thread that could try @chriswoodle's example git repo that replicates the problem.

The tasks is: run without nodemon, does the application behave as expected?

Using windows cmd, running node index and pressing CTRL + C sends SIGINT and waits for the process to exit.

image

Even when adding a timeout period.

image

@remy
Copy link
Owner

remy commented Oct 20, 2020

@chriswoodle I was pretty sure that the command prompt checked with confirmation whether you wanted to exit (the "are you sure y/n") on ctrl-c.

Why wouldn't that happen in this case? Or is this a particular flavour of Windows/the prompt/something else?

@remy
Copy link
Owner

remy commented Oct 20, 2020

Hmm... unless it's node intercepting the interrupt and doing something bespoke...

@chriswoodle
Copy link
Author

@chriswoodle I was pretty sure that the command prompt checked with confirmation whether you wanted to exit (the "are you sure y/n") on ctrl-c.

Why wouldn't that happen in this case? Or is this a particular flavour of Windows/the prompt/something else?

I believe this was the case in the past, "Terminate batch job (Y/N)?" is promoted when exiting batch/cmd scripts. I think node/npm switched over to powershell ps1 scripts, which do not have this prompt. Not %100 certain though, but I think it's something like this.

@remy
Copy link
Owner

remy commented Oct 20, 2020

Then I suspect this is what nodemon will need. I really don't want to add a bespoke binary 3rd party to do the windows-kill thing - it doesn't feel right. But I really know nothing about powershell so am looking to Windows users to help advise/drive.

@chriswoodle
Copy link
Author

Just to note,
Looking at my windows node install:
image

Runnng node from cmd

image

Directly executes node.exe, so no powershell involved.

@remy
Copy link
Owner

remy commented Oct 20, 2020

I wonder what the nodemon executable looks like on Windows? Isn't it one of those .cmd files?

@GIDustin
Copy link

nodemon is a cmd file, which calls node.exe with arguments:
image

so you have a node.exe for nodemon, monitoring a node.exe for the app:
image

From tasklist:
image

@oviecodes
Copy link

@remy how can I help with this. I've got a Windows machine.

@stale
Copy link

stale bot commented Dec 19, 2020

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 Dec 19, 2020
@caesay
Copy link

caesay commented Feb 3, 2021

Just bumping this because of the message from the bot and this does not appear to be resolved as of yet. Ideally nodemon should send a signal and wait for the process to exit before restarting and it does not currently do this.

To clarify on the "Terminate batch job (Y/N)?" question, it's true that this only happens with a batch/cmd file which is executed directly from terminal. For example, upon Ctrl-C:

  • node index.js no prompt
  • nodemon index.js (installed with -g) prompt present
  • npm run nodemon-pkg-script (installed locally as dev-dep) prompt present
  • node "node_modules/nodemon/bin/nodemon.js" no prompt

Additionally, I do not believe you need a third party binary to do this if you are happy to use powershell, the following line of powershell will give you a list of all of the processes running, their pid, and their parent pid - so from here it would be possible to terminate a process tree recursively with no additional binaries:

gwmi win32_process | select -property Name,ProcessId,ParentProcessId

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

remy commented Feb 3, 2021

Happy to take a PR that detects powershell and uses that method. I'm not in Windows at all, so I'm no help here (and unless someone actually contributes a PR to address this, nothing is really going to change - though I'm sure I've said this before )

countzero added a commit to countzero/nodemon that referenced this issue Apr 12, 2021
This commit silences all errors from the wmic.exe process. Which is save,
because the error case is handled by falling back to the child.pid.

It also optimizes the windows-kill invocation by minimizing (hiding)
the terminal window and keeping it open until the process has finished.
@countzero
Copy link
Contributor

Progress: I found a solution to hide the command prompt window (see above commit for details). Now there is only one bug left.

The "restart" event is triggered twice on the bus:

[nodemon] restarting due to changes...
The node process (13076) received a SIGINT POSIX signal, starting graceful shutdown...
Closing V8 inspector at: ws://0.0.0.0:9229/4b694f27-e0b6-4adf-ae55-f88d422552d3...
Terminating process with exit code 0...
[nodemon] restarting due to changes...
[nodemon] starting `node --expose-gc --inspect=0.0.0.0:9229 ./application/server/index.js`

@remy: I already detected the debug feature of nodemon and will look into the process order. My first hunch is, that nodemon always expects a kill signal on its child ("cmd.exe") and not on the childs child ("node"). Which perhaps causes two exit events somehow? Have to read some code to understand the context...

@countzero
Copy link
Contributor

Short status report:

I made a minimal proof of concept implementation with all the nodemon modifications I made so far. It is public and should be as developer friendly as possible to

  1. help narrowing down the bug of duplicate restart events and
  2. rapidly test the implementation in a Windows environment.

https://github.com/countzero/nodemon_windows_kill

@caesay & @chriswoodle: Do you have some time to test this?

@countzero
Copy link
Contributor

countzero commented Apr 20, 2021

Hi,

I optimized the https://github.com/countzero/nodemon_windows_kill proof of concept code further: Now it is cross-platform compatible, so that it can run side by side in a PowerShell context and a WSL Debian context:

nodemon_windows_kill

My findings from this are, that nodemon sometimes triggers POSIX signals multiple times in BOTH the Windows AND the UNIX context! I am not sure, if that is intended behaviour. @remy Is this actually intended behaviour, or is this a bug?

 npm run watch

> nodemon_windows_kill@0.0.1 watch D:\Privat\Projekte\nodemon_windows_kill
> npx nodemon --signal SIGINT ./index.js

[nodemon] main: 626a168ccaf6806b12a392660180508f21d0a7ee
[nodemon] to restart at any time, enter `rs`
[nodemon] watching path(s): *.*
[nodemon] watching extensions: js,mjs,json
[nodemon] starting `node ./index.js`
The server started listening on http://localhost:5938
[nodemon] restarting due to changes...
The node process (33676) received a SIGINT POSIX signal - initiating shutdown...
Terminating process with exit code 0...
[nodemon] starting `node ./index.js`
The server started listening on http://localhost:5941
[nodemon] restarting due to changes...
The node process (27500) received a SIGINT POSIX signal - initiating shutdown...
[nodemon] restarting due to changes...
The node process (27500) received a SIGINT POSIX signal - already shutting down, ignoring...
Terminating process with exit code 0...
[nodemon] starting `node ./index.js`
The server started listening on http://localhost:5943

@cowboyd
Copy link

cowboyd commented Apr 23, 2021

fyi, we wrote a very small package to do nothing except send a CTRL_C event to a windows process to handle situations exactly like this. https://github.com/thefrontside/ctrlc-windows/

Don't know if it would be a handy thing to use here, but it's worked well for us, and is extremely lightweight.

@wrexbe
Copy link

wrexbe commented Apr 28, 2021

Here is a way to send ctrl-c to the node process.

Command

powershell.exe -nologo -noprofile -executionpolicy bypass .\ctrlc.ps1 ProcessIdHere

File: ctrlc.ps1

Add-Type -Names 'w' -Name 'k' -M @(
    '[DllImport("kernel32.dll")] public static extern bool FreeConsole();',
    '[DllImport("kernel32.dll")] public static extern bool AttachConsole(uint p);',
    '[DllImport("kernel32.dll")] public static extern bool GenerateConsoleCtrlEvent(uint e, uint p);'
)
If ([w.k]::FreeConsole()) {
    If([w.k]::AttachConsole($args[0])) {
        If([w.k]::GenerateConsoleCtrlEvent(0, 0)){
            exit 0
        }
    }
}
throw;

Edit:

Made it so I didn't need a separate file. This approach doesn't work though. It sends the Ctrl-C to too many things.

const pid = 2500;
const ctrlcScript = Buffer.from(\`Add-Type -Names 'w' -Name 'k' -M @(
    '[DllImport("kernel32.dll")] public static extern bool FreeConsole();',
    '[DllImport("kernel32.dll")] public static extern bool AttachConsole(uint p);',
    '[DllImport("kernel32.dll")] public static extern bool GenerateConsoleCtrlEvent(uint e, uint p);'
);
If ([w.k]::FreeConsole()) {
    If([w.k]::AttachConsole(${pid})) {
        If([w.k]::GenerateConsoleCtrlEvent(0, 0)){
            exit 0;
        }
    }
}
exit 1;`.replace(/(\r\n|\n|\r)/gm, " "), 'utf16le').toString('base64');
var cp = require('child_process');
cp.exec(`powershell.exe -nologo -noprofile -executionpolicy bypass -EncodedCommand ${ctrlcScript}\`, function(e, stdout, stderr) {
  console.log(stdout);
  console.log(stderr);
});

@wrexbe
Copy link

wrexbe commented Apr 28, 2021

@countzero I tried your solution. The link for windows-kill was dead, but it worked for me after I changed it. Are you going to bundle the exe, and make a pull request?

countzero added a commit to countzero/nodemon that referenced this issue Apr 29, 2021
…1720)

This commit replaces the provisioning script, which downloaded
a specific windows-kill.exe binary, with the binary itself.
We are using the x64 version of the windows-kill.exe from:
https://github.com/ElyDotDev/windows-kill/releases/tag/1.1.4

We are now embedding the binary with the project directly to
decrease the complexity and avoid breaking the build by external
changes. Actually the maintainer of windows-kill changed his
GitHub username recently from "alirdn" to "ElyDotDev" which
broke the provisioning script!
@countzero
Copy link
Contributor

@remy I removed the provisioning script and added the windows-kill.exe binary instead.

You were right there to be sceptic. This way nodemon is way more resilient to external changes: Actually the maintainer of windows-kill changed his GitHub username recently from "alirdn" to "ElyDotDev" which broke the provisioning!

I updated https://github.com/countzero/nodemon_windows_kill to match my latest commit and added a PR for you: #1853

@countzero
Copy link
Contributor

@cowboyd & @wrexbe: Thank you for your suggestions. I am very interested in the native PowerShell only solution... What are your thoughts on why your suggestions are better than the windows-kill binary?

@wrexbe
Copy link

wrexbe commented Apr 29, 2021

@countzero I think the windows-kill binary is better. I couldn't get it to work correctly with just powershell, it is probably possible, but it might be a lot of work.

@remy
Copy link
Owner

remy commented May 5, 2021

Folks, there's a debug build available on npm from @countzero's great work.

Those with Windows, could you test the install using npm i -g nodemon@debug which should give you 2.0.8-alpha.a - and then check that it shuts the windows sub-process down correctly.

If you can comment here or on PR #1853 that would be perfect to help sign these changes off.

@wrexbe
Copy link

wrexbe commented May 5, 2021

2.0.8-alpha.a is correctly shutting down, and restarting for me.

It might help if 'SIGKILL' mapped to the previous method, to give people another option if the program they are working on ignores SIGINT, or just takes a long time, and they don't care about it safely exiting.

@countzero
Copy link
Contributor

@wrexbe: I like that idea!

So the use case is: As a developer on Windows I like nodemon to terminate the node process by force, to quickly restart my node process that does not require a graceful shutdown.

Under Windows nodemon would then have two behaviours:

  1. Graceful Shutdown: If the --signal option is not SIGKILL nodemon will gracefully terminate the node process group with a SIGINT. That means SIGINT will be send to the process group and nodemon waits for node to gracefully shutdown itself.

  2. Hard Shutdown: If the --signal option is SIGKILL nodemon will terminate the node process group by force with taskkill /t /f. In this case we are not able to send any signal under windows because sadly there is no such thing as SIGKILL.

@remy: If you are OK with this behaviour, I can extend the PR. Just need some time...

countzero added a commit to countzero/nodemon that referenced this issue May 9, 2021
This commit adds back the hard shutdown behaviour under Windows
as an opt-in: If the --signal option is "SIGKILL" nodemon will
terminate the process group by force without waiting for the
process to clean-up. This matches a SIGKILL on a UNIX system.
@countzero
Copy link
Contributor

countzero commented May 9, 2021

@wrexbe Done. Please take a look at countzero@f7c6e0d and @remy this commit is also included in the PR #1853!

@countzero
Copy link
Contributor

countzero commented May 18, 2021

@wrexbe @chriswoodle @cowboyd @nicholas-ochoa @JulianNicholls @usm076 @caesay @oviecodes @GIDustin @sridharjatla Do you have some time to test this solution to our shared problem?

You can simply check out https://github.com/countzero/nodemon_windows_kill to rapidly test this fix.

The SIGINT behaviour can be tested with:

npm run watch-graceful-shutdown

And the "old" SIGKILL behaviour with:

npm run watch-hard-shutdown

See https://github.com/countzero/nodemon_windows_kill/blob/main/package.json for technical details.

@remy I am not quite sure what else can be done here. What are the next steps?

@rjamesnw
Copy link

rjamesnw commented May 23, 2021

@remy That alpha works for me also, thanks! :) Just ran into this, but glad to find this thread.

@GIDustin
Copy link

GIDustin commented Jun 2, 2021

Your test project worked fine for me. I even tested without a SIGINT handler to ensure that apps without one would still function as they did in the past and that worked as well.

@chriswoodle
Copy link
Author

@countzero Seems to work for me in CMD, PowerShell, Git Bash:
image

However, I'm not sure if this is related, but your example repo doesen't detect file changes when running in WSL (I tested in WSL 2 Debian). Saving index.js does not trigger a restart
image

I tested your repo on the example repo I linked (https://github.com/chriswoodle/nodemon-1567) and I get the same behavior.

@countzero
Copy link
Contributor

countzero commented Jun 16, 2021

@chriswoodle I am currently using WSL 1 because I am working frequently with VirtualBox. This works with Debian:

image

Please note, that I did NOT modify the process restart behaviour of nodemon in a UNIX context. If you can reproduce this behaviour in a Debian under Microsofts WSL 2 you found a new bug which is unrelated to the changes discussed in this issue!

I am curious if you can reproduce this with the latest nodemon version...

@chriswoodle
Copy link
Author

@chriswoodle I am currently using WSL 1 because I am working frequently with VirtualBox. This works with Debian:

image

Please note, that I did NOT modify the process restart behaviour of nodemon in a UNIX context. If you can reproduce this behaviour in a Debian under Microsofts WSL 2 you found a new bug which is unrelated to the changes discussed in this issue!

I am curious if you can reproduce this with the latest nodemon version...

@countzero Yea, looks like this is unrelated bug, that issue happens on latest nodemon 2.0.7

@chriswoodle
Copy link
Author

chriswoodle commented Jun 21, 2021

Yep, I've opened a new issue: #1866

@countzero 's repo seems to work just fine.

remy pushed a commit that referenced this issue Jun 24, 2021
remy added a commit that referenced this issue Jun 25, 2021
* 'master' of github.com:remy/nodemon:
  fix: add support for SIGINT on Windows (fixes issue #1720) (#1853)
@remy
Copy link
Owner

remy commented Jun 25, 2021

Merged and live.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests