RFR: JDK-8185426: Jshell crashing on autocompletion

[Robert Field]

Looks fine.

It adds an import for CountDownLatch to ConsoleIOContext.java, which is 
not longer needed.

-Robert


On 08/24/2017 03:40 AM, Jan Lahoda wrote:
> Thanks for the comments. An updated webrev is here:
> http://cr.openjdk.java.net/~jlahoda/8185426/webrev.01/
>
> Thanks,
>    Jan
>
> On 17.8.2017 02:18, Robert Field wrote:
>>
>> On 07/28/17 07:45, Jan Lahoda wrote:
>>> I'd like to ask for a review of a fix for bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8185426
>>>
>>> Proposed webrev:
>>> http://cr.openjdk.java.net/~jlahoda/8185426/webrev.00/
>>>
>>> The problem here is that: jshell has a stateful completion, pressing
>>> <tab> several times will produce different outputs. The state of the
>>> completion is currently reset when the user performs action different
>>> than "COMPLETE" as determined by KeyMap. Sadly, even if the action is
>>> COMPLETE, JLine may (if there are further characters in the input)
>>> decide it won't perform the complete action, rather print the tab
>>> character. See:
>>> http://hg.openjdk.java.net/jdk10/jdk10/jdk/file/0861f2298fd7/src/jdk.internal.le/share/classes/jdk/internal/jline/console/ConsoleReader.java#l2669 
>>>
>>>
>>>
>>> This then can lead to the problem, as the completion may be using the
>>> stale state.
>>>
>>> So the patch is trying to ensure the completion state is reset if
>>> complete is not called: the state is reset always, and the complete
>>> method will restore it if needed.
>>>
>>> How does this look?
>>>
>>> Thanks,
>>>     Jan
>>
>> The change makes ConsoleIOContext public, adds two static fields and
>> embeds some test-specific code to the class.  This complicates the class
>> and mixes it with test code.
>> I think it would be better to add a public test access class, which
>> holds these statics and the test-specific code.  You would still need a
>> simple hook to get to this test support.
>>
>> Nit: the initialization of todo (line 252) is placed after a comment
>> that does not document it -- they should be switched.
>>
>> The meanings, invariants, and transitions of the new CompletionState
>> fields should be documented.  In general, ConsoleIOContext should be
>> documented.  The one existing comment (on the
>> ConsoleIOContext.complete() method) needs update.
>>
>> -Robert
>>
>>