Review request for test bug 8015765: java/nio/channels/ServerSocketChannel/AdaptServerSocket.java

[David Holmes]

Eric,

Glad to hear Thread priority changes are gone :)  (Just forget Thread 
priority even exists - it is meaningless.)

Minor style nit:

   82         try (ServerSocketChannel ssc = ServerSocketChannel.open(); 
ServerSocket sso = ssc.socket();) {

Use a line per declaration.

I don't know what the key aspects of this test are but I suspect that 
"clientStarted" really needs to indicate more than the fact the thread 
has started execution of its run() method - ie it should come as close 
as possible to the action that the main part of the test depends upon.

David



On 15/08/2013 1:22 PM, Eric Wang wrote:
> Hi Chris & Alan,
>
> Thanks for the review, I have updated the code based on your
> suggestions. Please help to review again, Thanks!
> http://cr.openjdk.java.net/~ewang/8015765/webrev.01/
> <http://cr.openjdk.java.net/%7Eewang/8015765/webrev.01/>
>
> Eric
> On 2013/8/13 21:48, Chris Hegarty wrote:
>> This test looks a little bizarre, and certainly racy. What you have
>> done should make the test more reliable.
>>
>> I noticed that the server socket channel is only closed if a socket is
>> accepted. I think it should be closed always, regardless of whether a
>> socket is accepted or not. Though this is not directly related to your
>> changes.
>>
>> -Chris
>>
>> On 08/13/2013 12:24 PM, Eric Wang wrote:
>>> Hi,
>>>
>>> Please help to review the fix below for the bug
>>> https://jbs.oracle.com/bugs/browse/JDK-8015765.
>>> http://cr.openjdk.java.net/~ewang/8015765/webrev.00/
>>>
>>> As the test is depended on thread schedule, it may fail
>>> intermittently.The fix is to adjust thread priority, and increase thread
>>> sleep duration, to make it more stable.
>>>
>>> Thanks,
>>> Eric
>