proper signal handling

[dherman]

Tool::exec doesn't propagate signals yet.

[dherman]

Some important links:

• unix:
  • https://doc.rust-lang.org/std/os/unix/process/trait.ExitStatusExt.html
• windows:
  • https://docs.microsoft.com/en-us/windows/console/ctrl-c-and-ctrl-break-signals
  • https://docs.microsoft.com/en-us/windows/console/setconsolectrlhandler

[charlespierce]

I did some investigation of this today. We don't currently propagate signals, however, what we do do is forward the STDIN to the child process. So if the user kills the process using a keyboard shortcut (like Ctrl + C to send SIGINT), that will be sent to the child process, and in fact will only go to the child process, instead of being interpreted by Volta.

However, if we send the SIGINT signal directly using e.g. kill -2, then it will interrupt the Volta process without affecting the child process, leading to some weird indeterminate behavior.

That said, I don't know if passing signals along is actually what we want to do, as the child process is a separate process, with a separate PID, from the parent. So it seems reasonable that a user would expect the processes to be mostly independent as far as direct signals are concerned.

Lastly, while the main shim child processes do forward along the STDIN, I'm not sure that all of our child processes do. So we should, at the very least, do some auditing to make sure that all of our child processes use .stdin(Stdio::inherit). This will make sure that when we pause waiting for a child process, we allow the Ctrl + C keyboard shortcut to propagate to the child process and allow those to be shut down gracefully as well.

[brendankenny]

Any chance of taking a second look at this issue?

I just ran into this in a CI script running a node server for testing. As a simple repro:

test-server.js

const server = require('http').createServer((_, response) => {
  response.end('hello there\n');
});
server.listen(10200);

test.sh

#!/bin/sh

node test-server.js &
sleep 5
curl http://localhost:10200
sleep 5
kill $!
curl http://localhost:10200

When using volta, the second curl call doesn't fail as it should, and a zombie node process spawned by http.Server is left running. Without volta the script runs as expected and there's no zombie process.

https://github.com/volta-cli/volta/issues/36#issuecomment-505724225 brings up good points about expectations (and ctrl-c mid-run does indeed properly kill the server process in this case, which is nice), but it seems like node is doing the right thing with signal handling in the non-volta case, and if you're a happy volta user but don't know much about volta shims (as I didn't until today :), it's not entirely clear how to know you need to handle this case, or, once you do, the best way to handle it.

[charlespierce]

@brendankenny Thanks for raising this up again! I think that the scripted use-case is an important point, since it's a spot where you can't (reasonably) send a Ctrl+C to the process to kill it, you instead need to be able to pass a signal.

[vajogaspar]

I'm working on an Electron App, which spawns child node processes (using the host machines' installed nodejs), and I need to terminate these processes with signals. When using Volta, childProcess.kill() won't terminate the child process, just the Volta Shim. Though process.kill(-childProcess.pid); does a good job on OSX and Linux as it kills the process group, but doesn't work on Windows. Is there any chance this gets fixed anytime soon?

[vanstinator]

That said, I don't know if passing signals along is actually what we want to do, as the child process is a separate process, with a separate PID, from the parent. So it seems reasonable that a user would expect the processes to be mostly independent as far as direct signals are concerned.

I would argue the opposite here. Volta shims should, arguably, be as transparent to the end user as possible. A user of Volta shouldn't need to work with the node process any differently than if they'd installed it natively. That in my mind was one of the nice selling points of Volta as a tool from the get-go in terms of how it holds node/npm/global versions of all your dependencies and seamlessly switches between them. Fudging with child processes to properly kill running processes seems to be the antithesis of how Volta sells itself. I realize actually implementing this might not be trivial. But it seems like an important piece to get right.

[charlespierce]

@vanstinator That's a good point and well stated, thanks! There's a lot of platform-specific nuance with signal handling (since Windows doesn't have signals in the same way as MacOS / Linux), but I agree that getting it right will make Volta more appropriately transparent, especially for these "launch as a sub-process" use-cases.

[pnearing]

Any progress on this? I'm developing a process manager that works with Volta, and I want to shutdown a given node process.

[dividedmind]

I'm working on an Electron App, which spawns child node processes (using the host machines' installed nodejs), and I need to terminate these processes with signals. When using Volta, childProcess.kill() won't terminate the child process, just the Volta Shim. Though process.kill(-childProcess.pid); does a good job on OSX and Linux as it kills the process group, but doesn't work on Windows. Is there any chance this gets fixed anytime soon?

Second that. I just wasted two hours trying to figure out why my processes weren't being terminated properly (and kept hugging ports) in a similar scenario. This makes volta nigh unusable.