nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
106.66k stars 29.09k forks source link

Ctrl + C Doesn't Kill Gracefully | Node v6.11.3 - Git Bash #16103

Closed dark-swordsman closed 6 years ago

dark-swordsman commented 6 years ago

Referencing #4182 which seems similar (but at the end, it was stated that a new issue should be opened).

Ctrl + C does not kill the server. The resolution to the issue was using:

process.on('SIGINT', function() {
    process.exit();
});

however, when I use it, my server exits immediately. It completely skips app.listen() and exits completely, even when I previously kill the process on the specific port. Without the process.exit() function, it starts up and I can Ctrl + C to exit, but the process still runs in the background. I opened an issue on StackOverflow which hasn't gone anywhere really. This happens across multiple projects, so it's not just one project. I'm pretty sure it started happening when I updated node.

I am able to use netstat -aon to list it, then tskill <pid> to kill it afterwards, but this isn't really a solution.

Here's my code with my express server:

const express = require('express');
const app = express();

var config = require('./config')(process.env.NODE_ENV);
var appPort = process.env.PORT || config.port;

app.listen(appPort, function () {
  console.log('UAction Test Server Running on Port ' + appPort + '!');
});

process.on('SIGINT', function() {
  console.log( "\nGracefully shutting down from SIGINT (Ctrl-C)" );
  // some other closing procedures go here
  process.exit(1);
});

UPDATE So the server is able to start now with:

process.on('SIGINT', function() {
  console.log( "\nGracefully shutting down from SIGINT (Ctrl-C)" );
  // some other closing procedures go here
  process.exit(1);
});

I added app.use('/', express.static('./src'));, but removing it didn't change anything.

When I do Ctrl + C, the console.log("\nGracefully shutting down from SIGINT (Ctrl-C)" ); doesn't appear, and trying to restart causes the same issue. Just for reference, here's the log:

$ npm start

> express-test@1.0.0 start E:\github\uaction\express-test
> node index.js

events.js:160
      throw er; // Unhandled 'error' event
      ^

Error: listen EADDRINUSE :::1337
    at Object.exports._errnoException (util.js:1020:11)
    at exports._exceptionWithHostPort (util.js:1043:20)
    at Server._listen2 (net.js:1258:14)
    at listen (net.js:1294:10)
    at Server.listen (net.js:1390:5)
    at EventEmitter.listen (E:\github\uaction\express-test\node_modules\express\lib\application.js:618:24)
    at Object.<anonymous> (E:\github\uaction\express-test\index.js:9:5)
    at Module._compile (module.js:570:32)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)

npm ERR! Windows_NT 10.0.15063
npm ERR! argv "C:\\Program Files\\nodejs\\node.exe" "C:\\Program Files\\nodejs\\node_modules\\npm\\bin\\npm-cli.js" "start"
npm ERR! node v6.11.3
npm ERR! npm  v3.10.10
npm ERR! code ELIFECYCLE
npm ERR! express-test@1.0.0 start: `node index.js`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the express-test@1.0.0 start script 'node index.js'.
npm ERR! Make sure you have the latest version of node.js and npm installed.
npm ERR! If you do, this is most likely a problem with the express-test package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR!     node index.js
npm ERR! You can get information on how to open an issue for this project with:
npm ERR!     npm bugs express-test
npm ERR! Or if that isn't available, you can get their info via:
npm ERR!     npm owner ls express-test
npm ERR! There is likely additional logging output above.

npm ERR! Please include the following file with any support request:
npm ERR!     E:\github\uaction\express-test\npm-debug.log
refack commented 6 years ago

Hi @dark-swordsman,

I can't repro. Is there a specific way that you start the process?

dark-swordsman commented 6 years ago

I start the process like any other. The package.json looks at "start": "node index.js". I use npm start to start the process. The process then starts with the following console messages:

$ npm start

> express-test@1.0.0 start E:\github\uaction\express-test
> node index.js

UAction Test Server Running on Port 1337!

I usually do Ctrl + C to exit the process, which usually gracefully kills it and it doesn't run in the background. What ends up happening is the command line returns to it's previous state:

$ npm start

> express-test@1.0.0 start E:\github\uaction\express-test
> node index.js

UAction Test Server Running on Port 1337!

DarkSwordsman@FUJIWARA_TOFU_SHOP MINGW64 /e/github/uaction/express-test (phonegap-testing)
$

Doing Ctrl + C without any process running results in a ^C being printed, which is normal behavior.

Then from there, doing npm start again (which I usually do and have done forever) results in a EADDRINUSE error (detailed above in OP).

