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

fix: add support for SIGINT on Windows (fixes issue #1720) #1853

Merged
merged 5 commits into from Jun 24, 2021

Conversation

countzero
Copy link
Contributor

This adds a windows-kill.exe binary which handles the sending of the POSIX signal SIGINT on Windows.

The repository https://github.com/countzero/nodemon_windows_kill contains a a cross-platform compatible test to quickly verify the correct behaviour of this fix.

This commit adds an install script that runs on 'npm install'. Its
purpose is to download, extract and verify a specific version of
the windows-kill.exe binary.

I decided not add the - windows only - executable directly to this
project to keep the dependencies clear. Furthermore I did not use
the node-window-kill package, because it comes with node-gyp which
requires python and vs2017 build tools on the target machine.
This commit replaces the taskkill invocation with windows-kill.exe.

We are now sending a SIGINT POSIX signal to the node process in a
Windows context.

Known issues: The windows-kill.exe has to be triggered in a detached
process context, which currently flashes a terminal window on each
reload. Furthermore the 'restart' event is triggered twice on the bus.
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.
…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!
@remy
Copy link
Owner

remy commented May 5, 2021

I've published a debug build to npm - npm i -g nodemon@debug should give you 2.0.8-alpha.a - let's get some windows testing with that and ship this PR.

Really great work seeing this through 👍

@remy remy added the not-stale Tell stalebot to ignore this issue label May 5, 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 Author

countzero commented May 9, 2021

@remy: I added the hard shutdown behaviour on SIGKILL only. This supports the use case of @wrexbe and is a nice addition!

@chriswoodle
Copy link

2.0.8-alpha.a works fine for me on windows.

@remy
Copy link
Owner

remy commented Jun 24, 2021

Right, this has been quiet for a bit, but let's ship this - I reckon it'll help those windows users - I've seen quite a few similar issues recently so hopefully this nails it.

Really excellent work @countzero

@remy remy merged commit 500c1b0 into remy:master Jun 24, 2021
remy added a commit that referenced this pull request 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

FFS - something has gone weird on travis - so this isn't actually live, so I'll need to fix whatever has happened then this will be live.

@github-actions
Copy link

🎉 This PR is included in version 2.0.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

@RepentAndBelieveTheGospel
Copy link

I get this with the latest version.

image

The system cannot find the file C:\Projects\node.js\complete-node-bootcamp\4-natours\starter\node_modules\nodemon\bin\windows-kill.exe.

I've initially installed nodemon globally, without adding it to the project dependencies. I've fixed this by doing a npm i nodemon --save-dev too.

Shouldn't it work by installing it globally only? It did before.

@remy
Copy link
Owner

remy commented Jun 30, 2021

@RepentAndBelieveTheGospel issue being tracked here: #1872

@RepentAndBelieveTheGospel

@remy Sorry, didn't check the opened issues.

@bentinata
Copy link

I'm not well-versed on Windows system and binaries, and I know compiling it is too much hassle for users. But can we have the source to the executables?

@hrieke
Copy link

hrieke commented Jul 27, 2022

@bentinata I believe the source code is from:
https://github.com/ElyDotDev/node-windows-kill
MIT Licensed

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

Successfully merging this pull request may close these issues.

None yet

6 participants