diff options
author | Ben Noordhuis <info@bnoordhuis.nl> | 2015-02-10 11:18:46 -0700 |
---|---|---|
committer | Trevor Norris <trev.norris@gmail.com> | 2015-03-19 12:06:56 -0700 |
commit | 2411bea0dfa9d48f94f95119bbacce122509681e (patch) | |
tree | 74126b3eb3def3f573abc63e1a85edae3be3800e | |
parent | c0766eb1a4dac9d9b555be271ef84ad401ba91b0 (diff) | |
download | node-2411bea0dfa9d48f94f95119bbacce122509681e.tar.gz |
lib: fix stdio/ipc sync i/o regression
process.send() should be synchronous, it should block until the message
has been sent in full, but it wasn't after the second-to-last libuv
upgrade because of commit libuv/libuv@393c1c5 ("unix: set non-block
mode in uv_{pipe,tcp,udp}_open"), which made its way into io.js in
commit 07bd05b ("deps: update libuv to 1.2.1").
Commit libuv/libuv@b36d4ff ("unix: implement uv_stream_set_blocking()")
as landed in io.js in commit 9681fca ("deps: update libuv to 1.4.0")
makes it possible to restore the synchronous behavior again and that's
precisely what this commit does.
The same line of reasoning applies to `net.Socket({ fd: 1 })`: creating
a socket object from a stdio file descriptor, like the `process.stdout`
getter does, should put the file descriptor in blocking mode for
compatibility reasons.
Reviewed-By: Julien Gilli <julien.gilli@joyent.com>
PR-URL: https://github.com/joyent/node/pull/9179
-rw-r--r-- | lib/child_process.js | 7 | ||||
-rw-r--r-- | lib/net.js | 6 | ||||
-rw-r--r-- | test/simple/test-child-process-sync-process-send.js | 44 | ||||
-rw-r--r-- | test/simple/test-net-sync-stdio.js | 44 |
4 files changed, 98 insertions, 3 deletions
diff --git a/lib/child_process.js b/lib/child_process.js index b4a889473..8b29132cb 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -586,6 +586,13 @@ exports._forkChild = function(fd) { // set process.send() var p = createPipe(true); p.open(fd); + + // p.open() puts the file descriptor in non-blocking mode + // but it must be synchronous for backwards compatibility. + var err = p.setBlocking(true); + if (err) + throw errnoException(err, 'setBlocking'); + p.unref(); setupChannel(process, p); diff --git a/lib/net.js b/lib/net.js index 6d12a70ae..70baab3bd 100644 --- a/lib/net.js +++ b/lib/net.js @@ -155,9 +155,9 @@ function Socket(options) { } else if (!util.isUndefined(options.fd)) { this._handle = createHandle(options.fd); this._handle.open(options.fd); - if ((options.fd == 1 || options.fd == 2) && - (this._handle instanceof Pipe) && - process.platform === 'win32') { + // this._handle.open() puts the file descriptor in non-blocking + // mode but it must be synchronous for backwards compatibility. + if ((options.fd == 1 || options.fd == 2) && this._handle instanceof Pipe) { // Make stdout and stderr blocking on Windows var err = this._handle.setBlocking(true); if (err) diff --git a/test/simple/test-child-process-sync-process-send.js b/test/simple/test-child-process-sync-process-send.js new file mode 100644 index 000000000..241364f08 --- /dev/null +++ b/test/simple/test-child-process-sync-process-send.js @@ -0,0 +1,44 @@ +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. + +var common = require('../common'); +var assert = require('assert'); +var fork = require('child_process').fork; +var N = 4 << 20; // 4 MB + +for (var big = '*'; big.length < N; big += big); + +if (process.argv[2] === 'child') { + process.send(big); + process.exit(42); +} + +var proc = fork(__filename, ['child']); + +proc.on('message', common.mustCall(function(msg) { + assert.equal(typeof msg, 'string'); + assert.equal(msg.length, N); + assert.equal(msg, big); +})); + +proc.on('exit', common.mustCall(function(exitCode) { + assert.equal(exitCode, 42); +})); diff --git a/test/simple/test-net-sync-stdio.js b/test/simple/test-net-sync-stdio.js new file mode 100644 index 000000000..4599c81a0 --- /dev/null +++ b/test/simple/test-net-sync-stdio.js @@ -0,0 +1,44 @@ +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. + +var common = require('../common'); +var assert = require('assert'); +var spawn = require('child_process').spawn; +var N = 4 << 20; // 4 MB + +for (var big = '*'; big.length < N; big += big); + +if (process.argv[2] === 'child') { + process.stdout.write(big); + process.exit(42); +} + +var stdio = ['inherit', 'pipe', 'inherit']; +var proc = spawn(process.execPath, [__filename, 'child'], { stdio: stdio }); + +var chunks = []; +proc.stdout.setEncoding('utf8'); +proc.stdout.on('data', chunks.push.bind(chunks)); + +proc.on('exit', common.mustCall(function(exitCode) { + assert.equal(exitCode, 42); + assert.equal(chunks.join(''), big); +})); |