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

[Chris Hegarty]

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>
>>>>
>>>>
>>>
>>
>>
>