child_process.spawn sh not killable

[paulpflug]

Comming from here:
darkguy2008/parallelshell#22

(btw: Usecase for #1009)

Everytime I need to spawn a process I do it like this

if (process.platform === 'win32') {
 sh = 'cmd';
 shFlag = '/c';
} else {
 sh = 'sh';
 shFlag = '-c';
}
var child = spawn(sh,[shFlag,cmd], {
 cwd: process.cwd,
 env: process.env,
 stdio: ['pipe', process.stdout, process.stderr]
})

The problem: child is unkillable on unix.
Example in coffee-script:

spawn = require("child_process").spawn
child = spawn "sh", ["-c", "node -e 'setTimeout(function(){},10000);'"]
child.on "close", process.exit
child.kill()
child.kill("SIGINT")
child.kill("SIGTERM")
child.kill("SIGHUP")
spawn "sh",["-c","kill -TERM "+child.pid]
spawn "sh",["-c","kill -INT "+child.pid]
spawn "sh",["-c","kill -HUP "+child.pid]

(on windows it works fine with sh replaced by cmd)
Even when I exit the process with process.exit(), the child stays alive.

Workaround I found:

spawn = require("child_process").spawn
child = spawn "sh", ["-c", "node -e 'setTimeout(function(){},10000);'"], detached: true
child.on "close", process.exit
spawn "sh",["-c","kill -INT -"+child.pid]

Killing by pgid works, by pid works not. Sending ^C from console works also, I assume it uses pgid always.

[bnoordhuis]

I don't think that's a bug. Your example sends signals to sh, not node. sh may elect to ignore SIGINT, SIGTERM, etc.

The reason it works with {detached: true } is that it creates a new session group, i.e., it calls setsid(2). In that case, PID == PGID, and sending a signal will deliver it to all processes in the group.

[paulpflug]

But with detached: false I would expect sh to get cleaned up on process.exit()

That should be the point of the detached option?

[bnoordhuis]

I think you have the wrong idea about processes and process groups. On exit, child processes get reparented, they're not forcibly killed by io.js or the kernel.

{detached: true } turns the child process into a group leader, that's really all it does.

[paulpflug]

From the node docs: options.detached

This makes it possible for the child to continue running after the parent exits.

What I read from that sentence: if detached = false the child get force killed

On exit, child processes get reparented, they're not forcibly killed by io.js or the kernel.

What you say is, that the child is always continuing after the parent exits, regardless of detached
I would definitivly expect different behavior as a user.
At least, we have to alter the docs 😄

[bnoordhuis]

The documentation for options.detached is not wrong as such, but it's not the whole story.

What you say is, that the child is always continuing after the parent exits, regardless of detached

Not quite. {detached: true } means, among other things, that the child process won't receive a SIGINT when you kill the parent with ^C.

/cc @nodejs/documentation - the documentation can probably be improved, although I'm not sure where to draw the line between inline documentation and pointing people to a UNIX process management primer.

[okdistribute]

@paulpflug what exact platform were you using to test this? (any more specific than unix?) I found the same problem, works on mac but not on ubuntu. Seems to close properly if I use /bin/bash instead, got the idea from #4432

[paulpflug]

@Karissa ubuntu ;) I'm unsure of the reliability of bash

currently I live with using detached: process.platform != "win32" on spawn
and killing by gpid

if (process.platform != "win32") {
 spawn("sh", ["-c", "kill -INT -"+child.pid]);
 } else {
 child.kill("SIGINT");
}

I would really appreciate a more beautiful solution, but found none so far.
Thanks for pointing out, though.

[sam-github]

@paulpflug If you want to send a signal to an entire process group, using detached (on non-windows) to put the child and its descendants in a group is exactly what unix process gouping is intended for.

Your spawning of kill should be unnecessary, use process.kill(-child.pid, 'SIGINT') - this should work, and if it doesn't, file a bug report!

[paulpflug]

@sam-github that is cool, didn't know that. And it does work 👍

I created a small wrapper: better-spawn