netstat -aon shows the localhost process running with a PID (which is random every time):

$ netstat -aon

Active Connections

  Proto  Local Address          Foreign Address        State           PID
  TCP    0.0.0.0:135            0.0.0.0:0              LISTENING       1100
  TCP    0.0.0.0:445            0.0.0.0:0              LISTENING       4
  TCP    0.0.0.0:1337           0.0.0.0:0              LISTENING       8012

then tskill <pid> is used to kill it.

I did find this in the documentation, which may be relevant, but I am not sure:

"SIGTERM and SIGINT have default handlers on non-Windows platforms that resets the terminal mode before exiting with code 128 + signal number. If one of these signals has a listener installed, its default behavior will be removed (Node.js will no longer exit)."

According to that, I am not sure what I may have installed... unless it may be firebase tools? I will uninstall them temporarily and see if that fixes it, since I also changed that recently.

UPDATE Uninstalling firebase, firebase-tools, phonegap, and cordova did not fix the issue.

refack commented 6 years ago

UPDATE Uninstalling firebase, firebase-tools, phonegap, and cordova did not fix the issue.

By my guess it wouldn't be a general CLI tool but rather something that traps keystrokes like ComEnu or clink. There are several players in you scenario:

  1. minnty
  2. bash
  3. npm
  4. node
  5. conhost
  6. possibly other keystroke traps

I'll try to reproduce so I can investigate, but the simplest answer I can give you, don't launch node from mintty (Ref: https://github.com/nodejs/node/pull/16104)

dark-swordsman commented 6 years ago

I just tried using Node.js command prompt (A CMD with Node: "Your environment has been set up for using Node.js 6.11.3 (x64) and npm.") and it was able to successfully kill and restart the process. It also confirms if I want to Terminate batch job:

E:\GitHub\uaction\express-test>npm start

> express-test@1.0.0 start E:\GitHub\uaction\express-test
> node index.js

UAction Test Server Running on Port 1337!
Terminate batch job (Y/N)? Y

E:\GitHub\uaction\express-test>npm start

> express-test@1.0.0 start E:\GitHub\uaction\express-test
> node index.js

UAction Test Server Running on Port 1337!
Terminate batch job (Y/N)? Y

E:\GitHub\uaction\express-test>

So I can confirm that this is an issue with (at least) MinTTY, hopefully not with Git Bash completely.

I will continue to find anything that would interfere with Bash (I may start with just a clean re-install; perhaps different settings) since I would prefer not to work in CMD... Lol

EDIT: I want to mention I had Git version 2.14.1 and am going to install version 2.14.2.2 which is the latest from Git.

UPDATE: Updating to Git version 2.14.2.2 did not work. I used the following settings:

- Use Git from Git Bash only
- Use the OpenSSL Library
- Checkout Windows-style, commit Unix-style line endings
- Use MinTTY (the deafult terminal of MSYS2)

- Enable file system caching
- Enable Git Credential Manager
- Disable symbolic links

I will come back and reinstall changing from MinTTY to CMD.

dark-swordsman commented 6 years ago

The issue was MinTTY. I don't know why, but changing to the Default Console Window fixed the issue.

EDIT: I referenced this issue in git-for-windows/git/issues/1219 which seems to be the most relevant issue. @dscho said something about MSYS2 being modified since v2.13.1 (which is what MinTTY uses) that causes this.

Here are the settings:

- Use Git from Git Bash only
- Use the OpenSSL Library
- Checkout Windows-style, commit Unix-style line endings
- Use Default Console Window (CMD)

- Enable file system caching
- Enable Git Credential Manager
- Disable symbolic links

Here's it gracefully closing and restarting:

DarkSwordsman@FUJIWARA_TOFU_SHOP MINGW64 /e/github/uaction/express-test (phonegap-testing)
$ npm start

> express-test@1.0.0 start E:\github\uaction\express-test
> node index.js

UAction Test Server Running on Port 1337!

DarkSwordsman@FUJIWARA_TOFU_SHOP MINGW64 /e/github/uaction/express-test (phonegap-testing)
$ npm start

> express-test@1.0.0 start E:\github\uaction\express-test
> node index.js

UAction Test Server Running on Port 1337!

DarkSwordsman@FUJIWARA_TOFU_SHOP MINGW64 /e/github/uaction/express-test (phonegap-testing)
$

Everyone tells me Mac is better, and I'm starting to believe that more and more everyday, but I really don't want to fork over thousands when I already have a great working computer.

Might try ElementaryOS though. 🤔

Sorry for cluttering Node with another issue, but hopefully this will help being here since many people may think it's Node's fault like I did.

refack commented 6 years ago

Everyone tells me Mac is better, and I'm starting to believe that more and more everyday, but I really don't want to fork over thousands when I already have a great working computer.

There are a few of us who develop node on Windows, so it's not a completely uncharted territory. Personally I use ComEmu to run most of my shells; "Git bash", cmd, and "WSL bash", and I don't have zombie process problems. I believe the problem you were experiencing was the shell was disconnecting from the process, but the process didn't receive the signal (similar in a sense to nohup). You could try running winpty npm start, I'm not sure but it might make sure the process dies if the tty streams are closed.

dark-swordsman commented 6 years ago

@refack doing that, I get an error:

DarkSwordsman@FUJIWARA_TOFU_SHOP MINGW64 /e/github/uaction/express-test (phonegap-testing)
$ winpty npm start
winpty: error: cannot start 'npm': Not found in PATH

I have never used or heard of winpty before, and searching for a solution to that error didn't return anything.

danikane commented 6 years ago
  • git version 2.14.2.windows.3
  • node version v6.11.4
  • Windows 10 x64

Hi all, so I experienced the same issue lately and as indicated here zombie processes were present when launching node in the git bash terminal MinTTY - launched from the default Git Bash shortcut and from the context menu on a folder - located in C:\Program Files\Git So I decided to use bash.exe located in C:\Program Files\Git\bin and voilà- no more zombie processes! Then I modified the shortcut and the windows shell context menu and no more nuisances.

See the screenshot below: image

Hope this helps.

KiritoCheng commented 6 years ago

I also met the same problem, this is the latest git bash bug, if back to the previous version can kill the process node

martinmckenna commented 6 years ago

chiming in here to say I'm having the same issue.

Switching away from MinTTY fixed the issue

hottredpen commented 6 years ago

i has same problem ,but my colleague use old git-bash version not the problem , so I use the git-bash version 2.8.1 now :)

