From 20176a98416353d4596900793f739d5ebf4f0ee1 Mon Sep 17 00:00:00 2001 From: Alexis Campailla Date: Wed, 26 Feb 2014 15:03:59 +0100 Subject: [PATCH] src: make stdout/sterr pipes blocking Expose `setBlocking` on Pipe's and if a pipe is being created for stdio on windows then make the pipes blocking. This fixes test-stream2-stderr-sync.js on Windows. Fixes #3584 --- lib/net.js | 11 ++- src/pipe_wrap.cc | 2 + src/stream_wrap.cc | 12 +++- src/stream_wrap.h | 2 + .../test-child-process-stdout-flush-exit.js | 69 +++++++++++++++++++ test/simple/test-stream2-stderr-sync.js | 5 +- 6 files changed, 98 insertions(+), 3 deletions(-) create mode 100644 test/simple/test-child-process-stdout-flush-exit.js diff --git a/lib/net.js b/lib/net.js index 1ed2e86aa2..751b04b05e 100644 --- a/lib/net.js +++ b/lib/net.js @@ -26,6 +26,8 @@ var util = require('util'); var assert = require('assert'); var cares = process.binding('cares_wrap'); var uv = process.binding('uv'); +var Pipe = process.binding('pipe_wrap').Pipe; + var cluster; var errnoException = util._errnoException; @@ -34,7 +36,6 @@ function noop() {} // constructor for lazy loading function createPipe() { - var Pipe = process.binding('pipe_wrap').Pipe; return new Pipe(); } @@ -147,6 +148,14 @@ 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') { + // Make stdout and stderr blocking on Windows + var err = this._handle.setBlocking(true); + if (err) + throw errnoException(err, 'setBlocking'); + } this.readable = options.readable !== false; this.writable = options.writable !== false; } else { diff --git a/src/pipe_wrap.cc b/src/pipe_wrap.cc index 1fc9a560fa..3f19601789 100644 --- a/src/pipe_wrap.cc +++ b/src/pipe_wrap.cc @@ -91,6 +91,8 @@ void PipeWrap::Initialize(Handle target, NODE_SET_PROTOTYPE_METHOD(t, "unref", HandleWrap::Unref); NODE_SET_PROTOTYPE_METHOD(t, "ref", HandleWrap::Ref); + NODE_SET_PROTOTYPE_METHOD(t, "setBlocking", StreamWrap::SetBlocking); + NODE_SET_PROTOTYPE_METHOD(t, "readStart", StreamWrap::ReadStart); NODE_SET_PROTOTYPE_METHOD(t, "readStop", StreamWrap::ReadStop); NODE_SET_PROTOTYPE_METHOD(t, "shutdown", StreamWrap::Shutdown); diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc index 0a1bf92d6e..3a2d110efa 100644 --- a/src/stream_wrap.cc +++ b/src/stream_wrap.cc @@ -87,7 +87,6 @@ void StreamWrap::UpdateWriteQueueSize() { object()->Set(env()->write_queue_size_string(), write_queue_size); } - void StreamWrap::ReadStart(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args.GetIsolate()); HandleScope scope(env->isolate()); @@ -496,6 +495,17 @@ void StreamWrap::WriteUcs2String(const FunctionCallbackInfo& args) { WriteStringImpl(args); } +void StreamWrap::SetBlocking(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args.GetIsolate()); + HandleScope scope(env->isolate()); + + StreamWrap* wrap = Unwrap(args.This()); + + assert(args.Length() > 0); + int err = uv_stream_set_blocking(wrap->stream(), args[0]->IsTrue()); + + args.GetReturnValue().Set(err); +} void StreamWrap::AfterWrite(uv_write_t* req, int status) { WriteWrap* req_wrap = CONTAINER_OF(req, WriteWrap, req_); diff --git a/src/stream_wrap.h b/src/stream_wrap.h index 0f657fe920..92159fe117 100644 --- a/src/stream_wrap.h +++ b/src/stream_wrap.h @@ -126,6 +126,8 @@ class StreamWrap : public HandleWrap { static void WriteUtf8String(const v8::FunctionCallbackInfo& args); static void WriteUcs2String(const v8::FunctionCallbackInfo& args); + static void SetBlocking(const v8::FunctionCallbackInfo& args); + inline StreamWrapCallbacks* callbacks() const { return callbacks_; } diff --git a/test/simple/test-child-process-stdout-flush-exit.js b/test/simple/test-child-process-stdout-flush-exit.js new file mode 100644 index 0000000000..49a0ec0e5f --- /dev/null +++ b/test/simple/test-child-process-stdout-flush-exit.js @@ -0,0 +1,69 @@ +// 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 path = require('path'); + +// if child process output to console and exit +if (process.argv[2] === 'child') { + console.log('hello'); + for (var i = 0; i < 200; i++) { + console.log('filler'); + } + console.log('goodbye'); + process.exit(0); +} else { + // parent process + var spawn = require('child_process').spawn; + + // spawn self as child + var child = spawn(process.argv[0], [process.argv[1], 'child']); + + var gotHello = false; + var gotBye = false; + + child.stderr.setEncoding('utf8'); + child.stderr.on('data', function (data) { + console.log('parent stderr: ' + data); + assert.ok(false); + }); + + // check if we receive both 'hello' at start and 'goodbye' at end + child.stdout.setEncoding('utf8'); + child.stdout.on('data', function (data) { + if (data.slice(0, 6) == 'hello\n') { + gotHello = true; + } else if (data.slice(data.length - 8) == 'goodbye\n') { + gotBye = true; + } else { + gotBye = false; + } + }); + + child.on('close', function (data) { + assert(gotHello); + assert(gotBye); + }); +} diff --git a/test/simple/test-stream2-stderr-sync.js b/test/simple/test-stream2-stderr-sync.js index 9e41ac7f03..9e2d3ec223 100644 --- a/test/simple/test-stream2-stderr-sync.js +++ b/test/simple/test-stream2-stderr-sync.js @@ -67,7 +67,10 @@ function child1() { // using a net socket function child2() { var net = require('net'); - var socket = new net.Socket({ fd: 2 }); + var socket = new net.Socket({ + fd: 2, + readable: false, + writable: true}); socket.write('child 2\n'); socket.write('foo\n'); socket.write('bar\n');