Ctrl+C must be hit twice to kill parallel tasks

[coryhouse]

Love this! There's one quirk I'd love to figure out. I'm sending two tasks to the same command line using npm-run-all. When I do, eslint-watch fails to be killed by the first Ctrl+C. I have to hit it twice. This only occurs on Mac. I created a simple repo to convey the issue.

It appears the first Ctrl+C kills eslint-watch, but it appears to swallow the kill signal and not pass it on to other running tasks. I can use many other tasks in parallel with npm-run-all and they all get killed when I hit Ctrl+C, so this quirk is unique to eslint-watch.

[rizowski]

Thanks for posting about this. I will take a look and see what I can dig up.

[rizowski]

@coryhouse I know I have had issues with npm-run-all before. but I'll have to look a little deeper into the issues to find it. Second note, I was looking at the sample project you provided, it doesn't provide the commands you might be running. Do you have two different watching processes running at the same time or anything like that? A little more detail should help figure out what might be going on.

I have seen this issue without the process being nested and definitely revolves around the watcher.
Looking up some information, here are some notes on exiting a process in nodejs: http://stackoverflow.com/a/9828572

[coryhouse]

Thanks for the link. I've never seen npm scripts require me to hit Ctrl+C twice to exit. Usually only hitting it once is required. The answer on StackOverflow that mentions that you have to hit it twice is referring to the Node REPL.

Here's an example script that I'm referring to. I have to hit Ctrl+C twice to kill that script.

As you can see, one of the tools it's calling is eslint-watch. If I remove the call to lint:watch (which ultimately calls eslint-watch), it only requires hitting Ctrl+C once to kill the process.

[rizowski]

@coryhouse A little update so I can keep you informed. I was messing around with react-slingshot and I noticed that as soon as you saved an asset eslint-watch would exit after picking up the file change event. I hooked up process.on('SIGINT') and process.on('uncaughtException') to see what might be happening. I have at least found that Eslint is choking on trying to load eslint-plugin-react. Not sure if you have run into that issue before. I double checked to ensure the plugin was installed.

If that is the case, I am thinking it is most likely an issue with eslint-watch. As for the process requiring (CTRL + C) x2, it seems to be linked to the exception being thrown. If I handle the exception it will exit on the first CTRL + C. A little more investigating is required, however, one step forward. 😄

[coryhouse]

Interesting. When you say "Eslint is choking with trying to load eslint-plugin-react", when is it "choking"?

[rizowski]

It happens after the file change event is fired. For some context, eslint-watch will spawn an eslint process for grabbing eslint's options and running the first lint. If the watch flag is specified, then it will use eslint's CLI-Engine for further linting.

[rizowski]

Linking #28 which also dealt with npm-run-all

[rizowski]

@coryhouse It might have been possible that I fixed this as well in the path-fix branch. Could you verify this for me?

[coryhouse]

Unfortunately, this issue persists on the branch. I ran the branch in React Slingshot to confirm.

[rizowski]

@coryhouse I took another swing at this on Friday and tried to figure out what might be the cause. I think it may be two things. It's possible I am not shutting down other libraries correctly (Chokidar keypress) and they are holding onto the process. Or it's possible there may be a problem with parallel tasks with npm-run-all. Eslint-watch exits just fine if there isn't anything else running alongside it. (Could be short circuiting to single task and not take the parallel workflow)

I am leaning more towards the first option. Either way, I think I have an idea on how to solve this problem. Bad thing is, it incorporates a new workflow for the tool which means a decent size refactor.

Still thinking about it, I just wanted to give you an update.

[coryhouse]

Thanks for digging further! 👍 Appreciate your hard work and the update.

[rizowski]

@coryhouse I've done several fixes for the project in general and I came back to this issue to see if there was something else I could do. I ended up messing around with passing SIGINT SIGKILL and even tried doing process.abort(). I took pictures of the results:

Here is the logs with npm start being called on react-slingshot, eslint-watch process id being 17677:

You can see that process running here in my monitor:

I press CTRL + C and get the following executed code right before aborting.

Process monitor showing the pid no longer exists:

Even after that, it is still not exiting npm-run-all. I have tried looking up ways to identify if the process is being wrapped by a parent process such as detecting if the process.send function exists. It seems strange that the function exists regardless of if it is forked or spawned from a parent process. The documentation clearly states here that it would be undefined.

I am starting to wonder if there is a bug with the library npm-run-all uses for spawning child threads? I'm not entirely sure. I think the next step is to remove the key listener that I have on stdin and see if that still holds the parent process. If that doesn't then I am not entirely sure what would be causing the issue. 💩

[coryhouse]

Tricky business! Thanks so much for digging in and for the update!

[rizowski]

@coryhouse You are using windows correct?

[coryhouse]

I use both. Especially important since I author JavaScript courses for people on all platforms. :) Same story with the React Slingshot project I mentioned which is one example of where this issue is occurring. Thanks again for investigating!

[rizowski]

I am going to remove this from the 3.0 milestone and release the changes that have been made so far. I am hoping to get this fixed soon.

[rizowski]

@coryhouse @brokentone or anyone else that may have run into this issue. I tried testing V5 on a previous version of React Slingshot to see if I could still reproduce the issue. Upgrading to esw@V5 made the issue go away. Although I've noticed that React Slingshot has since moved onto concurrently which does not seem to produce the problem. Sorry it took so long, but it should be fixed now in V5.

(Currently pushing out a few minor issues that weren't foreseen with the v5 release. But that should be out shortly.)

[coryhouse]

Great to hear! Thanks for the fix and followup @rizowski!