Skip to content

Commit 3c54f81

Browse files
insightfulsaddaleax
insightfuls
authored and
addaleax
committed
tty: add ref() so process.stdin.ref() etc. work
Also squashed from: * test: move tty-wrap isrefed test to pseudo-tty/ * test: test tty-wrap handle isrefed properly * test: improve failure messages in isrefed tests PR-URL: #7360 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: James M Snell <jasnell.gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
1 parent b20bc13 commit 3c54f81

File tree

8 files changed

+135
-83
lines changed

8 files changed

+135
-83
lines changed

‎src/tty_wrap.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ void TTYWrap::Initialize(Local<Object> target,
3434

3535
env->SetProtoMethod(t, "close", HandleWrap::Close);
3636
env->SetProtoMethod(t, "unref", HandleWrap::Unref);
37+
env->SetProtoMethod(t, "ref", HandleWrap::Ref);
3738
env->SetProtoMethod(t, "hasRef", HandleWrap::HasRef);
3839

3940
StreamWrap::AddMethods(env, t, StreamBase::kFlagNoShutdown);

‎test/README.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,11 @@ On how to run tests in this direcotry, see
105105
<td>Yes</td>
106106
<td>Various tests that are able to be run in parallel.</td>
107107
</tr>
108+
<tr>
109+
<td>pseudo-tty</td>
110+
<td>Yes</td>
111+
<td>Tests that require stdin/stdout/stderr to be a TTY.</td>
112+
</tr>
108113
<tr>
109114
<td>pummel</td>
110115
<td>No</td>

‎test/parallel/test-handle-wrap-isrefed-tty.js‎

Lines changed: 0 additions & 33 deletions
This file was deleted.

‎test/parallel/test-handle-wrap-isrefed.js‎

Lines changed: 79 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -3,99 +3,128 @@
33
const common = require('../common');
44
const strictEqual = require('assert').strictEqual;
55

6-
function makeAssert(message) {
7-
return function(actual, expected) {
8-
strictEqual(actual, expected, message);
9-
};
10-
}
11-
12-
136
// child_process
147
{
15-
const assert = makeAssert('hasRef() not working on process_wrap');
168
const spawn = require('child_process').spawn;
179
const cmd = common.isWindows ? 'rundll32' : 'ls';
1810
const cp = spawn(cmd);
19-
assert(Object.getPrototypeOf(cp._handle).hasOwnProperty('hasRef'), true);
20-
assert(cp._handle.hasRef(), true);
11+
strictEqual(Object.getPrototypeOf(cp._handle).hasOwnProperty('hasRef'),
12+
true, 'process_wrap: hasRef() missing');
13+
strictEqual(cp._handle.hasRef(),
14+
true, 'process_wrap: not initially refed');
2115
cp.unref();
22-
assert(cp._handle.hasRef(), false);
16+
strictEqual(cp._handle.hasRef(),
17+
false, 'process_wrap: unref() ineffective');
2318
cp.ref();
24-
assert(cp._handle.hasRef(), true);
25-
cp._handle.close(common.mustCall(() => assert(cp._handle.hasRef(), false)));
19+
strictEqual(cp._handle.hasRef(),
20+
true, 'process_wrap: ref() ineffective');
21+
cp._handle.close(common.mustCall(() =>
22+
strictEqual(cp._handle.hasRef(),
23+
false, 'process_wrap: not unrefed on close')));
2624
}
2725

2826

29-
// dgram
27+
// dgram ipv4
3028
{
31-
const assert = makeAssert('hasRef() not working on udp_wrap');
3229
const dgram = require('dgram');
33-
3430
const sock4 = dgram.createSocket('udp4');
35-
assert(Object.getPrototypeOf(sock4._handle).hasOwnProperty('hasRef'), true);
36-
assert(sock4._handle.hasRef(), true);
31+
strictEqual(Object.getPrototypeOf(sock4._handle).hasOwnProperty('hasRef'),
32+
true, 'udp_wrap: ipv4: hasRef() missing');
33+
strictEqual(sock4._handle.hasRef(),
34+
true, 'udp_wrap: ipv4: not initially refed');
3735
sock4.unref();
38-
assert(sock4._handle.hasRef(), false);
36+
strictEqual(sock4._handle.hasRef(),
37+
false, 'udp_wrap: ipv4: unref() ineffective');
3938
sock4.ref();
40-
assert(sock4._handle.hasRef(), true);
41-
sock4._handle.close(
42-
common.mustCall(() => assert(sock4._handle.hasRef(), false)));
39+
strictEqual(sock4._handle.hasRef(),
40+
true, 'udp_wrap: ipv4: ref() ineffective');
41+
sock4._handle.close(common.mustCall(() =>
42+
strictEqual(sock4._handle.hasRef(),
43+
false, 'udp_wrap: ipv4: not unrefed on close')));
44+
}
4345

46+
47+
// dgram ipv6
48+
{
49+
const dgram = require('dgram');
4450
const sock6 = dgram.createSocket('udp6');
45-
assert(Object.getPrototypeOf(sock6._handle).hasOwnProperty('hasRef'), true);
46-
assert(sock6._handle.hasRef(), true);
51+
strictEqual(Object.getPrototypeOf(sock6._handle).hasOwnProperty('hasRef'),
52+
true, 'udp_wrap: ipv6: hasRef() missing');
53+
strictEqual(sock6._handle.hasRef(),
54+
true, 'udp_wrap: ipv6: not initially refed');
4755
sock6.unref();
48-
assert(sock6._handle.hasRef(), false);
56+
strictEqual(sock6._handle.hasRef(),
57+
false, 'udp_wrap: ipv6: unref() ineffective');
4958
sock6.ref();
50-
assert(sock6._handle.hasRef(), true);
51-
sock6._handle.close(
52-
common.mustCall(() => assert(sock6._handle.hasRef(), false)));
59+
strictEqual(sock6._handle.hasRef(),
60+
true, 'udp_wrap: ipv6: ref() ineffective');
61+
sock6._handle.close(common.mustCall(() =>
62+
strictEqual(sock6._handle.hasRef(),
63+
false, 'udp_wrap: ipv6: not unrefed on close')));
5364
}
5465

5566

5667
// pipe
5768
{
58-
const assert = makeAssert('hasRef() not working on pipe_wrap');
5969
const Pipe = process.binding('pipe_wrap').Pipe;
6070
const handle = new Pipe();
61-
assert(Object.getPrototypeOf(handle).hasOwnProperty('hasRef'), true);
62-
assert(handle.hasRef(), true);
71+
strictEqual(Object.getPrototypeOf(handle).hasOwnProperty('hasRef'),
72+
true, 'pipe_wrap: hasRef() missing');
73+
strictEqual(handle.hasRef(),
74+
true, 'pipe_wrap: not initially refed');
6375
handle.unref();
64-
assert(handle.hasRef(), false);
76+
strictEqual(handle.hasRef(),
77+
false, 'pipe_wrap: unref() ineffective');
6578
handle.ref();
66-
assert(handle.hasRef(), true);
67-
handle.close(common.mustCall(() => assert(handle.hasRef(), false)));
79+
strictEqual(handle.hasRef(),
80+
true, 'pipe_wrap: ref() ineffective');
81+
handle.close(common.mustCall(() =>
82+
strictEqual(handle.hasRef(),
83+
false, 'pipe_wrap: not unrefed on close')));
6884
}
6985

7086

7187
// tcp
7288
{
73-
const assert = makeAssert('hasRef() not working on tcp_wrap');
7489
const net = require('net');
7590
const server = net.createServer(() => {}).listen(0);
76-
assert(Object.getPrototypeOf(server._handle).hasOwnProperty('hasRef'), true);
77-
assert(server._handle.hasRef(), true);
78-
assert(server._unref, false);
91+
strictEqual(Object.getPrototypeOf(server._handle).hasOwnProperty('hasRef'),
92+
true, 'tcp_wrap: hasRef() missing');
93+
strictEqual(server._handle.hasRef(),
94+
true, 'tcp_wrap: not initially refed');
95+
strictEqual(server._unref,
96+
false, 'tcp_wrap: _unref initially incorrect');
7997
server.unref();
80-
assert(server._handle.hasRef(), false);
81-
assert(server._unref, true);
98+
strictEqual(server._handle.hasRef(),
99+
false, 'tcp_wrap: unref() ineffective');
100+
strictEqual(server._unref,
101+
true, 'tcp_wrap: _unref not updated on unref()');
82102
server.ref();
83-
assert(server._handle.hasRef(), true);
84-
assert(server._unref, false);
85-
server._handle.close(
86-
common.mustCall(() => assert(server._handle.hasRef(), false)));
103+
strictEqual(server._handle.hasRef(),
104+
true, 'tcp_wrap: ref() ineffective');
105+
strictEqual(server._unref,
106+
false, 'tcp_wrap: _unref not updated on ref()');
107+
server._handle.close(common.mustCall(() =>
108+
strictEqual(server._handle.hasRef(),
109+
false, 'tcp_wrap: not unrefed on close')));
87110
}
88111

89112

90113
// timers
91114
{
92-
const assert = makeAssert('hasRef() not working on timer_wrap');
93115
const timer = setTimeout(() => {}, 500);
94116
timer.unref();
95-
assert(Object.getPrototypeOf(timer._handle).hasOwnProperty('hasRef'), true);
96-
assert(timer._handle.hasRef(), false);
117+
strictEqual(Object.getPrototypeOf(timer._handle).hasOwnProperty('hasRef'),
118+
true, 'timer_wrap: hasRef() missing');
119+
strictEqual(timer._handle.hasRef(),
120+
false, 'timer_wrap: unref() ineffective');
97121
timer.ref();
98-
assert(timer._handle.hasRef(), true);
99-
timer._handle.close(
100-
common.mustCall(() => assert(timer._handle.hasRef(), false)));
122+
strictEqual(timer._handle.hasRef(),
123+
true, 'timer_wrap: ref() ineffective');
124+
timer._handle.close(common.mustCall(() =>
125+
strictEqual(timer._handle.hasRef(),
126+
false, 'timer_wrap: not unrefed on close')));
101127
}
128+
129+
130+
// see also test/pseudo-tty/test-handle-wrap-isrefed-tty.js

‎test/pseudo-tty/ref_keeps_node_running.js‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
'use strict';
2+
require('../common');
3+
4+
const { TTY, isTTY } = process.binding('tty_wrap');
5+
const strictEqual = require('assert').strictEqual;
6+
7+
strictEqual(isTTY(0), true, 'fd 0 is not a TTY');
8+
9+
const handle = new TTY(0);
10+
handle.readStart();
11+
handle.onread = () => {};
12+
13+
function isHandleActive(handle) {
14+
return process._getActiveHandles().some((active) => active === handle);
15+
}
16+
17+
strictEqual(isHandleActive(handle), true, 'TTY handle not initially active');
18+
19+
handle.unref();
20+
21+
strictEqual(isHandleActive(handle), false, 'TTY handle active after unref()');
22+
23+
handle.ref();
24+
25+
strictEqual(isHandleActive(handle), true, 'TTY handle inactive after ref()');
26+
27+
handle.unref();

‎test/pseudo-tty/ref_keeps_node_running.out‎

Whitespace-only changes.

‎test/pseudo-tty/test-handle-wrap-isrefed-tty.js‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
'use strict';
2+
3+
// see also test/parallel/test-handle-wrap-isrefed.js
4+
5+
const common = require('../common');
6+
const strictEqual = require('assert').strictEqual;
7+
const ReadStream = require('tty').ReadStream;
8+
const tty = new ReadStream(0);
9+
const isTTY = process.binding('tty_wrap').isTTY;
10+
strictEqual(isTTY(0), true, 'tty_wrap: stdin is not a TTY');
11+
strictEqual(Object.getPrototypeOf(tty._handle).hasOwnProperty('hasRef'),
12+
true, 'tty_wrap: hasRef() missing');
13+
strictEqual(tty._handle.hasRef(),
14+
true, 'tty_wrap: not initially refed');
15+
tty.unref();
16+
strictEqual(tty._handle.hasRef(),
17+
false, 'tty_wrap: unref() ineffective');
18+
tty.ref();
19+
strictEqual(tty._handle.hasRef(),
20+
true, 'tty_wrap: ref() ineffective');
21+
tty._handle.close(common.mustCall(() =>
22+
strictEqual(tty._handle.hasRef(),
23+
false, 'tty_wrap: not unrefed on close')));

‎test/pseudo-tty/test-handle-wrap-isrefed-tty.out‎

Whitespace-only changes.

0 commit comments

Comments
 (0)