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

Stdout is not completely flushed on process exit #2972

Closed
tschaub opened this issue Sep 21, 2015 · 12 comments
Closed

Stdout is not completely flushed on process exit #2972

tschaub opened this issue Sep 21, 2015 · 12 comments
Labels
duplicate Issues and PRs that are duplicates of other issues or PRs. process Issues and PRs related to the process subsystem.

Comments

@tschaub
Copy link

tschaub commented Sep 21, 2015

The following fails with Node 4.1 (but passes on 0.12 and 0.10) on OS 10.10.5:

var assert = require('assert');
var spawn = require('child_process').spawn;

var out = new Array(Math.pow(2, 13)).join('.') + '!!';

if (process.argv[2] === '--child') {
  process.stdout.write(out);
  process.exit(0);
} else {
  var child = spawn(process.argv[0], [process.argv[1], '--child']);

  var got = '';
  child.stdout.on('data', function(data) {
    got += String(data);
  });

  child.on('exit', function() {
    assert.equal(got, out);
  });
}

The child writes 8193 bytes to stdout. The parent only gets 8192 before the exit.

See also nodejs/node-v0.x-archive#8329.

@brendanashworth brendanashworth added the process Issues and PRs related to the process subsystem. label Sep 21, 2015
@mscdex
Copy link
Contributor

mscdex commented Sep 21, 2015

Duplicate of #784, #1771, #1741, #2148.

@tschaub
Copy link
Author

tschaub commented Sep 21, 2015

It's not clear to me from the above issues that they refer to the same regression between 0.12 and 4.0.

In #2148, you mention the same problem occurs with 0.12 (this is not the case here). #1741 and #2148 are also about writing to stdout/err in a loop.

The test case above writes a single string to stdout. It fails when the string is 8193 bytes. It passes at 8192. And this only occurs with Node 4 (not 0.10 or 0.12).

@bnoordhuis
Copy link
Member

The issue is that process.stdout.write() is asynchronous but doesn't get time to run to completion due to the process.exit() call immediately afterwards. It's a variation of (at least some of) the issues @mscdex linked to.

@bnoordhuis
Copy link
Member

Forgot to mention, in your example you can fix the issue by dropping the immediate process.exit() call and rewriting the process.stdout.write() call to process.stdout.write(out, process.exit).

@tschaub
Copy link
Author

tschaub commented Sep 21, 2015

Forgot to mention, in your example you can fix the issue by dropping the immediate process.exit() call and rewriting the process.stdout.write() call to process.stdout.write(out, process.exit).

Thanks for the extra detail @bnoordhuis. Still curious what exactly changed since 0.12 that causes the test above to fail at 8192 bytes. Any chance this size is configurable?

In my case, I cannot change where process.exit() is called in the child process.

@bnoordhuis
Copy link
Member

Still curious what exactly changed since 0.12 that causes the test above to fail at 8192 bytes. Any chance this size is configurable?

Stdio is fully asynchronous now (and that's not configurable.) It used to be synchronous, mostly, but with so many caveats that even core team members often didn't know when it would or wouldn't be.

@tschaub
Copy link
Author

tschaub commented Sep 23, 2015

Stdio is fully asynchronous now (and that's not configurable.) It used to be synchronous, mostly, but with so many caveats that even core team members often didn't know when it would or wouldn't be.

Thanks @bnoordhuis. It looks like the code from 20176a9 is still there, and the docs were recently changed to say that process.stdout.write() always blocks (see 892bf65). So process.stdout.write() blocks but is asynchronous?

It sounds like the only way to ensure that process.stdout.write() results in a write to stdout is to never call process.exit() (assuming you may not control all the call sites of process.stdout.write() or call it in a loop etc.).

@Fishrock123
Copy link
Contributor

Duplicate of #784, #1771, #1741, #2148.

Oh dear, this again.

It sounds like the only way to ensure that process.stdout.write() results in a write to stdout is to never call process.exit() (assuming you may not control all the call sites of process.stdout.write() or call it in a loop etc.).

Yes. process.exit() terminates the process as soon as possible. It's not "safe" per-say.

Still curious what exactly changed since 0.12 that causes the test above to fail at 8192 bytes. Any chance this size is configurable?

That's when something at OS(?)-level starts breaking it into chunks.

@Fishrock123
Copy link
Contributor

(Closing as a dupe, feel free to continue discussing)

@Fishrock123
Copy link
Contributor

Also, updated the breaking change documentation on this: https://github.com/nodejs/node/wiki/API-changes-between-v0.10-and-v4#process

@bnoordhuis
Copy link
Member

For people coming here from Google, this:

Stdio is fully asynchronous now

Is correct at the time of writing, except when stdio is redirected to a file. That includes special files like /dev/null and /dev/zero but excludes /dev/tty, /dev/stdout, etc. and bash's /dev/tcp pseudo-files.

See also #3170, where I was very careful to use the word 'may' in 'may block'. :-)

orangejulius added a commit to pelias/fuzzy-tester that referenced this issue Apr 4, 2016
`process.exit` quits immediately, without allowing buffered output from
say, a `console.log` to be printed. See these Node.js issues:

nodejs/node#3669
nodejs/node#3170
nodejs/node#2972 (comment)
@mikemaccana
Copy link
Contributor

See #6456 for the latest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate Issues and PRs that are duplicates of other issues or PRs. process Issues and PRs related to the process subsystem.
Projects
None yet
Development

No branches or pull requests

6 participants