RFR 8144906: Fix jshell's ToolBasicTest

[Jan Lahoda]

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
>