Skip to content

Commit

Permalink
tty: fix 'resize' event regression
Browse files Browse the repository at this point in the history
It's not wholly clear what commit introduced the regression but between
v8.4.0 and v8.5.0 the 'resize' event stopped getting emitted when the
tty was resized.

The SIGWINCH event listener apparently was being installed before the
support code for `process.on('SIGWINCH', ...)` was.  Fix that by moving
said support code to real early in the bootstrap process.

This commit also seems to fix a Windows-only "write EINVAL" error for
reasons even less well-understood...

Fixes: #16141
Fixes: #16194
PR-URL: #16225
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
  • Loading branch information
bnoordhuis authored and MylesBorins committed Dec 11, 2017
1 parent 7ac760b commit f321921
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/internal/bootstrap_node.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

const _process = NativeModule.require('internal/process');
_process.setupConfig(NativeModule._source);
_process.setupSignalHandlers();
NativeModule.require('internal/process/warning').setup();
NativeModule.require('internal/process/next_tick').setup();
NativeModule.require('internal/process/stdio').setup();
Expand All @@ -55,7 +56,6 @@
_process.setup_cpuUsage();
_process.setupMemoryUsage();
_process.setupKillAndExit();
_process.setupSignalHandlers();
if (global.__coverage__)
NativeModule.require('internal/process/write-coverage').setup();

Expand Down
5 changes: 5 additions & 0 deletions test/pseudo-tty/pseudo-tty.status
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,8 @@ prefix pseudo-tty
[$system==aix]
# being investigated under https://github.com/nodejs/node/issues/9728
test-tty-wrap : FAIL, PASS

[$system==solaris]
# https://github.com/nodejs/node/pull/16225 - `ioctl(fd, TIOCGWINSZ)` seems
# to fail with EINVAL on SmartOS when `fd` is a pty from python's pty module.
test-tty-stdout-resize : FAIL, PASS
11 changes: 11 additions & 0 deletions test/pseudo-tty/test-tty-stdout-resize.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
'use strict';
const { mustCall } = require('../common');
const { notStrictEqual } = require('assert');

// tty.WriteStream#_refreshSize() only emits the 'resize' event when the
// window dimensions change. We cannot influence that from the script
// but we can set the old values to something exceedingly unlikely.
process.stdout.columns = 9001;
process.stdout.on('resize', mustCall());
process.kill(process.pid, 'SIGWINCH');
setImmediate(mustCall(() => notStrictEqual(process.stdout.columns, 9001)));
Empty file.

0 comments on commit f321921

Please sign in to comment.