[PATCH] Review request: 8004177: test/java/lang/Thread/GenerifyStackTraces.java doesn't clean-up

[Eric Wang]

Hi David,

Thanks for review this issue, i'll file another bug to track this.

Regards,
Eric
On 2013/5/16 12:30, David Holmes wrote:
> Hi Eric,
>
> Cleanup of threads looks good.
>
> The synchronization between waitForDump and finishDump is a bit dodgy 
> but that is a different issue.
>
> Cheers,
> David
>
> On 15/05/2013 9:07 PM, Eric Wang wrote:
>> On 2013/5/15 17:38, Alan Bateman wrote:
>>> On 15/05/2013 10:10, Eric Wang wrote:
>>>> Hi,
>>>>
>>>> Please help to review the fix for bug 8004177
>>>> <https://jbs.oracle.com/bugs/browse/JDK-8004177> and 8004178
>>>> <https://jbs.oracle.com/bugs/browse/JDK-8004178>, this fix is to make
>>>> sure all child threads finished before main thread exits.
>>>>
>>>> http://cr.openjdk.java.net/~ewang/8004177/webrev.00/
>>>> <http://cr.openjdk.java.net/%7Eewang/8004177/webrev.00/>
>>>>
>>>> For 8004178, the test StackTraces.java is same as
>>>> GenerifyStackTraces.java, so just remove it.
>>>>
>>>> Thanks,
>>>> Eric
>>> It's good to have fix this test so that it doesn't leave a thread
>>> behind (in this case slowing down the execution of subsequent tests by
>>> taking thread dumps every 2 seconds)
>>>
>>> The change you propose is okay but a bit odd to have ThreadOne signal
>>> DumpThread to shutdown. An alternative would be to have the main
>>> thread signal DumpThread, as in:
>>>
>>>   one.join();
>>>
>>>   finished = true;
>>>   dt.join();
>>>
>>> Better still might be to move the flag into the DumpThread class and
>>> have it define a shutdown method so the main thread does:
>>>
>>>   one.join();
>>>
>>>   dt.shutdown();
>>>   dt.join();
>>>
>>> I think that would be a bit cleaner.
>>>
>>> -Alan.
>>>
>>>
>>>
>> Hi Alan,
>>
>> Thanks for the suggestion, I have updated the fix again, Can you please
>> help to take a look?
>> http://cr.openjdk.java.net/~ewang/8004177/webrev.01/
>> <http://cr.openjdk.java.net/%7Eewang/8004177/webrev.01/>
>>
>> Regards,
>> Eric