RFR of JDK-8049316: TEST_BUG: java/nio/channels/Selector/Wakeup.java fails in same binary run b19
Hamlin Li
huaming.li at oracle.com
Thu Dec 1 02:27:19 UTC 2016
Hi Roger,
Thank you for reviewing, fixed timeout scale and pushed.
Thank you
-Hamlin
On 2016/12/1 3:49, Roger Riggs wrote:
> Hi Hamlin,
>
>
> On 11/29/2016 9:09 PM, Hamlin Li wrote:
>> Hi Roger,
>>
>> Thank you for reviewing, please check comments in line.
>>
>> webrev: http://cr.openjdk.java.net/~mli/8049316/webrev.01/
>>
>>
>> On 2016/11/30 0:03, Roger Riggs wrote:
>>> Hi Hamlin,
>>>
>>> Wakeup.java:
>>>
>>> - Some refactoring may make it easier to understand.
>>> Perhaps moving waitSleeper to be a method on Sleeper.
>>> The use of cyclicBarrier seems fine but could use a comment
>>> somewhere saying what
>>> threads/functions are being coordinated; putting them both in
>>> Sleeper would make it easier to see.
>>>
>>> - line 126,127: Add a Sleeper constructor to initialize these as
>>> needed.
>>>
>>> - Some of the test stimulus is buried in the newSleeper methods that
>>> was previously
>>> in line in main. It seemed clearer what case was being tested in
>>> the original.
>>> They are buried in static factory 'newSleeper' functions with
>>> uninformative names.
>>> lines 112, 116, 120.
>> All above fixed.
> I was thinking that the createSleeper(sel, want, before) method forced
> two cases together that
> didn't need to be in the same method and could be separated in the
> newSleeperWantInterrupt and
> newSleeperWithInterruptBeforeSelect factory methods.
>
>
> line 104: the timeout value should be scaled by
> jdk/testlibrary/Utils.adjustTimeout (test.timeout.factor).
>
>>>
>>> - If I read it right, some cases where events don't happen that
>>> used to be reported as exceptions
>>> ("Interrupt never delivered") will now be reported as the test
>>> timing out. (infinite loop at line 85).
>> No, it will finally checked by a time sleeper.finish(0). At first, I
>> don't want to depends on a specific time, because I don't know what
>> exact timeout we should use, but as you asked, I think it's ok to
>> finish(20_000), it should be long enough.
>
> Otherwise is fine.
>
> Thanks, Roger
>
>>
>> Thank you
>> -Hamlin
>>>
>>> Thanks, Roger
>>>
>>> On 11/29/2016 4:23 AM, Hamlin Li wrote:
>>>> Would you please review the below patch?
>>>>
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8049316
>>>> webrev: http://cr.openjdk.java.net/~mli/8049316/webrev.00/
>>>>
>>>> Root cause:
>>>> 1. it depends on sleeping time to check failure, which is not
>>>> reliable in some extreme situation
>>>> 2. it mix several tests together with a loop in Sleeper
>>>>
>>>> Solution:
>>>> 1. synchronize between threads.
>>>> 2. isolate tests.
>>>>
>>>> Thank you
>>>> Hamlin
>>>
>>
>
More information about the core-libs-dev
mailing list