dark-swordsman commented 6 years ago

@KiritoCheng @martinmckenna @hottredpen I suggest you guys post that in the actual issue with a little more information. git-for-windows/git#1219

I'm sure it will help dscho pinpoint the issue a little more.

dscho commented 6 years ago

I'm sure it will help dscho pinpoint the issue a little more.

I am pretty certain that I know what the culprit is. It is time that is lacking.

Essentially, this code needs to be replaced with code that injects the thread in the entire process tree, i.e. reuses the enumeration in terminate_process_tree() even for the gentle case.

But it also needs to send a MSYS2/Cygwin signal instead if any of the encountered PIDs is actually referring to an MSYS2/Cygwin process. But only for that arm of the process tree.

Not an easy project. And as long as I am the only one to answer questions on the bug tracker like "why does my terminal not show any input anymore after I forget the - in ls | vim -?", it is unlikely that I will have the time to tackle involved projects such as overhauling the Ctrl+C handling in Git for Windows' Bash.

Of course, if one (or more) of you gentle people could give me a hand with those more mundane tickets. Then. Then I could...

SamanthaAdrichem commented 6 years ago

I also have this problem on windows. It starts occuring when i have an npm script with && in it like a && b

then the CTRL + C command is never send to the b script.

$ npm -v 5.5.1

$ node --version v8.9.1

sedulous-mortal commented 6 years ago

I was able to get all my node processes to die directly from the Git Bash shell on Windows 10 by typing taskkill -F -IM node.exe - this ends all the node processes on my computer at once. I found I could also use taskkill //F //IM node.exe. Not sure why both - and // work in this context. Hope this helps!

SamanthaAdrichem commented 6 years ago

We're just running it in phpstorm now instead of git bash. phpstorm does propperly kill the process for it kills the terminal.

But thnx though!

dscho commented 6 years ago

Thanks to an excellent contribution by the excellent @afsmith92, the next Git for Windows version will come with a proper fix.

You can even get it early by downloading https://bintray.com/git-for-windows/pacman/download_file?file_path=x86_64%2Fmsys2-runtime-2.9.0-6-x86_64.pkg.tar.xz and unpacking its usr\bin\msys-2.0.dll over your existing Git's.

afsmith92 commented 6 years ago

After further testing, it turns out my change didn't end up fixing the issue, but @dscho generously stepped in and took the time to resolve it.

giovannipds commented 6 years ago

It's occurring to me too, node v8.9.3, Git for Windows 2.16.1 (x64), Windows 10 (x64), using ConEmu by cmder. Tried what @dscho suggested but it didn't solve it. Tried to update Node to v8.9.4 too, same thing... It "fixes" for a while when I restart.

