RFR [7181748] TEST_BUG: java/lang/ThreadGroup/Suspend test fails intermittently

[Ivan Gerasimov]

Thank you Chris, I would really appreciate your sponsor's help!

Here's the patch for this change: 
http://cr.openjdk.java.net/~igerasim/2commit/7181748-jdk8-Suspend-test-fails.patch

Sincerely yours,
Ivan

On 13.06.2013 14:35, Chris Hegarty wrote:
> The updated webrev looks good to me Ivan. Sorry for sending you off on 
> the wrong track!
>
> Let me know if you need a sponsor to push this.
>
> -Chris.
>
> On 06/13/2013 11:33 AM, Ivan Gerasimov wrote:
>> Thank you, David.
>>
>> I've reverted the code back, added volatile specification to the count
>> variable.
>> Would you please review the webrev with this only change (comparing to
>> webrev.0):
>> http://cr.openjdk.java.net/~igerasim/7181748/webrev.2/
>>
>> Sincerely yours,
>> Ivan
>>
>> On 12.06.2013 13:49, David Holmes wrote:
>>> On 12/06/2013 12:13 AM, Ivan Gerasimov wrote:
>>>> Chris and David, thanks for review.
>>>>
>>>> I've updated the test so the threads use CountDownLatch to wait for 
>>>> each
>>>> other.
>>>> Now, instead of checking whether the 'second' thread has incremented a
>>>> count, the 'main' thread waits for it to call countDown().
>>>> If a timeout was elapsed before the call, I assume that the 'second'
>>>> thread is suspended and the test passes.
>>>
>>> I do not advise doing this. The second thread can be suspended inside
>>> the CountdownLatch code. When suspend is involved you do not want any
>>> direct interaction between the main thread and the thread being
>>> suspended.
>>>
>>> David
>>> -----
>>>
>>>> Could you please review the updated webrev:
>>>> http://cr.openjdk.java.net/~igerasim/7181748/webrev.1/
>>>>
>>>> Sincerely,
>>>> Ivan
>>>>
>>>> On 11.06.2013 13:37, Chris Hegarty wrote:
>>>>> On 06/11/2013 08:08 AM, David Holmes wrote:
>>>>>> On 11/06/2013 1:54 AM, Chris Hegarty wrote:
>>>>>>> I'm not sure I ever saw this test fail, but it does look like it 
>>>>>>> has
>>>>>>> issues.
>>>>>>>
>>>>>>> I would much prefer to see a j.u.c.Latch/Phaser used to synchronize
>>>>>>> across these threads.
>>>>>>
>>>>>> I don't think that is possible. The main thread wants to reset the
>>>>>> count
>>>>>> after the suspend has been issued by "first", but once the 
>>>>>> suspend has
>>>>>> been issued the "first" thread can't signal the synchronizer to let
>>>>>> the
>>>>>> main thread proceed. The poll around isAlive() does address the main
>>>>>> issue of a slowly starting thread.
>>>>>
>>>>> Ah yes, thanks David. In which case I'm fine with the change.
>>>>>
>>>>>> For completeness the count variable should be volatile as well.
>>>>>
>>>>> Agreed.
>>>>>
>>>>> -Chris.
>>>>>
>>>>>>
>>>>>> Cheers,
>>>>>> David
>>>>>>
>>>>>>> -Chris.
>>>>>>>
>>>>>>> On 10/06/2013 13:51, Ivan Gerasimov wrote:
>>>>>>>> Hello everyone!
>>>>>>>>
>>>>>>>> The test of ThreadGroup.Suspend() was reported to fail on rare
>>>>>>>> occasions.
>>>>>>>> It can happen on a busy machine that 1 second delay would not be
>>>>>>>> enough
>>>>>>>> for the second thread to start.
>>>>>>>> Then the first thread would suspend only itself and the test would
>>>>>>>> fail.
>>>>>>>> Earlier, another test was updated for similar reasons [1], [2].
>>>>>>>>
>>>>>>>> The proposed test can still report false *positive* results if the
>>>>>>>> second thread has had no chance to execute during one second
>>>>>>>> after it
>>>>>>>> had started.
>>>>>>>> To avoid them there must be a way do distinguish suspended 
>>>>>>>> threads.
>>>>>>>>
>>>>>>>> WEBREV: http://cr.openjdk.java.net/~igerasim/7181748/webrev.0/
>>>>>>>> BUG: http://bugs.sun.com/view_bug.do?bug_id=7181748
>>>>>>>>
>>>>>>>>
>>>>>>>> [1] http://bugs.sun.com/view_bug.do?bug_id=7084033
>>>>>>>> [2] http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/184578f3e8b9
>>>>>>>>
>>>>>>>> Sincerely yours,
>>>>>>>> Ivan Gerasimov
>>>>>>>> <https://jbs.oracle.com/bugs/browse/JDK-7084033>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
>