Review request for bug 8015762: java/nio/channels/DatagramChannel/AdaptDatagramSocket.java fails intermittently

[Chris Hegarty]

This looks ok to me ( since we are not expecting a timeout anyway, the 
change to 15 secs should not have any negative impact ).

-Chris.

On 09/18/2013 09:14 AM, Eric Wang wrote:
> Hi Chris,
>
> Yes, It looks a bit odd, i tried to cut down thread number of
> UdpEchoRequest to make response sent before socket timeout.
> Essentially, it is a timing issue. so the new fix below is to update
> change to SO_TIMEOUT value from 5 seconds to 15. I have tested on
> jsn-vm49.us for 20000 times and all passed.
> http://cr.openjdk.java.net/~ewang/8015762/webrev.02/test/java/nio/channels/DatagramChannel/AdaptDatagramSocket.java.sdiff.html
> <http://cr.openjdk.java.net/%7Eewang/8015762/webrev.02/test/java/nio/channels/DatagramChannel/AdaptDatagramSocket.java.sdiff.html>
>
>
> Can you please help to review?
> Thanks,
> Eric
> On 2013/9/11 18:30, Chris Hegarty wrote:
>> On 11/09/2013 10:47, Eric Wang wrote:
>>>
>>> On 2013/9/11 17:23, Alan Bateman wrote:
>>>> On 11/09/2013 09:58, Eric Wang wrote:
>>>>> Hi Alan,
>>>>>
>>>>> Sorry for late. I have re-fixed this failure, Can you please help to
>>>>> review?
>>>>> I executed the tests on the host jsn-vm49.us for thousands times and
>>>>> found the test failed as setting SO_TIMEOUT for 5 seconds is not
>>>>> enough to wait response sent by a new created thread of
>>>>> UdpEchoRequest. It may caused by thread schedule as there's maybe
>>>>> more than 3 threads executing at sametime or full GC as lots of
>>>>> UdpEchoRequest created in runtime.
>>>>>
>>>>> The fix is to change the SO_TIMEOUT from 5 seconds to 10 and not
>>>>> create a new thread of UdpEchoRequest to send response. I have run
>>>>> the fix for 20000 times, it works fine.
>>>>> http://cr.openjdk.java.net/~ewang/8015762/webrev.01/
>>>>> <http://cr.openjdk.java.net/%7Eewang/8015762/webrev.01/>
>>>>>
>>>> Thanks for confirming that the 5 second timeout is insufficient, that
>>>> part is clear now.
>>>>
>>>> The webrev also updates TestServers so that start runs the task
>>>> directly. Is this meant to be part of this change?
>>>>
>>>> -Alan.
>>>>
>>> Yes, it is a part of the fix, i have tested if only update timeout to 10
>>> sec, the test maybe still failed as the new created thread of
>>> UdpEchoRequest doesn't get chance to run. so I workaround the thread.
>>
>> This does look a little odd. I'm don't see why it is necessary. If
>> there is a still a timing issue should there be some other form of
>> synchronization between threads?
>>
>> -Chris.
>>
>>>
>>> Thanks,
>>> Eric
>>>
>