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

process.on beforeExit broken on Windows #9067

Closed
joaocgreis opened this issue Oct 12, 2016 · 7 comments
Closed

process.on beforeExit broken on Windows #9067

joaocgreis opened this issue Oct 12, 2016 · 7 comments
Labels
process Issues and PRs related to the process subsystem. windows Issues and PRs related to the Windows platform.

Comments

@joaocgreis
Copy link
Member

  • Version: all >=4.0.0
  • Platform: Windows
  • Subsystem: v8, lib (?)

Investigating #8821 (comment) , I found that process.on('beforeExit', ...) is broken on Windows if node is not invoked from the Windows Command Prompt. Here is a simple repro:

const child = require('child_process');
//const script = "console.log('start');"; // OK
//const script = "process.on('exit', () => console.log('exit')); console.log('start');"; // OK
const script = "process.on('beforeExit', () => console.log('beforeExit')); console.log('start');";
const proc = child.spawn(process.execPath, ['-e', script], { encoding: 'utf8' });
proc.stdout.on('data', (data) => console.log(`stdout: ${data}`));
proc.stderr.on('data', (data) => console.log(`stderr: ${data}`));

This triggers an infinite loop printing beforeExit to stdout. This happens when node spawns itself (as in the repro above) or when node is started from gitbash. This does not happen when node is started from the Windows Command Prompt with

node -e "process.on('beforeExit', () => console.log('beforeExit')); console.log('start')"

but happens with

node -e "process.on('beforeExit', () => console.log('beforeExit')); console.log('start')" | cat

so it might be related to output redirection or output not being a TTY.

Bisected, first bad commit is c431725 (V8 4.5 update before node v4.0.0).

cc @nodejs/platform-windows @bnoordhuis

@joaocgreis joaocgreis added the windows Issues and PRs related to the Windows platform. label Oct 12, 2016
@mscdex mscdex added the process Issues and PRs related to the process subsystem. label Oct 12, 2016
@Fishrock123
Copy link
Member

@joaocgreis what happens if you follow it up by adding a setImmediate() to the end?

@joaocgreis
Copy link
Member Author

node -e "process.on('beforeExit', () => console.log('beforeExit')); console.log('start'); setImmediate(function(){});"

does not make any difference, still gets stuck in a loop printing beforeExit.

@Fishrock123
Copy link
Member

@joaocgreis I meant, does that make it loop with a pipe, or still not?

@bnoordhuis
Copy link
Member

node -e "process.on('beforeExit', () => console.log('beforeExit')); console.log('start')" | cat

That isn't too surprising. The same could happen on Unices with a slow reader and a full pipe.

Bisected, first bad commit is c431725 (V8 4.5 update before node v4.0.0).

That seems an unlikely culprit, seeing how it only touches files in deps/v8.

@seishun
Copy link
Contributor

seishun commented Oct 13, 2016

Seems to be expected behavior to me. console.log is non-blocking when writing to a pipe, so calling it schedules additional work for the event loop, effectively cancelling the exit.

@sam-github
Copy link
Contributor

Expected behaviour as described by @seishun, and documented: https://nodejs.org/api/process.html#process_event_beforeexit

@joaocgreis
Copy link
Member Author

This was surprising for me. Thanks for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
process Issues and PRs related to the process subsystem. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

No branches or pull requests

6 participants