Skip to content

Commit cf20b6b

Browse files
committed
Fix nodejs#2257 pause/resume semantics for stdin
This makes it so that the stdin TTY-wrap stream gets ref'ed on .resume() and unref'ed on .pause() The semantics of the names "pause" and "resume" are a bit weird, but the important thing is that this corrects an API change from 0.4 -> 0.6 which made it impossible to read from stdin multiple times, without knowing when it might end up being closed. If no one has it open, this lets the process die naturally. LGTM'd by @ry
1 parent 6f86b9c commit cf20b6b

File tree

7 files changed

+78
-7
lines changed

7 files changed

+78
-7
lines changed

lib/net.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,9 @@ Object.defineProperty(Socket.prototype, 'bufferSize', {
194194

195195

196196
Socket.prototype.pause = function() {
197-
this._handle.readStop();
197+
if (this._handle) {
198+
this._handle.readStop();
199+
}
198200
};
199201

200202

lib/tty.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,19 @@ function ReadStream(fd) {
8383
this.on('newListener', onNewListener);
8484
}
8585
inherits(ReadStream, net.Socket);
86+
8687
exports.ReadStream = ReadStream;
8788

89+
ReadStream.prototype.pause = function() {
90+
this._handle.unref();
91+
return net.Socket.prototype.pause.call(this);
92+
};
93+
94+
ReadStream.prototype.resume = function() {
95+
this._handle.ref();
96+
return net.Socket.prototype.resume.call(this);
97+
};
98+
8899

89100
ReadStream.prototype.isTTY = true;
90101

src/handle_wrap.cc

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,10 @@ Handle<Value> HandleWrap::Unref(const Arguments& args) {
6464

6565
UNWRAP
6666

67-
// Calling this function twice should never happen.
68-
assert(wrap->unref == false);
67+
// Calling unnecessarily is a no-op
68+
if (wrap->unref) {
69+
return v8::Undefined();
70+
}
6971

7072
wrap->unref = true;
7173
uv_unref(uv_default_loop());
@@ -74,6 +76,24 @@ Handle<Value> HandleWrap::Unref(const Arguments& args) {
7476
}
7577

7678

79+
// Adds a reference to keep uv alive because of this thing.
80+
Handle<Value> HandleWrap::Ref(const Arguments& args) {
81+
HandleScope scope;
82+
83+
UNWRAP
84+
85+
// Calling multiple times is a no-op
86+
if (!wrap->unref) {
87+
return v8::Undefined();
88+
}
89+
90+
wrap->unref = false;
91+
uv_ref(uv_default_loop());
92+
93+
return v8::Undefined();
94+
}
95+
96+
7797
Handle<Value> HandleWrap::Close(const Arguments& args) {
7898
HandleScope scope;
7999

@@ -82,10 +102,8 @@ Handle<Value> HandleWrap::Close(const Arguments& args) {
82102
assert(!wrap->object_.IsEmpty());
83103
uv_close(wrap->handle__, OnClose);
84104

85-
if (wrap->unref) {
86-
uv_ref(uv_default_loop());
87-
wrap->unref = false;
88-
}
105+
106+
HandleWrap::Ref(args);
89107

90108
wrap->StateChange();
91109

src/handle_wrap.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ class HandleWrap {
4949
static void Initialize(v8::Handle<v8::Object> target);
5050
static v8::Handle<v8::Value> Close(const v8::Arguments& args);
5151
static v8::Handle<v8::Value> Unref(const v8::Arguments& args);
52+
static v8::Handle<v8::Value> Ref(const v8::Arguments& args);
5253

5354
protected:
5455
HandleWrap(v8::Handle<v8::Object> object, uv_handle_t* handle);

src/pipe_wrap.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ void PipeWrap::Initialize(Handle<Object> target) {
8585

8686
NODE_SET_PROTOTYPE_METHOD(t, "close", HandleWrap::Close);
8787
NODE_SET_PROTOTYPE_METHOD(t, "unref", HandleWrap::Unref);
88+
NODE_SET_PROTOTYPE_METHOD(t, "ref", HandleWrap::Ref);
8889

8990
NODE_SET_PROTOTYPE_METHOD(t, "readStart", StreamWrap::ReadStart);
9091
NODE_SET_PROTOTYPE_METHOD(t, "readStop", StreamWrap::ReadStop);

src/tty_wrap.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ class TTYWrap : StreamWrap {
6969

7070
NODE_SET_PROTOTYPE_METHOD(t, "close", HandleWrap::Close);
7171
NODE_SET_PROTOTYPE_METHOD(t, "unref", HandleWrap::Unref);
72+
NODE_SET_PROTOTYPE_METHOD(t, "ref", HandleWrap::Ref);
7273

7374
NODE_SET_PROTOTYPE_METHOD(t, "readStart", StreamWrap::ReadStart);
7475
NODE_SET_PROTOTYPE_METHOD(t, "readStop", StreamWrap::ReadStop);
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
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+
console.error("before opening stdin");
23+
process.stdin.resume();
24+
console.error("stdin opened");
25+
setTimeout(function() {
26+
console.error("pausing stdin");
27+
process.stdin.pause();
28+
setTimeout(function() {
29+
console.error("opening again");
30+
process.stdin.resume();
31+
setTimeout(function() {
32+
console.error("pausing again");
33+
process.stdin.pause();
34+
console.error("should exit now");
35+
}, 1);
36+
}, 1);
37+
}, 1);

0 commit comments

Comments
 (0)