RFR: JDK-8185426: Jshell crashing on autocompletion

[Jan Lahoda]

On 24.8.2017 21:04, Robert Field wrote:
> Looks fine.

Thanks.

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

Ooops, thanks for noticing. Removed and pushed.

Jan

>
> -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
>>>
>>>
>