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

child_process 'close' always (?) emitted after 'exit' #37998

Closed
addaleax opened this issue Mar 31, 2021 · 2 comments
Closed

child_process 'close' always (?) emitted after 'exit' #37998

addaleax opened this issue Mar 31, 2021 · 2 comments
Labels
child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations.

Comments

@addaleax
Copy link
Member

  • Version: master
  • Platform: Linux 5.4.0-70-generic Mani #78-Ubuntu SMP Fri Mar 19 13:29:52 UTC 2021 x86_64 x86_64 x86_64 GNU/Linuxd
  • Subsystem: child_process

What steps will reproduce the bug?

const child_process = require('child_process');
const proc = child_process.spawn('bash', ['-c', 'exec 0>&- 1>&- 2>&-; sleep 5'], {
  stdio: ['inherit', 'pipe', 'inherit']
});
proc.stdout.pipe(process.stdout);
proc.on('exit', () => console.log('exit'));
proc.on('close', () => console.log('close'));

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior?

'close' should be emitted immediately (all stdio streams are closed), 'exit' after 5 seconds

What do you see instead?

5 seconds pass, then 'exit' is emitted and then 'closed'

@addaleax addaleax added the child_process Issues and PRs related to the child_process subsystem. label Mar 31, 2021
@Linkgoron
Copy link
Member

Linkgoron commented Mar 31, 2021

IMO this is a documentation issue, or at least this is the expected original behavior of 'close'. IMO this is also reflected in the callback arguments, which are either an exit code or a kill signal, which can only be provided if the process is finished. I'm not saying that the current behavior is correct, just that it's IMO what I would expect from the documentation.

@addaleax
Copy link
Member Author

@Linkgoron That’s fair, and it’s okay to say we don’t want to break this behavior by changing it, but in that case, it would really be nice to document this.

@addaleax addaleax added the doc Issues and PRs related to the documentations. label Mar 31, 2021
Linkgoron added a commit to Linkgoron/node that referenced this issue Apr 10, 2021
clarify the 'close' event description in the child_process docs.

fixes: nodejs#37998
BethGriggs pushed a commit that referenced this issue Apr 15, 2021
clarify the 'close' event description in the child_process docs.

fixes: #37998

PR-URL: #38181
Fixes: #37998
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this issue May 1, 2021
clarify the 'close' event description in the child_process docs.

fixes: #37998

PR-URL: #38181
Fixes: #37998
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
danielleadams pushed a commit that referenced this issue May 8, 2021
clarify the 'close' event description in the child_process docs.

fixes: #37998

PR-URL: #38181
Fixes: #37998
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants