tapjs / foreground-child

Run a child as if it's the foreground process. Give it stdio. Exit when it exits.
http://tapjs.github.io/foreground-child/
ISC License
39 stars 14 forks source link

Forward IPC messages to foregrounded child process #10

Closed addaleax closed 8 years ago

addaleax commented 8 years ago

If the current process has an IPC channel available as indicated by the presence of process.send, redirect all messages from the parent process to the foregrounded child and vice versa.

Ref: https://github.com/tapjs/spawn-wrap/issues/23

/cc @jamestalmage @isaacs

jamestalmage commented 8 years ago

Also, this discussion: https://github.com/jamestalmage/__nyc-ipc-argv-bug/issues/1

isaacs commented 8 years ago

Looks like the obviously correct fix to me. The Travis failure seems like a timing issue in another test. When I get back to a non-iOS keyboard later today I'll land, unless someone else wants to beat me to it. Lgtm.

addaleax commented 8 years ago

@isaacs I think the tests failures are resolved by #11… and good to hear verification that this is what foreground-child is supposed to do, thanks. :)

isaacs commented 8 years ago

So, at some more digging, it turns out that we can in fact determine what fd was used as the IPC channel, but only by relying on some exposed-but-internal API surface in Node. (Not like this module isn't already doing a bit of that, of course).

At first, I remembered that the NODE_IPC_FD environ gets set to tell the child proc which fd to use for ipc. However, that environment variable gets stripped out at the start of the process.

It turns out that the IPC pipe object exposes its fd, though, and it's on process._channel.

However, other file descriptors are not detectable, and will always be cut off by foreground-child (and thus by spawn-wrap). Also, if the parent process is using fd=2 or something for IPC, then I imagine weird stuff could happen by setting stderr to inherit.

// ipc.js
var fs = require('fs')
if (process.argv[2] === 'child') {
  console.log(process._channel)
  fs.writeSync(3, 'some-data\n')
} else {
  var fd = fs.openSync('some-file', 'w')
  var spawn = require('child_process').spawn
  var child = spawn(process.execPath, [__filename, 'child'], {
    stdio: [
      'inherit',
      'inherit',
      'inherit',
      fd,
      'ipc'
    ]
  })
  child.on('close', function (code, sig) {
    console.log(code, sig)
    console.log(fs.readFileSync('some-file', 'utf8'))
  })
}
$ node ipc.js
Pipe {
  _externalStream: {},
  fd: 4,
  writeQueueSize: 0,
  buffering: false,
  onread: [Function],
  sockets: { got: {}, send: {} } }
0 null
some-data

So, I think that the best approach is to land this PR (which I'm about to do) and then just explain in the readme that other arbitrary FDs won't be mapped in the foregrounded child proc.