ktsangop commented 6 years ago

@dscho and @afsmith92 thank you for your effort! I can confirm that it works for my setup :

38leinaD commented 6 years ago

I am facing the same issue for Gradle instead of nodejs:

Git for windows version 2.16.2 Windows 7 x64

Fixed when applying the patch from @dscho . Should i create a seperate issue for it? It seems to be the same/related problem.

dscho commented 6 years ago

Fixed when applying the patch from @dscho

Then it should be fixed in Git for Windows v2.17.0, which was released yesterday...

msdevstep commented 6 years ago

I can confirm that v2.17.0 resolved the issue after installing over the top of my existing installation. Thanks @dscho

38leinaD commented 6 years ago

@dscho I collegue of mine and me has the same result: "./gradlew tasks" or similar gradle commands do not exit on Ctrl+C

It does not work for us on 2.17.0. It only works again when copying in the dll from here afterwars: https://bintray.com/git-for-windows/pacman/download_file?file_path=x86_64%2Fmsys2-runtime-2.9.0-6-x86_64.pkg.tar.xz

Git for windows version 2.17.0 Windows 7 x64

dscho commented 6 years ago

It does not work for us on 2.17.0

There is exactly one way how it can work. But there are gazillions of ways for it not to work. Would you care to impart some details?

38leinaD commented 6 years ago

@dscho what kind of information would help? To summarize:

Git for windows version 2.17.0
Windows 7 x64
java version "1.8.0_161"
Java(TM) SE Runtime Environment (build 1.8.0_161-b12)
Java HotSpot(TM) 64-Bit Server VM (build 25.161-b12, mixed mode)
curl -O https://gradle.org/next-steps/?version=4.6&format=bin
unzip gradle-4.6-bin.zip
mkdir testapp
cd testapp/
../gradle-4.6/bin/gradle init --type java-application
../gradle-4.6/bin/gradle clean build // press CTRL+C

This task takes a few seconds to download depdenencies the first time; so just any example where you cannot kill the process.

Now i apply your msys-2.0.dll from this thread and try it again.

../gradle-4.6/bin/gradle clean build // press CTRL+C Now it works.

dscho commented 6 years ago

Now i apply your msys-2.0.dll from this thread and try it again.

../gradle-4.6/bin/gradle clean build // press CTRL+C Now it works.

I guess the culprit is that I had to change this again because it caused more problems. Ctrl+C is really a vexing problem, and whatever way we do it, it seems to break somebody's scenario.

So currently we call GenerateConsoleCtrlEvent() because that was the only way node.js programs could react to the signal. But then your Java example seems to fail to interrupt? What seems to work for you is the thing we did before, where we inserted a remote thread calling ExitProcess() (and we had to enumerate the process tree to do it correctly), but the problem with that was that the SIGINT could not be caught because there was none.

Damn if you do, damn if you don't.

dscho commented 6 years ago

Okay, so I spent way too many hours on trying to fix this, but I have something to show: The newest Git for Windows snapshot at https://wingit.blob.core.windows.net/files/index.html should fix this. Please test.

codynova commented 6 years ago

@dscho Thanks for your hard work on this, it's been plaguing me forever. I actually had a script just to call taskkill on node. I've tested the snapshot (commit e6ee2f786d) on Windows 10 and Node v9.8 - it's working like a charm! 😁

38leinaD commented 6 years ago

@dscho For the gradle-scenario described above it does not work. the process does not terminate on ctrl-c.

steranka commented 5 years ago

For the record, this happens with cygwin as well.

$ cygcheck --version
cygcheck (msys) 2.6.0
System Checker for Msys
Copyright (C) 1998 - 2016 Cygwin Authors

I searched and found this command helps me know what process to kill:

wmic process where "name like '%node%'" get processid,commandline

Found from [https://superuser.com/questions/1003921/how-to-show-full-command-line-of-all-processes-in-windows]()

dscho commented 5 years ago

For the record, this happens with cygwin as well.

$ cygcheck --version
cygcheck (msys) 2.6.0
System Checker for Msys
Copyright (C) 1998 - 2016 Cygwin Authors

That looks more like MSYS than Cygwin. (And a pretty old one at that.)

ozyman42 commented 5 years ago

Does anyone know of a workaround for the Mintty shell that ships with MSYS2? I don't want to give up MSYS2 or use git bash, because I love tmux and pacman too much.

neddongo commented 3 years ago

So what about those of us who want to use mintty? We're just SOL? This literally doesn't happen with any other program but Node, so why do you blame the terminal?