Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detaching child process from parent fails to execute child #115

Closed
SamVerschueren opened this issue Nov 21, 2017 · 3 comments · May be fixed by nju33/remonade#121
Closed

Detaching child process from parent fails to execute child #115

SamVerschueren opened this issue Nov 21, 2017 · 3 comments · May be fixed by nju33/remonade#121

Comments

@SamVerschueren
Copy link
Contributor

SamVerschueren commented Nov 21, 2017

We had an issue in Alfy haunting us for a while. Alfred only shows the results when the process exits. In order to detect if a workflow has an update, we spawn a new process which does this for us in the form of alfred-notifier.

This is how we did it first

execa('check.js');

Then we noticed what was going on and tried to unref the child process.

const cp = execa('check.js');
cp.unref();

Unfortunately, this is not enough. In order to detach the child process, you must set the detached flag to true and make sure there is no communication channel between child and parent.

const cp = execa('check.js', {
    detached: true,
    stdio: 'ignore'
});
cp.unref();

However, this results in check.js not being executed. Using child_process directly however, did the trick.

const subProcess = cp.spawn('check.js', [], {
    detached: true,
    stdio: 'ignore'
});
subProcess.unref();

How to reproduce

Create two files, child.js and index.js.

child.js

#!/usr/bin/env node
'use strict';
const name = process.argv[2];

setTimeout(() => {
	console.log(`Hello World from ${name}`);
}, 3000);

index.js

'use strict';
const path = require('path');
const fs = require('fs');
const cp = require('child_process');
const execa = require('execa');

const outCp = fs.openSync('out-cp.log', 'w');
const outExeca = fs.openSync('out-execa.log', 'w');

// Spawn the child process with `child_process`
const subProcess = cp.spawn(path.join(__dirname, 'child.js'), ['child_process'], {
	detached: true,
	stdio: ['ignore', outCp, outCp]
});
subProcess.unref();

// Spawn the child process with `execa`
const subProcess2 = execa(path.join(__dirname, 'child.js'), ['execa'], {
	detached: true,
	stdio: ['ignore', outExeca, outExeca]
});
subProcess2.unref();

Run node index.js. The process exits immediately and you will notice that two files are being created, out-cp.log and out-execa.log. After 3 seconds, the content of out-cp.log changes but the file out-execa.log keeps being empty.

@kasselTrankos
Copy link

hi, im using
node --version ::: v6.12.0
and when run node index.js
give me this error:

internal/child_process.js:328
throw errnoException(err, 'spawn');
^

Error: spawn EACCES
at exports._errnoException (util.js:1020:11)
at ChildProcess.spawn (internal/child_process.js:328:11)
at Object.exports.spawn (child_process.js:369:9)
at Object. (/home/vera/help-man/index.js:11:23)
at Module._compile (module.js:570:32)
at Object.Module._extensions..js (module.js:579:10)
at Module.load (module.js:487:32)
at tryModuleLoad (module.js:446:12)
at Function.Module._load (module.js:438:3)
at Module.runMain (module.js:604:10)
need upgrade my node ?
thanks

@SamVerschueren
Copy link
Contributor Author

Run chmod +x child.js

@SamVerschueren
Copy link
Contributor Author

SamVerschueren commented Dec 19, 2017

I debugged this issue and found out that when I set cleanup to false, it works as expected. Probably because we unref the child process, causing the child to to think the parent exited and kills itself.

Should we set cleanup to false when detached is set to true? Guess it makes sense. Not sure what we shoul do if detached is set to true and cleanup is explicitely set to true as well.

tomsotte added a commit to tomsotte/execa that referenced this issue Jan 17, 2019
… well

This attempt to fix the problem described by the issue sindresorhus#96.
If a child process is killed, the descendents of that child process won't be killed as well. This happens on Linux but not on Windows [^1].
A solution is the "PID range hack" [^2] that uses the `detached` mode for spawning a process and then kills that child process by killing the PID group, using `process.kill(-pid)`.

*Implementation*

- added an internal option `killByPid` as a remained for the spawned child process that it will be `detached` and to kill it by PID
- expanded and moved to a separate function the routine to kill the spawned process to `killSpawned`
- the `ChildProcess#kill` method of the spawned child process will be replaced by the `killSpawned` routine, to kill by pid if necessary
- the `killSpawned` routine signals that the child process has been killed, if it has been killed by the pid

I checked and all the tests pass.

This implementation also consider the issue sindresorhus#115 and shouldn't interfere with the detached/cleanup fix.

[^1]: https://nodejs.org/api/child_process.html#child_process_subprocess_kill_signal
[^2]: https://azimi.me/2014/12/31/kill-child_process-node-js.html
tomsotte added a commit to tomsotte/execa that referenced this issue Jan 17, 2019
This is an attempt to fix the problem described by the issue sindresorhus#96.
If a child process is killed, the descendents of that child process won't be killed as well. This happens on Linux but not on Windows [^1].
The adopted solution is the "PID range hack" [^2] that uses the `detached` mode for spawning a process and then kills that child process by killing the PID group, using `process.kill(-pid)`, effectively killing all the descendents.

*Implementation*

- added an internal option `killByPid` as a remained for the spawned child process that it will be `detached` and to kill it by PID
- expanded and moved to a separate function the routine to kill the spawned process to `killSpawned`
- the `ChildProcess#kill` method of the spawned child process will be replaced by the `killSpawned` routine, to kill by pid if necessary
- the `killSpawned` routine signals that the child process has been killed, if it has been killed by the pid

I checked and all the tests pass.

This implementation also considers the issue sindresorhus#115 and shouldn't interfere with the detached/cleanup fix.

[^1]: https://nodejs.org/api/child_process.html#child_process_subprocess_kill_signal
[^2]: https://azimi.me/2014/12/31/kill-child_process-node-js.html
tomsotte added a commit to tomsotte/execa that referenced this issue Jan 20, 2019
This is an attempt to fix the problem described by the issue sindresorhus#96.
If a child process is killed, the descendents of that child process won't be killed as well. This happens on Linux but not on Windows [^1].
The adopted solution is the "PID range hack" [^2] that uses the `detached` mode for spawning a process and then kills that child process by killing the PID group, using `process.kill(-pid)`, effectively killing all the descendents.

*Implementation*

- added an internal option `killByPid` as a remained for the spawned child process that it will be `detached` and to kill it by PID
- expanded and moved to a separate function the routine to kill the spawned process to `killSpawned`
- the `ChildProcess#kill` method of the spawned child process will be replaced by the `killSpawned` routine, to kill by pid if necessary
- the `killSpawned` routine signals that the child process has been killed, if it has been killed by the pid

I checked and all the tests pass.

This implementation also considers the issue sindresorhus#115 and shouldn't interfere with the detached/cleanup fix.

[^1]: https://nodejs.org/api/child_process.html#child_process_subprocess_kill_signal
[^2]: https://azimi.me/2014/12/31/kill-child_process-node-js.html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants