Skip to content

Commit 20176a9

Browse files
orangemochatjfontaine
authored andcommitted
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
1 parent c1bb886 commit 20176a9

6 files changed

+98
-3
lines changed

lib/net.js

+10-1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ var util = require('util');
2626
var assert = require('assert');
2727
var cares = process.binding('cares_wrap');
2828
var uv = process.binding('uv');
29+
var Pipe = process.binding('pipe_wrap').Pipe;
30+
2931

3032
var cluster;
3133
var errnoException = util._errnoException;
@@ -34,7 +36,6 @@ function noop() {}
3436

3537
// constructor for lazy loading
3638
function createPipe() {
37-
var Pipe = process.binding('pipe_wrap').Pipe;
3839
return new Pipe();
3940
}
4041

@@ -147,6 +148,14 @@ function Socket(options) {
147148
} else if (!util.isUndefined(options.fd)) {
148149
this._handle = createHandle(options.fd);
149150
this._handle.open(options.fd);
151+
if ((options.fd == 1 || options.fd == 2) &&
152+
(this._handle instanceof Pipe) &&
153+
process.platform === 'win32') {
154+
// Make stdout and stderr blocking on Windows
155+
var err = this._handle.setBlocking(true);
156+
if (err)
157+
throw errnoException(err, 'setBlocking');
158+
}
150159
this.readable = options.readable !== false;
151160
this.writable = options.writable !== false;
152161
} else {

src/pipe_wrap.cc

+2
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,8 @@ void PipeWrap::Initialize(Handle<Object> target,
9191
NODE_SET_PROTOTYPE_METHOD(t, "unref", HandleWrap::Unref);
9292
NODE_SET_PROTOTYPE_METHOD(t, "ref", HandleWrap::Ref);
9393

94+
NODE_SET_PROTOTYPE_METHOD(t, "setBlocking", StreamWrap::SetBlocking);
95+
9496
NODE_SET_PROTOTYPE_METHOD(t, "readStart", StreamWrap::ReadStart);
9597
NODE_SET_PROTOTYPE_METHOD(t, "readStop", StreamWrap::ReadStop);
9698
NODE_SET_PROTOTYPE_METHOD(t, "shutdown", StreamWrap::Shutdown);

src/stream_wrap.cc

+11-1
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,6 @@ void StreamWrap::UpdateWriteQueueSize() {
8787
object()->Set(env()->write_queue_size_string(), write_queue_size);
8888
}
8989

90-
9190
void StreamWrap::ReadStart(const FunctionCallbackInfo<Value>& args) {
9291
Environment* env = Environment::GetCurrent(args.GetIsolate());
9392
HandleScope scope(env->isolate());
@@ -496,6 +495,17 @@ void StreamWrap::WriteUcs2String(const FunctionCallbackInfo<Value>& args) {
496495
WriteStringImpl<UCS2>(args);
497496
}
498497

498+
void StreamWrap::SetBlocking(const FunctionCallbackInfo<Value>& args) {
499+
Environment* env = Environment::GetCurrent(args.GetIsolate());
500+
HandleScope scope(env->isolate());
501+
502+
StreamWrap* wrap = Unwrap<StreamWrap>(args.This());
503+
504+
assert(args.Length() > 0);
505+
int err = uv_stream_set_blocking(wrap->stream(), args[0]->IsTrue());
506+
507+
args.GetReturnValue().Set(err);
508+
}
499509

500510
void StreamWrap::AfterWrite(uv_write_t* req, int status) {
501511
WriteWrap* req_wrap = CONTAINER_OF(req, WriteWrap, req_);

src/stream_wrap.h

+2
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,8 @@ class StreamWrap : public HandleWrap {
126126
static void WriteUtf8String(const v8::FunctionCallbackInfo<v8::Value>& args);
127127
static void WriteUcs2String(const v8::FunctionCallbackInfo<v8::Value>& args);
128128

129+
static void SetBlocking(const v8::FunctionCallbackInfo<v8::Value>& args);
130+
129131
inline StreamWrapCallbacks* callbacks() const {
130132
return callbacks_;
131133
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// Copyright Joyent, Inc. and other Node contributors.
2+
//
3+
// Permission is hereby granted, free of charge, to any person obtaining a
4+
// copy of this software and associated documentation files (the
5+
// "Software"), to deal in the Software without restriction, including
6+
// without limitation the rights to use, copy, modify, merge, publish,
7+
// distribute, sublicense, and/or sell copies of the Software, and to permit
8+
// persons to whom the Software is furnished to do so, subject to the
9+
// following conditions:
10+
//
11+
// The above copyright notice and this permission notice shall be included
12+
// in all copies or substantial portions of the Software.
13+
//
14+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
15+
// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
16+
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
17+
// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
18+
// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
19+
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
20+
// USE OR OTHER DEALINGS IN THE SOFTWARE.
21+
22+
23+
24+
25+
var common = require('../common');
26+
var assert = require('assert');
27+
var path = require('path');
28+
29+
// if child process output to console and exit
30+
if (process.argv[2] === 'child') {
31+
console.log('hello');
32+
for (var i = 0; i < 200; i++) {
33+
console.log('filler');
34+
}
35+
console.log('goodbye');
36+
process.exit(0);
37+
} else {
38+
// parent process
39+
var spawn = require('child_process').spawn;
40+
41+
// spawn self as child
42+
var child = spawn(process.argv[0], [process.argv[1], 'child']);
43+
44+
var gotHello = false;
45+
var gotBye = false;
46+
47+
child.stderr.setEncoding('utf8');
48+
child.stderr.on('data', function (data) {
49+
console.log('parent stderr: ' + data);
50+
assert.ok(false);
51+
});
52+
53+
// check if we receive both 'hello' at start and 'goodbye' at end
54+
child.stdout.setEncoding('utf8');
55+
child.stdout.on('data', function (data) {
56+
if (data.slice(0, 6) == 'hello\n') {
57+
gotHello = true;
58+
} else if (data.slice(data.length - 8) == 'goodbye\n') {
59+
gotBye = true;
60+
} else {
61+
gotBye = false;
62+
}
63+
});
64+
65+
child.on('close', function (data) {
66+
assert(gotHello);
67+
assert(gotBye);
68+
});
69+
}

test/simple/test-stream2-stderr-sync.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,10 @@ function child1() {
6767
// using a net socket
6868
function child2() {
6969
var net = require('net');
70-
var socket = new net.Socket({ fd: 2 });
70+
var socket = new net.Socket({
71+
fd: 2,
72+
readable: false,
73+
writable: true});
7174
socket.write('child 2\n');
7275
socket.write('foo\n');
7376
socket.write('bar\n');

0 commit comments

Comments
 (0)