`killSignal` option has no effect
The killSignal option inherited from child_process.spawn() has no effect. If this this option is specified it should set the default signal for killing the child as per the documentation of child_process.spawn().
For example:
// parent.mjs
import { execa } from "execa"
const child = execa("node", ["child.js"], { killSignal: "SIGINT", stdio: "inherit" })
setTimeout(() => {
child.kill()
}, 100)
// child.mjs
const onExit = require('signal-exit')
onExit((code, signal) => {
console.log(`child received ${signal}`)
})
setInterval(() => {}, 1000)
If I run node parent.mjs I expect to see child received SIGINT. Instead I see child received SIGTERM.
Hi @geigerzaehler,
killSignal is the signal used for terminations due to the timeout, signal or maxBuffer options, not explicit termination with child.kill(). The reason is because specific signals can be passed directly to child.kill(signal).
Note: this is not specific to Execa, i.e. child_process.spawn() has the same behavior.
Thanks for clearing this up @ehmicky and sorry I misunderstood the purpose of killSignal.
The reason for the misunderstanding on my side came from the following use case I have: I want to be able to configure the signal that is used to kill processes when the parent exits. I assumed that I could use killSignal for this but that’s not the case. Concretely, I want the following to work
// parent.mjs
import { execa } from "execa"
const child = execa("node", ["child.mjs"], { killSignal: "SIGINT", stdio: "inherit" })
setTimeout(() => {
process.exit(0)
}, 100)
// child.mjs
import onExit from 'signal-exit'
onExit((code, signal) => {
console.log(`child received ${signal}`)
})
setInterval(() => {}, 1000)
The output should be child received SIGTERM instead of child received SIGTERM.
Happy to open a feature request issue for this and close this issue.
When a parent process exits, its child processes are terminated by the OS, not by Node.js. Therefore, it is not possible to control which signal is being sent.
(Note: when the child process is detached, Unix does not terminate it (unlike Windows), which is why Execa provides with a cleanup option, but that's a different topic).
I believe the only way for you to control which signal is being sent there would be to explicitly call child.kill(signal).
When a parent process exits, its child processes are terminated by the OS, not by Node.js. Therefore, it is not possible to control which signal is being sent.
I thought the following code was responsible for killing the process if it is not detached: https://github.com/sindresorhus/execa/blob/e4b929547ade335b0dd9d7afe1a35989e19fbc8a/lib/kill.js#L95-L97
The documentation also states, that the cleanup option controls this behavior. I tested it and if I SIGKILL the parent so that the exit hook is not run or set cleanup to false the child stays around. (If you press Ctrl+C in the shell that runs the parent the child will be terminated in any case—but this is due to the shell sending SIGINT to all processes in process group.)
I believe the only way for you to control which signal is being sent there would be to explicitly call
child.kill(signal).
Yes, that sounds like a good workaround. But this requires more discipline on the caller side since you need to collect all the processes in an array and then call .kill() on exit. This is what we’re doing here. So a setting that allows us to control the signal send to children on shutdown of the process would make this a lot easier.
Thanks for the additional context, I see what you mean in terms of complexity from the caller's standpoint.
You're also correct about the cleanup option. To be clear, this option is currently only meant to fix the cross-platform issue I mentioned above, as opposed to as a way to customize the signal being sent.
Introducing an option to customize the signal sent by the kill() of the cleanup option is feasible. However, there might be the following issues limiting the usefulness of that feature:
- This would only work when
detachedisfalse. - This would not work when the parent process is crashing or is terminated with
SIGKILL.
I am curious what are your thoughts on this @sindresorhus?
I'm not against having a cleanupKillSignal option added, but it's also not something I have time/interest in reviewing a PR for (meaning @ehmicky would need to be willing to review it). It's solving a rather obscure use-case.
Revisiting this issue, I think our current implementation of killSignal is inconsistent:
- It is used by the
timeoutandsignaloptions. - It is not used by the
cleanupoption,maxBufferoption,.cancel()method. It also not used when the process is killed on stream errors or child processerrorevent.
If a user is specifying a killSignal, it means that signal is the best way to terminate the process. Therefore, it should be used in all the cases above.
A second point is that while child_process.spawn() does not use killSignal as the default value for the .kill() method, I do not see a good reason not to. I think most users would assume this to be the case, which includes @geigerzaehler.
It would also simplify implementing the first point above: instead of passing killSignal in multiple places in the codebase, we can just make it the default value for .kill().
@sindresorhus What do you think about making the killSignal option the default value for the .kill() method's first argument? If neither the killSignal nor the .kill() method's first argument is set (which is probably almost all our users), SIGTERM would still be used.
@sindresorhus What do you think about making the killSignal option the default value for the .kill() method's first argument? If neither the killSignal nor the .kill() method's first argument is set (which is probably almost all our users), SIGTERM would still be used.
👍 As long as it's clearly documented.