RFR 8144906: Fix jshell's ToolBasicTest

[Robert Field]

Thank you. Very cool!

This should solve one of the most frequent problems that regular users 
encounter, output arriving after the prompt.

-Robert



On December 10, 2015 3:29:27 AM Jan Lahoda <jan.lahoda at oracle.com> wrote:

> On 9.12.2015 22:10, Robert Field wrote:
>> Jan,
>>
>> Can you explain how using the same channel assures that the processing of 
>> output will be completed before the snippet completes?
>
> Considering for example this user's snippet:
> System.err.println("A");
>
> Currently, in the RemoteAgent's VM, the "A\n" is first written to the
> err, and then the confirmation the action was finished is sent using the
> command socket.
>
> But, in the main JShell process, the RemoteAgent's err and the command
> socket are processed using two different threads. So, it may, I think,
> happen the thread that handles the command socket will read and process
> the confirmation the action finished before the thread that copies
> RemoteAgent's err to proc.err finishes the copying. Which then can lead
> to a prompt being written to the output before the user's output.
>
> The idea in the proposed patch is that we serialize all out, err and the
> command stream into one stream, and then there is a single thread in the
> main JShell process that splits the stream and sends the data into
> appropriate targets. So, everything that arrives into our "err" stream
> in the RemoteAgent process before the action ends should be sent to the
> main process before the action finished confirmation, and it should be
> copied into the target proc.err before the command finished confirmation
> is read and processed.
>
> This may still require to flush the streams so that the data are really
> sent/visible, but should eliminate the problem with data being handled
> in the wrong order.
>
> Does this make sense?
>
> Jan
>
>>
>> Thanks,
>> Robert
>>
>>> On Dec 9, 2015, at 7:24 AM, Jan Lahoda <jan.lahoda at oracle.com> wrote:
>>>
>>> Hello,
>>>
>>> The ToolBasicTest is @ignored now, as it fails on some platforms. I am 
>>> proposing a partial fix for that, the patch is available here:
>>> http://cr.openjdk.java.net/~jlahoda/8144906/webrev.00/
>>>
>>> Notes:
>>> -currently, there are three communication channels between the RemoteAgent 
>>> process and the main JShell process: the command socket and RemoteAgent's 
>>> out and err. The problem is that it is not easy to synchronize events 
>>> between these channels, so it may happen that the command is already 
>>> finished, while the out and err has not been fully processed yet. My 
>>> proposal here is to change the System.out and System.err so that these also 
>>> use the socket stream, so that there is (normally) only one communication 
>>> channel, allowing us to fully process the out/err before finishing a 
>>> command. This should also fix JDK-8131013. Note I kept the existing code to 
>>> copy the RemoteAgent's stdout/stderr, just in case the RemoteAgent prints 
>>> something there (for debugging, for example), even though this shouldn't be 
>>> typically used.
>>>
>>> -unfortunately, there are still some problems on Solaris, which I wasn't 
>>> able to debug yet, so the test is excluded on Solaris.
>>>
>>> -I added synchronization to JDIConnection, which should ensure proper 
>>> access to the fields, and avoid the need to catch the NPE in JDIConnection.
>>>
>>> -ReplToolTesting now converts the platform's line endings to '\n', to 
>>> eliminate platform specific problems with line endings.
>>>
>>> How does this look?
>>>
>>> Thanks,
>>>     Jan
>>