RFR: JDK-8185426: Jshell crashing on autocompletion

[Robert Field]

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