From 3c54f8199c80fbc5b5d710938e1eaf3c8b84019e Mon Sep 17 00:00:00 2001 From: Ben Schmidt Date: Tue, 19 Jul 2016 13:50:27 +1000 Subject: [PATCH] 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: https://github.com/nodejs/node/pull/7360 Reviewed-By: Anna Henningsen Reviewed-By: Sam Roberts Reviewed-By: James M Snell Reviewed-By: Jeremiah Senkpiel --- src/tty_wrap.cc | 1 + test/README.md | 5 + test/parallel/test-handle-wrap-isrefed-tty.js | 33 ----- test/parallel/test-handle-wrap-isrefed.js | 129 +++++++++++------- test/pseudo-tty/ref_keeps_node_running.js | 27 ++++ test/pseudo-tty/ref_keeps_node_running.out | 0 .../test-handle-wrap-isrefed-tty.js | 23 ++++ .../test-handle-wrap-isrefed-tty.out | 0 8 files changed, 135 insertions(+), 83 deletions(-) delete mode 100644 test/parallel/test-handle-wrap-isrefed-tty.js create mode 100644 test/pseudo-tty/ref_keeps_node_running.js create mode 100644 test/pseudo-tty/ref_keeps_node_running.out create mode 100644 test/pseudo-tty/test-handle-wrap-isrefed-tty.js create mode 100644 test/pseudo-tty/test-handle-wrap-isrefed-tty.out diff --git a/src/tty_wrap.cc b/src/tty_wrap.cc index 26f061b99b34e8..5b7518324773ae 100644 --- a/src/tty_wrap.cc +++ b/src/tty_wrap.cc @@ -34,6 +34,7 @@ void TTYWrap::Initialize(Local target, env->SetProtoMethod(t, "close", HandleWrap::Close); env->SetProtoMethod(t, "unref", HandleWrap::Unref); + env->SetProtoMethod(t, "ref", HandleWrap::Ref); env->SetProtoMethod(t, "hasRef", HandleWrap::HasRef); StreamWrap::AddMethods(env, t, StreamBase::kFlagNoShutdown); diff --git a/test/README.md b/test/README.md index 949a275792abb0..653f78cc6d2eb5 100644 --- a/test/README.md +++ b/test/README.md @@ -105,6 +105,11 @@ On how to run tests in this direcotry, see Yes Various tests that are able to be run in parallel. + + pseudo-tty + Yes + Tests that require stdin/stdout/stderr to be a TTY. + pummel No diff --git a/test/parallel/test-handle-wrap-isrefed-tty.js b/test/parallel/test-handle-wrap-isrefed-tty.js deleted file mode 100644 index ad312be1f77862..00000000000000 --- a/test/parallel/test-handle-wrap-isrefed-tty.js +++ /dev/null @@ -1,33 +0,0 @@ -'use strict'; - -const common = require('../common'); -const strictEqual = require('assert').strictEqual; -const spawn = require('child_process').spawn; - -function makeAssert(message) { - return function(actual, expected) { - strictEqual(actual, expected, message); - }; -} -const assert = makeAssert('hasRef() not working on tty_wrap'); - -if (process.argv[2] === 'child') { - // Test tty_wrap in piped child to guarentee stdin being a TTY. - const ReadStream = require('tty').ReadStream; - const tty = new ReadStream(0); - assert(Object.getPrototypeOf(tty._handle).hasOwnProperty('hasRef'), true); - assert(tty._handle.hasRef(), true); - tty.unref(); - assert(tty._handle.hasRef(), false); - tty._handle.close( - common.mustCall(() => assert(tty._handle.hasRef(), false))); - return; -} - -// Use spawn so that we can be sure that stdin has a _handle property. -// Refs: https://github.com/nodejs/node/pull/5916 -const proc = spawn(process.execPath, [__filename, 'child'], { stdio: 'pipe' }); -proc.stderr.pipe(process.stderr); -proc.on('exit', common.mustCall(function(exitCode) { - process.exitCode = exitCode; -})); diff --git a/test/parallel/test-handle-wrap-isrefed.js b/test/parallel/test-handle-wrap-isrefed.js index b5dbeb23bfd63a..66353fcc0362b3 100644 --- a/test/parallel/test-handle-wrap-isrefed.js +++ b/test/parallel/test-handle-wrap-isrefed.js @@ -3,99 +3,128 @@ const common = require('../common'); const strictEqual = require('assert').strictEqual; -function makeAssert(message) { - return function(actual, expected) { - strictEqual(actual, expected, message); - }; -} - - // child_process { - const assert = makeAssert('hasRef() not working on process_wrap'); const spawn = require('child_process').spawn; const cmd = common.isWindows ? 'rundll32' : 'ls'; const cp = spawn(cmd); - assert(Object.getPrototypeOf(cp._handle).hasOwnProperty('hasRef'), true); - assert(cp._handle.hasRef(), true); + strictEqual(Object.getPrototypeOf(cp._handle).hasOwnProperty('hasRef'), + true, 'process_wrap: hasRef() missing'); + strictEqual(cp._handle.hasRef(), + true, 'process_wrap: not initially refed'); cp.unref(); - assert(cp._handle.hasRef(), false); + strictEqual(cp._handle.hasRef(), + false, 'process_wrap: unref() ineffective'); cp.ref(); - assert(cp._handle.hasRef(), true); - cp._handle.close(common.mustCall(() => assert(cp._handle.hasRef(), false))); + strictEqual(cp._handle.hasRef(), + true, 'process_wrap: ref() ineffective'); + cp._handle.close(common.mustCall(() => + strictEqual(cp._handle.hasRef(), + false, 'process_wrap: not unrefed on close'))); } -// dgram +// dgram ipv4 { - const assert = makeAssert('hasRef() not working on udp_wrap'); const dgram = require('dgram'); - const sock4 = dgram.createSocket('udp4'); - assert(Object.getPrototypeOf(sock4._handle).hasOwnProperty('hasRef'), true); - assert(sock4._handle.hasRef(), true); + strictEqual(Object.getPrototypeOf(sock4._handle).hasOwnProperty('hasRef'), + true, 'udp_wrap: ipv4: hasRef() missing'); + strictEqual(sock4._handle.hasRef(), + true, 'udp_wrap: ipv4: not initially refed'); sock4.unref(); - assert(sock4._handle.hasRef(), false); + strictEqual(sock4._handle.hasRef(), + false, 'udp_wrap: ipv4: unref() ineffective'); sock4.ref(); - assert(sock4._handle.hasRef(), true); - sock4._handle.close( - common.mustCall(() => assert(sock4._handle.hasRef(), false))); + strictEqual(sock4._handle.hasRef(), + true, 'udp_wrap: ipv4: ref() ineffective'); + sock4._handle.close(common.mustCall(() => + strictEqual(sock4._handle.hasRef(), + false, 'udp_wrap: ipv4: not unrefed on close'))); +} + +// dgram ipv6 +{ + const dgram = require('dgram'); const sock6 = dgram.createSocket('udp6'); - assert(Object.getPrototypeOf(sock6._handle).hasOwnProperty('hasRef'), true); - assert(sock6._handle.hasRef(), true); + strictEqual(Object.getPrototypeOf(sock6._handle).hasOwnProperty('hasRef'), + true, 'udp_wrap: ipv6: hasRef() missing'); + strictEqual(sock6._handle.hasRef(), + true, 'udp_wrap: ipv6: not initially refed'); sock6.unref(); - assert(sock6._handle.hasRef(), false); + strictEqual(sock6._handle.hasRef(), + false, 'udp_wrap: ipv6: unref() ineffective'); sock6.ref(); - assert(sock6._handle.hasRef(), true); - sock6._handle.close( - common.mustCall(() => assert(sock6._handle.hasRef(), false))); + strictEqual(sock6._handle.hasRef(), + true, 'udp_wrap: ipv6: ref() ineffective'); + sock6._handle.close(common.mustCall(() => + strictEqual(sock6._handle.hasRef(), + false, 'udp_wrap: ipv6: not unrefed on close'))); } // pipe { - const assert = makeAssert('hasRef() not working on pipe_wrap'); const Pipe = process.binding('pipe_wrap').Pipe; const handle = new Pipe(); - assert(Object.getPrototypeOf(handle).hasOwnProperty('hasRef'), true); - assert(handle.hasRef(), true); + strictEqual(Object.getPrototypeOf(handle).hasOwnProperty('hasRef'), + true, 'pipe_wrap: hasRef() missing'); + strictEqual(handle.hasRef(), + true, 'pipe_wrap: not initially refed'); handle.unref(); - assert(handle.hasRef(), false); + strictEqual(handle.hasRef(), + false, 'pipe_wrap: unref() ineffective'); handle.ref(); - assert(handle.hasRef(), true); - handle.close(common.mustCall(() => assert(handle.hasRef(), false))); + strictEqual(handle.hasRef(), + true, 'pipe_wrap: ref() ineffective'); + handle.close(common.mustCall(() => + strictEqual(handle.hasRef(), + false, 'pipe_wrap: not unrefed on close'))); } // tcp { - const assert = makeAssert('hasRef() not working on tcp_wrap'); const net = require('net'); const server = net.createServer(() => {}).listen(0); - assert(Object.getPrototypeOf(server._handle).hasOwnProperty('hasRef'), true); - assert(server._handle.hasRef(), true); - assert(server._unref, false); + strictEqual(Object.getPrototypeOf(server._handle).hasOwnProperty('hasRef'), + true, 'tcp_wrap: hasRef() missing'); + strictEqual(server._handle.hasRef(), + true, 'tcp_wrap: not initially refed'); + strictEqual(server._unref, + false, 'tcp_wrap: _unref initially incorrect'); server.unref(); - assert(server._handle.hasRef(), false); - assert(server._unref, true); + strictEqual(server._handle.hasRef(), + false, 'tcp_wrap: unref() ineffective'); + strictEqual(server._unref, + true, 'tcp_wrap: _unref not updated on unref()'); server.ref(); - assert(server._handle.hasRef(), true); - assert(server._unref, false); - server._handle.close( - common.mustCall(() => assert(server._handle.hasRef(), false))); + strictEqual(server._handle.hasRef(), + true, 'tcp_wrap: ref() ineffective'); + strictEqual(server._unref, + false, 'tcp_wrap: _unref not updated on ref()'); + server._handle.close(common.mustCall(() => + strictEqual(server._handle.hasRef(), + false, 'tcp_wrap: not unrefed on close'))); } // timers { - const assert = makeAssert('hasRef() not working on timer_wrap'); const timer = setTimeout(() => {}, 500); timer.unref(); - assert(Object.getPrototypeOf(timer._handle).hasOwnProperty('hasRef'), true); - assert(timer._handle.hasRef(), false); + strictEqual(Object.getPrototypeOf(timer._handle).hasOwnProperty('hasRef'), + true, 'timer_wrap: hasRef() missing'); + strictEqual(timer._handle.hasRef(), + false, 'timer_wrap: unref() ineffective'); timer.ref(); - assert(timer._handle.hasRef(), true); - timer._handle.close( - common.mustCall(() => assert(timer._handle.hasRef(), false))); + strictEqual(timer._handle.hasRef(), + true, 'timer_wrap: ref() ineffective'); + timer._handle.close(common.mustCall(() => + strictEqual(timer._handle.hasRef(), + false, 'timer_wrap: not unrefed on close'))); } + + +// see also test/pseudo-tty/test-handle-wrap-isrefed-tty.js diff --git a/test/pseudo-tty/ref_keeps_node_running.js b/test/pseudo-tty/ref_keeps_node_running.js new file mode 100644 index 00000000000000..de3c6531ef6924 --- /dev/null +++ b/test/pseudo-tty/ref_keeps_node_running.js @@ -0,0 +1,27 @@ +'use strict'; +require('../common'); + +const { TTY, isTTY } = process.binding('tty_wrap'); +const strictEqual = require('assert').strictEqual; + +strictEqual(isTTY(0), true, 'fd 0 is not a TTY'); + +const handle = new TTY(0); +handle.readStart(); +handle.onread = () => {}; + +function isHandleActive(handle) { + return process._getActiveHandles().some((active) => active === handle); +} + +strictEqual(isHandleActive(handle), true, 'TTY handle not initially active'); + +handle.unref(); + +strictEqual(isHandleActive(handle), false, 'TTY handle active after unref()'); + +handle.ref(); + +strictEqual(isHandleActive(handle), true, 'TTY handle inactive after ref()'); + +handle.unref(); diff --git a/test/pseudo-tty/ref_keeps_node_running.out b/test/pseudo-tty/ref_keeps_node_running.out new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/test/pseudo-tty/test-handle-wrap-isrefed-tty.js b/test/pseudo-tty/test-handle-wrap-isrefed-tty.js new file mode 100644 index 00000000000000..4baf6200962cc3 --- /dev/null +++ b/test/pseudo-tty/test-handle-wrap-isrefed-tty.js @@ -0,0 +1,23 @@ +'use strict'; + +// see also test/parallel/test-handle-wrap-isrefed.js + +const common = require('../common'); +const strictEqual = require('assert').strictEqual; +const ReadStream = require('tty').ReadStream; +const tty = new ReadStream(0); +const isTTY = process.binding('tty_wrap').isTTY; +strictEqual(isTTY(0), true, 'tty_wrap: stdin is not a TTY'); +strictEqual(Object.getPrototypeOf(tty._handle).hasOwnProperty('hasRef'), + true, 'tty_wrap: hasRef() missing'); +strictEqual(tty._handle.hasRef(), + true, 'tty_wrap: not initially refed'); +tty.unref(); +strictEqual(tty._handle.hasRef(), + false, 'tty_wrap: unref() ineffective'); +tty.ref(); +strictEqual(tty._handle.hasRef(), + true, 'tty_wrap: ref() ineffective'); +tty._handle.close(common.mustCall(() => + strictEqual(tty._handle.hasRef(), + false, 'tty_wrap: not unrefed on close'))); diff --git a/test/pseudo-tty/test-handle-wrap-isrefed-tty.out b/test/pseudo-tty/test-handle-wrap-isrefed-tty.out new file mode 100644 index 00000000000000..e69de29bb2d1d6