Kill child when exiting about execa

[sindresorhus]

This is how the native childProcess.exec() works too. It would indeed be useful, but I don't really see how it would be possible to know what processes are executed as it's the shell that spawns them, not Node.js. Thoughts? I would suggest you instead use execa('ipfs', ['daemon']) both for performance and security reasons.

[dignifiedquire]

This is how the native childProcess.exec() works too

Yes, sadly :( That's why I thought this might be a good place to improve on it :)

but I don't really see how it would be possible to know what processes are executed

What I currently do is keep track of the processes I spawn and attach handlers to exit and SIGINT on process to clean them up immediately before node.js exits. I totally understand if you think this is out of scope for this library but I wanted to mention as I can imagine me not being the only person needing this.
It probably would make sense to have this behind an option though rather than enabling it by default.

I would suggest you instead use execa('ipfs', ['daemon']) both for performance and security reasons.

The thing is that ipfs is not a node.js script but rather a binary out of my control.

[sindresorhus]

execa('ipfs', ['daemon']) works with any binary, not just Node.js.

[dignifiedquire]

execa('ipfs', ['daemon']) works with any binary, not just Node.js.

Right..silly me. But I encounter the same issue with that, the process is spawned but not terminated when the parent exits.

[sindresorhus]

Hmm. I assumed Node.js would clean up direct child processes when exiting. Sounds like something it should do. Can you try opening an issue on Node.js first? And comment the link here. Would be better to try to fix it for everyone, not just this module. If they decline, we can add it here.

[dignifiedquire]

Sure, I will do that later tonight

[dignifiedquire]

Just filed an issue with a minimal example: nodejs/node#7951

[dignifiedquire]

So as you can see from the issue this is the expected unixy behaviour. I still think it would be a great addition to optionally destroy the spawned process in this module. To catch that as referenced in the issue we could use https://www.npmjs.com/package/signal-exit.

Let me know if you are interested and I can work on a PR for adding this tracking behaviour.

[sindresorhus]

I'm not surprised. Unexpected behavior usually is expected UNIX behavior...

Sure, PR welcome :)