Allow child.kill() and exec() to kill child's children

[arturadib]

A situation frequently encountered when running a script via exec() is that the script itself spawns other child processes. Specifying the {timeout:...} option to exec() kills only the script but not its children, and it seems desirable to allow Node to kill a child's children for a clean timeout kill.

Would it make sense to add an option to exec(), say e.g. {killSubchildren:true}, to handle such cases?

PS: I poked around the code and noticed /bin/kill is not being used, and a C-version is used instead (in process_wrap.cc). With /bin/kill, we can simply do e.g. kill -TERM -PARENT_ID to kill all children processes of parent PARENT_ID. Not sure how to go about this in process_wrap.cc.

[arturadib]

It wasn't explicit above, but this will probably require recursively traveling down the child's process tree and killing all found processes, not just the grandchildren.

[bnoordhuis]

You can accomplish this by passing {setsid:true} to child_process.spawn() so the child and its children run in a new process group and then later on kill said process group with process.kill(-child_pid, signal).

I'm in two minds about adding this to core: I kind of see why it'd be useful but how often will people really need it? It's probably a pain to support on Windows too.

[arturadib]

I must be missing something trivial, but I can't make setsid spawn a new group ID. Whether or not I set it to true, my child processes all get the same pgid of the node process. For example, here are the processes running after spawning a bash script that calls sleep:

  PID  PPID   GID  RGID  PGID COMM
24449 21855    20    20 24449 node
24451 24449    20    20 24449 /bin/bash
24452 24451    20    20 24449 sleep

So e.g. if I kill process group 24449 it will kill the parent process node too, which I don't want. Tried Node v0.4.7 and latest master branch, doesn't matter.

Any ideas?

PS: Could this be because of the new child_process_uv? I can't seem to track down where setsid() is called from there; neither process_wrap.cc nor process.c seem to call it. node_child_process.cc does seem to call setsid(), but I can't seem to trace it back to child.spawn().

[arturadib]

I just ran a grep -R 'setsid' * in test/ and nothing came up.

Should we have a unit test for setsid?

[bnoordhuis]

PS: Could this be because of the new child_process_uv?

Yes, you need to use 0.4.12 or the --use-legacy switch with master. The libuv back end doesn't support setsid.

[dominictarr]

there is a subtle difference between how PGID and PPID kill -$SIG -$PID (negative pid) kills the process group, not all the children of the PID.
negative pid means PGID, not PPID.

this problem is confusing because exec actually creates a sh which parses the command, and runs it.

this is misleading in the docs...

I encountered this problem the other day, but I made a module to find all the child pids.
https://github.com/dominictarr/ps-tree

[eladb]

Related to joyent/libuv#338

[jasnell]

@chrisdickinson was a decision ever made to actually close this one?

[jasnell]

Closing due to lack of activity.

[davidrapin]

For those (like me) we arrive late in this discussion and are looking for an answer:

• for a process to kills it's children when it dies, it has to be the leader of its process group
• to make a process the leader of its process group, the only option in nodejs (at least since v4) is to use the {detached: true} option in spawn or exec 

see https://nodejs.org/api/child_process.html#child_process_options_detached:

On non-Windows platforms, if options.detached is set to true, the child process will be made the leader of a new process group and session. Note that child processes may continue running after the parent exits regardless of whether they are detached or not. See setsid(2) for more information.

[chesucr]

I am using exec() to run a command from shell and I had to face this problem. So, in order to kill the child process of sh I had to install the module ps-tree.

[davisjam]

Late to the party, but here's a recipe for spawning children and killing all of their descendants after a timeout. This particular function is intended to work with an async.parallel call. Works on Linux.

/* Input: (repo, id, timeout, cb)
 * Output: ()
 * Calls cb(obj)
 * obj: keys repo, logdir
 * logdir: name of logdir, 'ERROR' if get-re fails, 'TIMEOUT' if it takes too long
 */
function doStuff (stuff, id, timeout, cb) {
  var pref = 'doStuff ' + id + ': ';

  var resultObj = { "stuff": stuff, "logdir": "ERROR" };

  /* There's two ways to call cb:
   *  - via successful child process completion (perhaps with an error, NBD)
   *  - via timeout
   * Each must prevent the other from calling cb.
   */

  var timeout_timer = null;
  var timed_out = false;

  /* Start a child. */
  var spawnOpts = { 'detached' : true // Allows killing of all of child's descendants.
                                      // cf. http://azimi.me/2014/12/31/kill-child_process-node-js.html
                                      //     https://github.com/nodejs/node-v0.x-archive/issues/1811
                  }
  log(0, pref + 'child_process.spawn: get_stuff <' + get_stuff + '> args [\'' + stuff + '\']');
  var child = child_process.spawn(stuff, [stuff], spawnOpts);

  /* Handle ongoing progress from child. */
  var stdout = '';
  var stderr = '';

  child.stdout.on('data', function (data) { stdout += data; });
  child.stderr.on('data', function (data) { stderr += data; });
  child.on('close', function (rc) {
    /* If we timed_out already, nothing to do. */
    if (timed_out)
      return; // timeout_timer has already called cb

    /* Cancel the timeout_timer, then get logdir and call cb. */

    // Cancel the timeout timer (defined below).
    if (timeout_timer)
      clearTimeout(timeout_timer);

    // Parse stdout for logdir
    var match = /Log files are in (.+)/.exec(stdout);
    if (match)
      resultObj.logdir = match[1];
    else
      log(0, pref + 'couldn\'t parse stdout ' + stdout);

    log(0, pref + 'repo <' + resultObj.stuff + '> logdir <' + resultObj.logdir + '>');
    return cb(resultObj);
  });

  /* Start a timeout timer. */
  timeout_timer = setTimeout(function() {
    /* Kill the child and its descendants, call cb with TIMEOUT. */
    log(0, pref + 'timed out');
    timed_out = true;
    try {
      // Kill child and all its descendants.
      process.kill(-child.pid, 'SIGTERM');
      process.kill(-child.pid, 'SIGKILL');
    } catch (e) {}

    resultObj.logdir = 'TIMEOUT';
    cb(resultObj);
  }, 1000*timeout);

  return;
};