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

[Robert Field]

Thumbs up on the update.

-Robert


On 2/18/19 8:37 AM, Jan Lahoda wrote:
> 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