Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Bug with readline close() not closing properly in Windows #5927

Closed
danielchatfield opened this issue Jul 28, 2013 · 2 comments
Closed

Bug with readline close() not closing properly in Windows #5927

danielchatfield opened this issue Jul 28, 2013 · 2 comments

Comments

@danielchatfield
Copy link

I apologise for the awful test case to illustrate the bug:

var readline = require('readline');

var opt = {
    input: process.stdin,
    output: process.stdout
};


var createInterface = function() {
    var rl = readline.createInterface(opt);
    var origWrite = rl._ttyWrite;
    rl._ttyWrite = function( s, key ) {
        key = key || {};
        if ( key.name === "w" ) return;

        origWrite.apply( this, arguments );
    }
    return rl;
};

var rl = createInterface();


var onKeypress = function(s, key) {
    if ( key && (key.name === "enter" || key.name === "return") ) {
        //rl.pause();
        rl.close();
        rl = null;
        rl = createInterface();
    }
}

process.stdin.on( "keypress", onKeypress );

Run this then enter 'qwerty'. (the w will not appear since it is being intercepted by the modified _ttyWrite).

Now hit enter (to close the readline, remove it and create a new one).

Now try typing 'qwerty', it should be the same but it isn't, this time 'qwerty' is printed.

If you uncomment the rl.pause() it works properly - but since rl.pause() is called from within rl.close() I don't have a clue why it doesn't work without it.

@SBoudrias
Copy link

Probably something to do with _setRawMode

https://github.com/joyent/node/blob/master/lib/readline.js#L258-L274

@danielchatfield
Copy link
Author

@SBoudrias I think putting the pause() above the _setRawMode would fix this. I'm going to test this now (although it might take a while to get my head around building node).

piscisaureus pushed a commit that referenced this issue Aug 17, 2013
On windows, libuv will immediately make a `ReadConsole` call (in the
thread pool) when a 'flowing' `uv_tty_t` handle is switched to
line-buffered mode. That causes an immediate issue for some users,
since libuv can't cancel the `ReadConsole` operation on Windows 8 /
Server 2012 and up if the program switches back to raw mode later.

But even if this will be fixed in libuv at some point, it's better to
avoid the overhead of starting work in the thread pool and immediately
cancelling it afther that.

See also f34f1e3, where the same change is made for the opposite
flow, e.g. move `resume()` after `_setRawMode(true)`.

Fixes #5927

This is a backport of dfb0461 (see #5930) to the v0.10 branch.
piscisaureus pushed a commit that referenced this issue Aug 17, 2013
On windows, libuv will immediately make a `ReadConsole` call (in the
thread pool) when a 'flowing' `uv_tty_t` handle is switched to
line-buffered mode. That causes an immediate issue for some users,
since libuv can't cancel the `ReadConsole` operation on Windows 8 /
Server 2012 and up if the program switches back to raw mode later.

But even if this will be fixed in libuv at some point, it's better to
avoid the overhead of starting work in the thread pool and immediately
cancelling it afther that.

See also f34f1e3, where the same change is made for the opposite
flow, e.g. move `resume()` after `_setRawMode(true)`.

Fixes #5927

This is a backport of dfb0461 (see #5930) to the v0.8 branch.
gibfahn pushed a commit to ibmruntimes/node that referenced this issue Mar 30, 2016
Check that piping a large chunk of data from `process.stdin`
into `process.stdout` does not lose any data by verifying that
the output has the same size as the input.

This is a regression test for nodejs#5927 and fails for the commits
in the range [ace1009..89abe86).

PR-URL: nodejs/node#5949
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
smanders pushed a commit to externpro-archive/node-v0.x-archive that referenced this issue Apr 8, 2016
Check that piping a large chunk of data from `process.stdin`
into `process.stdout` does not lose any data by verifying that
the output has the same size as the input.

This is a regression test for nodejs#5927 and fails for the commits
in the range [ace1009..89abe86).

PR-URL: nodejs/node#5949
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
richardlau pushed a commit to ibmruntimes/node that referenced this issue Apr 12, 2016
Check that piping a large chunk of data from `process.stdin`
into `process.stdout` does not lose any data by verifying that
the output has the same size as the input.

This is a regression test for nodejs#5927 and fails for the commits
in the range [ace1009..89abe86).

PR-URL: nodejs/node#5949
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants