Looks okay to me.
Thanks,
David
On 11/10/2013 10:32 PM, 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.hol...@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
