summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBen Noordhuis <info@bnoordhuis.nl>2015-02-10 11:18:46 -0700
committerTrevor Norris <trev.norris@gmail.com>2015-03-19 12:06:56 -0700
commit2411bea0dfa9d48f94f95119bbacce122509681e (patch)
tree74126b3eb3def3f573abc63e1a85edae3be3800e
parentc0766eb1a4dac9d9b555be271ef84ad401ba91b0 (diff)
downloadnode-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.js7
-rw-r--r--lib/net.js6
-rw-r--r--test/simple/test-child-process-sync-process-send.js44
-rw-r--r--test/simple/test-net-sync-stdio.js44
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);
+}));