-
-
Notifications
You must be signed in to change notification settings - Fork 243
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
Have to hit CTRL + c twice to kill run-all in terminal #74
Comments
Thank you for this issue, and I'm sorry that I delayed.
(I don't have macOS, so I cannot confirm the behavior) |
At least, on Windows, |
@mysticatea I can help with debugging this issue on OSX Sierra. |
So - I have spent some time dealing with the windows/CTRL+C issue and would like to help. There are actually other problems: in CMD shells CTRL+C will only kill the parent process. The child processes are detached and must be killed manually. I think most people use git bash, which may handle this better since it uses cygwin process model, but I use TCC/LE (a CMD replacement) and this comes up a lot. I hope nobody actually uses CMD itself, but that also is legitimate and will come up. This is actually a bug with "npm run" itself, too. So - I have made changes to npm-run-all in my fork that solves this problem and ensures that CTRL+C will be trapped and correctly terminate the child processes. I would like to make a pull request, but I am concerned about the impact my changes would have on non-windows environments or possibly even use cases other than my own basic one (starting parallel tasks from command line). The specific thing I'm concerned about is that in order for this to work correctly in windows, To sum up the changes generally: The basic issue in windows is that there's no actual SIGINT signal. So when dealing with child processes, any code that depends on propagating SIGINT won't work. (This includes node spawn). In order to actually capture SIGINT you need to listen on stdin for a SIGINT, and then manually terminate child processes. So - while the shell might correctly terminate the outermost process when you hit CTRL+C, the SIGINT signal doesn't actually do anything when sent to child processes. Additionally, it is possible for the child process itself to receive the CTRL+C signal rather than the parent process (and I don't understand exactly why this is, but it's easily reproducible) so you also need to handle possible user-initiated terminations of a child process and treat it differently from the process just ending normally or with an error (e.g. - always do an "abort") |
For me, most of the time CTRL + C doesn't even work when calling it multiple times. Need to exit the CMD and reboot sometimes 👎 |
CTRL+C doesn't work at all for me on Win 10 |
Same problem on Mac OS high sierra. Capturing the signals and pass along to the children with 8s auto-exit timeout should solve. |
Same problem for me using Git Bash/Windows 7. CTRL+C doesn't abort the processes, and I have to manually end them in task manager. |
Same for me too, Windows 10/Git bash, CTRL+C doesn't abort the processes and freezes up bash. Attempting to run browser-sync server and watch tasks, not sure if that is relevant. |
For me, have to hit several times CTRL+C to kill the process. |
Been a big proponent of npm-run-all for a long time but in https://github.com/begin-examples/node-create-react-app I have to hold down ctrl + c to get it to eventually terminate. Switched to concurrently and it works fine. 🤷 |
I have use it for twice, but I'm happy with that. :D |
I replaced the {
"scripts": {
"prestart": "run-s clean build-ts",
"start": "npm run watch-ts & npm run watch-node",
"watch-node": "delay 1 && node --enable-source-maps --watch dist/index.js",
"watch-ts": "tsc --watch",
"win-start": "run-p watch-ts watch-node"
}
} It's not cross platform, but Windows devs can use the |
On macOS Sierra 10.12.1 when I use the npm-run-all in my start script I have to hit ctrl+c twice for it to kill the process. Would expect I can just hit ctrl+c once and that would kill the process.
The text was updated successfully, but these errors were encountered: