Skip to content

Commit

Permalink
test: ensure _handle property existence
Browse files Browse the repository at this point in the history
`test-stdtout-close-unref.js` will fail if `process.stdin._handle` does
not exist. On UNIX-like operating systems, you can see this failure this
way:

    ./node test/parallel/test-stdout-close-unref.js < /dev/null

This issue has been experienced by @bengl and @drewfish in a Docker
container. I'm not sure why they are experiencing it in their
environment, but since it is possible that the `_handle` property does
not exist, let's use `child_process.spawn()` to make sure it exists.

PR-URL: #5916
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
  • Loading branch information
Trott committed Apr 1, 2016
1 parent 0551021 commit a20c700
Showing 1 changed file with 26 additions and 12 deletions.
38 changes: 26 additions & 12 deletions test/parallel/test-stdout-close-unref.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,30 @@
'use strict';
require('../common');
var assert = require('assert');
const common = require('../common');
const assert = require('assert');
const spawn = require('child_process').spawn;

var errs = 0;
if (process.argv[2] === 'child') {
var errs = 0;

process.stdin.resume();
process.stdin._handle.close();
process.stdin._handle.unref(); // Should not segfault.
process.stdin.on('error', function(err) {
errs++;
});
process.stdin.resume();
process.stdin._handle.close();
process.stdin._handle.unref(); // Should not segfault.
process.stdin.on('error', function(err) {
errs++;
});

process.on('exit', function() {
assert.strictEqual(errs, 1);
});
process.on('exit', function() {
assert.strictEqual(errs, 1);
});
return;
}

// Use spawn so that we can be sure that stdin has a _handle property.
// Refs: https://github.com/nodejs/node/pull/5916
const proc = spawn(process.execPath, [__filename, 'child'], { stdio: 'pipe' });

proc.stderr.pipe(process.stderr);
proc.on('exit', common.mustCall(function(exitCode) {
if (exitCode !== 0)
process.exitCode = exitCode;

This comment has been minimized.

Copy link
@Fishrock123

Fishrock123 Apr 1, 2016

Contributor

@Trott btw, if the process errors after this, the exit code will be overriden anyways.

If node core set it before, it won't reach here because that means it must be exiting somewhere, afaik.

This comment has been minimized.

Copy link
@Trott

Trott Apr 1, 2016

Author Member

@Fishrock123 True for the test as it currently stands. FWIW, I'm coding defensively in case someone adds another test later and the file ends up looking something like this:

'use strict';
const spawn = require('child_process').spawn;

if (process.argv[2] === 'child') {
  throw new Error('ERRORED');
}

if (process.argv[2] === 'child2') {
  return;
}

const proc = spawn(process.execPath, [__filename, 'child']);
proc.on('exit', (exitCode) => {
  console.error('Currently, the exit code is set to', process.exitCode);
  process.exitCode = exitCode;
  console.error('I just set the exit code to', exitCode);
});

const proc2 = spawn(process.execPath, [__filename, 'child2'])
proc2.on('exit', (exitCode) => {
  // Oops, this could override an error code from `proc`.
  // Worse, it's timing-dependent so it could be flaky.
  console.error('The exit code is now', process.exitCode);
  process.exitCode = exitCode;
  console.error('And now I have set it to', exitCode);
});
}));

0 comments on commit a20c700

Please sign in to comment.