RFR: JDK-8218287: jshell tool: input behavior unstable after 12-ea+24 on Windows

[Jan Lahoda]

On 15.2.2019 21:05, Robert Field wrote:
> Thumbs up!
>
> This is important to fix.  Getting it in the JDK for full shake-out
> would be good.
>
> Per our discussion, let's try to find someone who can test multibyte
> encodings on Windows, with this in.

Unfortunately, turns out there are at least two problems:
-currently (JDK 12/13) ReadConsoleInput is used to read input, but 
ReadConsoleInputW used to be used before - and ReadConsoleInput at least 
in some cases does not read non-ASCII characters correctly
-the NonBlockingReaderInputStream that is created to convert Reader to 
InputStream is converting bytes to ints improperly - should be unsigned 
conversion, but is signed. As a result, the "int read()" method 
sometimes returns a negative integer, which is then ignored.

An updated patch that strives to fix these is here:
http://cr.openjdk.java.net/~jlahoda/8218287/webrev.01/

Jan

>
> -Robert
>
>
> On 2/13/19 6:45 AM, Jan Lahoda wrote:
>> Hi,
>>
>> When JShell was upgraded to use JLine 3.9.0, the input from which the
>> Terminal is reading had to be wrapped, so that JShell can detect
>> Ctrl-C while the user's code is running. The mechanism for this is
>> that the StopDetectingInputStream is actively reading the input to
>> find out if Ctrl-C was pressed. (And other characters are stored aside
>> and returned from the StopDetectingInputStream when JLine is reading
>> from it.)
>>
>> Unfortunately, it turns out this wrapping is not done as expected on
>> Windows (when using the Windows terminal). The issue is that the
>> source of input in the AbstractWindowsTerminal is a Reader, which is
>> directly used to read the input by JLine, and is also converted to an
>> InputStream, which is typically not used. And this InputStream is
>> wrapped by the StopDetectingInputStream. So when the
>> StopDetectingInputStream is trying to detect Ctrl-C, it may "steal" an
>> input character, which is then missing in the Reader used by JLine,
>> and hence is lost (the StopDetectingInputStream has it stored aside,
>> but it is never read from it).
>>
>> The proposed patch is to not use the Reader that is the source of
>> input directly, but rather create a new one based on the wrapped
>> InputStream.
>>
>> Webrev: http://cr.openjdk.java.net/~jlahoda/8218287/webrev.00/
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8218287
>>
>> How does this look?
>>
>> Thanks,
>>     Jan