RFR: 8004183: test/sun/management/jmxremote/LocalRMIServerSocketFactoryTest.java doesn't clean-up

Jaroslav Bachorik jaroslav.bachorik at oracle.com
Mon Oct 14 00:58:13 PDT 2013


Looks fine.

-JB-

On 11.10.2013 14:32, Peter Allwin wrote:
> Thanks for the feedback, good points!
>
> I've put an updated webrev here:
>
> http://cr.openjdk.java.net/~allwin/8004183/webrev.01/
>
> Changes:
>      boolean instead of Boolean
>      no longer sets worker as daemon
>
> Regards,
> /peter
>
> On Oct 11, 2013, at 1:13 PM, David Holmes <david.holmes at oracle.com> wrote:
>
>> On 11/10/2013 8:18 PM, Daniel Fuchs wrote:
>>> Hi Peter,
>>>
>>> Looks good to me - but you might want to use 'boolean' for
>>> isRunning rather than 'Boolean'.
>>
>> Definitely!
>>
>>> Joining on the daemon thread is probably not necessary,
>>> but there's no harm in it (the important part being
>>> isRunning=false + s.close()).
>>
>> It is pointless having the thread be a daemon now. If running in the same VM as anything else joining will ensure this test is cleaned up before the next test commences.
>>
>> David
>> -----
>>
>>> best regards,
>>>
>>> -- daniel (not a reviewer)
>>>
>>> On 10/11/13 11:53 AM, shanliang wrote:
>>>> Looks good to me.
>>>>
>>>> Shanliang
>>>>
>>>>
>>>> Peter Allwin wrote:
>>>>> Hello!
>>>>>
>>>>> Looking for reviews of this fix where a jmxremote test left a worker
>>>>> thread running after completion. Fix is to flag the thread to finish
>>>>> and join before test method exit.
>>>>>
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8004183
>>>>> cr: http://cr.openjdk.java.net/~allwin/8004183/webrev.00
>>>>>
>>>>> Thanks!
>>>>> /peter
>>>>
>>>
>



More information about the serviceability-dev mailing list