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