RFR of JDK-8171404,(ch) java/nio/channels/AsynchronousSocketChannel/Basic.java failed with "AsynchronousCloseException expected"

[Hamlin Li]

On 19/04/2018 11:39 PM, Alan Bateman wrote:
>
>
> On 18/04/2018 06:54, Hamlin Li wrote:
>> would you please review the following patch?
>>
>> bugs:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8171404
>> https://bugs.openjdk.java.net/browse/JDK-8201520
>> https://bugs.openjdk.java.net/browse/JDK-8161991
>>
>> webrev: http://cr.openjdk.java.net/~mli/8171404/webrev.00/
>>
>>
>> On my local physical mac, the test takes about 30 seconds to finish, 
>> to give test sufficient time on extremely busy system and reduce the 
>> timeout chance, the timeout value is adjusted to 600, and let it run 
>> in a othervm.
>>
>> On extreme situation, theoretically the test could fail in following 
>> cases:
>>
>> a. 1. detected that write buffer is filled up in main thread, go 
>> ahead, 2. pending write completed in async thread pool, 3. write 
>> concurrently to the channel in main thread before write starts in 
>> async thread pool, it will cause "WritePendingException expected"
>>
>> b. 1. detected that write buffer is filled up in main thread, 2. get 
>> expected exception when write concurrently to the channel in main 
>> thread, 3. pending write completed in async thread pool, 4. 
>> ch.close() in main thread, 5. try to write to the closed channel in 
>> thread pool, it will cause "AsynchronousCloseException expected"
>>
>> To fix these 2 issues,
>>
>> 1. set the receive buffer on server side to a minimal value;
>>
>> 2. spend more time when wait for filling up write buffer on busy system.
>>
>> 3. add log to collect more information in case it still fails.
> Running it in /othervm and changing the timeout is fine.
Hi Alan,
Thank you for reviewing.
>
> The existing output goes to System.out, the new trace messages that 
> you are adding is going to System.err so I assume will make it harder 
> to debug. Is there a reason for that?
Fixed.
I thought it might help to print out instantly with err as some of them 
are printed out from other threads in a thread pool, but seems it does 
not matter?
>
> "revBuf", I think you have a typo in the parameter name, something 
> like "recvBufSize" might be better.
>
> "it may not help to set receive buffer" - can this be left out of the 
> patch as I'm sure it will confuse further maintainers of this test.
fixed
>
> You've changed the sleep in the loop that waits for the socket buffer 
> to fill up but I assume the timeout doesn't matter as it loops while 
> there isn't any more progress. What you have is okay of course but I'm 
> not sure that it will help much.
I think this might help at extreme situation, so would like to keep it, 
how do you think about it?

the webrev is updated in place.

Thank you
-Hamlin
>
> -Alan
>
>
>
>