RFR for bug JDK-8025209 Intermittent test failure: java/net/Socket/asyncClose/AsyncClose.java

Eric Wang yiming.wang at oracle.com
Mon Mar 31 13:48:13 UTC 2014


Hi Chris,

Thanks for the push. I'm glad to learn the modern way from your refactor.

- Cheers,
Eric
On 2014/3/31 18:37, Chris Hegarty wrote:
> Thanks Eric,
>
> Pushed
>   http://hg.openjdk.java.net/jdk9/dev/jdk/rev/e0d1a825848f
>
> There was a small issue of secs vs millis. Your webrev used 5 millis 
> as a sleep rather than 5000. I fixed this before pushing.
>
> Also, with the changes the time for the test increased so I trivially 
> added a change to have the various socket type checks run in parallel. 
> With this the run time of the test is half of that of the original test.
>
> -Chris.
>
>
> On 30/03/14 03:55, Eric Wang wrote:
>> Hi Chris,
>>
>> I have updated the webrev according to your suggestions. Can you please
>> review it again?
>> http://cr.openjdk.java.net/~ewang/JDK-8025209/webrev.01/
>>
>> Thanks to review the fix and sponsor it for me.
>> -Eric
>>
>> On 2014/3/28 18:41, Chris Hegarty wrote:
>>> Hi Eric,
>>>
>>> I've taken a look at the webrev and I mainly agree with the
>>> changes/refactoring.
>>>
>>> On the issue of increasing the timeout to 20 secs; there is a clear
>>> race in this test, and your refactoring has reduced it as much as
>>> possible. Since the sleep will always occur, I would prefer to reduce
>>> it from 20 secs to 5 secs. That, in combination with your use of a
>>> latch, should be sufficient.
>>>
>>> Also, there is a variant of each test that runs with a 5,000
>>> millisecond timeout. This timeout should be increased to 20,000
>>> milliseconds. Otherwise, the blocking IO call may timeout rather than
>>> be interrupted ( which is what the test is testing for ).
>>>
>>> If you address the above issues, I will be happy to sponsor this
>>> change for you.
>>>
>>> -Chris.
>>>
>>> On 28/03/14 05:54, Eric Wang wrote:
>>>> Hi Chris & All,
>>>>
>>>> Sorry that the fix spent a bit long time for internal review.
>>>> Can you please review the fix below for bug JDK-8025209
>>>> <https://bugs.openjdk.java.net/browse/JDK-8025209>. The fix is just to
>>>> make the thread synchronization is more robust and made some cleanup.
>>>> http://cr.openjdk.java.net/~ewang/JDK-8025209/webrev.00/
>>>>
>>>> Thanks,
>>>> Eric
>>>> On 2014/3/5 18:01, Chris Hegarty wrote:
>>>>> On 5 Mar 2014, at 09:48, Eric Wang<yiming.wang at oracle.com>  wrote:
>>>>>
>>>>>> Hi everyone,
>>>>>>
>>>>>> Hi Everyone,
>>>>>>
>>>>>> I'm working on the test
>>>>>> bughttps://bugs.openjdk.java.net/browse/JDK-8025209, The test uses
>>>>>> Thread.sleep to sync-up threads which is not a correct assumption.
>>>>>> The fix is just to sync-up 2 threads using proper way.
>>>>>>
>>>>>> The webrev will be sent after internal review. Please let me know
>>>>>> if you have any comment.
>>>>> This test is part of the OpenJDK source. Please just post your
>>>>> webrev externally for review.
>>>>>
>>>>> Thanks,
>>>>> -Chris.
>>>>>
>>>>>> Thanks,
>>>>>> Eric
>>>>>>
>>>>
>>



More information about the net-dev mailing list