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

[David Holmes]

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