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: stdout/stderr flaky on CentOS 5 #2563

Closed
Trott opened this issue Aug 26, 2015 · 4 comments
Closed

child_process: stdout/stderr flaky on CentOS 5 #2563

Trott opened this issue Aug 26, 2015 · 4 comments
Labels
child_process Issues and PRs related to the child_process subsystem. invalid Issues and PRs that are invalid.

Comments

@Trott
Copy link
Member

Trott commented Aug 26, 2015

This is currently (but hopefully not for long--I have a PR to fix it) the contents of test/parallel/test-process-argv-0.js:

'use strict';
var util = require('util');
var path = require('path');
var assert = require('assert');
var spawn = require('child_process').spawn;
var common = require('../common');

console.error('argv=%j', process.argv);
console.error('exec=%j', process.execPath);

if (process.argv[2] !== 'child') {
  var child = spawn(process.execPath, [__filename, 'child'], {
    cwd: path.dirname(process.execPath)
  });

  var childArgv0 = '';
  var childErr = '';
  child.stdout.on('data', function(chunk) {
    childArgv0 += chunk;
  });
  child.stderr.on('data', function(chunk) {
    childErr += chunk;
  });
  child.on('exit', function() {
    console.error('CHILD: %s', childErr.trim().split('\n').join('\nCHILD: '));
    assert.equal(childArgv0, process.execPath);
  });
}
else {
  process.stdout.write(process.argv[0]);
}

From time to time on CentOS 5 only, stderr and stdout from the child process stop working after the first line is written to stderr.

See, for example this centos5-32 test result or this centos5-64 test result.

See further discussion at #2541.

CentOS Project will support CentOS 5 until March 31, 2017.

@Trott Trott added confirmed-bug Issues with confirmed bugs. child_process Issues and PRs related to the child_process subsystem. labels Aug 26, 2015
@mscdex
Copy link
Contributor

mscdex commented Aug 26, 2015

Shouldn't the test be listening for the child's close event instead of its exit event to ensure that all output is received?

@Trott
Copy link
Member Author

Trott commented Aug 26, 2015

If I'm understanding you (@mscdex) correctly, you're saying that there's no guarantee that exit on the child process will fire after the data events on the stdio streams? And when it doesn't, this test blows up? That this issue is (was) a bug in the old test code and not in node?

And if so, do you feel confident about that and closing this issue is in order? Or would you prefer someone else weigh in?

@mscdex
Copy link
Contributor

mscdex commented Aug 26, 2015

exit can definitely fire before all output from a child process is received. close was added a long time ago to combat this. I think the commit you pushed to fix this effectively does the same thing though, it's just that the assertion will happen a little bit later than the child process's close event.

So yes, my guess is that changing exit to close would have fixed the test and it wasn't a bug in node/iojs. However, this can be closed now since your fix will work too.

@Trott
Copy link
Member Author

Trott commented Aug 26, 2015

Awesome. Closing because this isn't a bug in node.

@Trott Trott closed this as completed Aug 26, 2015
@Trott Trott added invalid Issues and PRs that are invalid. and removed confirmed-bug Issues with confirmed bugs. labels Aug 26, 2015
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. invalid Issues and PRs that are invalid.
Projects
None yet
Development

No branches or pull requests

2 participants