JDK 9 RFR of JDK-8132548: java/lang/ThreadGroup/Stop.java fails with "RuntimeException: Failure"
Amy Lu
amy.lu at oracle.com
Mon Jul 11 01:20:36 UTC 2016
Thank you for all the valuable comments.
I'm updating the webrev...
Thanks,
Amy
On 7/9/16 1:34 AM, Martin Buchholz wrote:
> jdk/test/java/util/concurrent/tck has thousands of test methods. It
> used to take minutes to run them all, but now only takes 10 seconds,
> mostly due to replacements of sleeps with faster and more robust
> alternatives, often CountDownLatch.
>
> On Fri, Jul 8, 2016 at 10:17 AM, joe darcy <joe.darcy at oracle.com
> <mailto:joe.darcy at oracle.com>> wrote:
>
> The most surefire way to make sure the test doesn't fail anymore
> is to hg rm the test; if and unless the code is actually removed,
> that would not be the most appropriate approach though ;-)
>
> While it might be overkill for this particular test, I think it
> would be preferable to start replacing our various sleep calls in
> tests with count down latches or other structures as appropriate.
> Converting this test could help provide a good example of the process.
>
> (As alluded to earlier, the test suite as a whole could be made to
> run somewhat faster. Tests which wait for seconds when the entire
> test could commonly run in milliseconds in many cases are
> proportionately a good candidate to speed up. The absolute wait
> time are also problematic on the other extreme, running under
> -Xint on a heavily loaded test system, "should never take this
> long" numbers often aren't enough.)
>
> Thanks,
>
> -Joe
>
>
> On 7/7/2016 11:46 PM, Amy Lu wrote:
>> Yes, but I just thought that for a test that testing a deprecated
>> (since 1.2) API, and failed with very very low frequency
>> (actually, only one time during the years), we might not want to
>> spend much effort on a big change, like rewrite with
>> CountDownLatch :-)
>>
>>
>> Thanks,
>> Amy
>>
>> On 7/8/16 2:36 PM, Martin Buchholz wrote:
>>> CountDownLatch is a better way of waiting for events, like for
>>> the two threads to be started and for Thread.stop to have been
>>> called.
>>>
>>> The test should ensure that ThreadDeath is indeed thrown. If
>>> the threads in the group notify the main thread via a latch when
>>> they catch ThreadDeath, then all the sleeps in this test can be
>>> removed.
>>>
>>> On Thu, Jul 7, 2016 at 11:01 PM, Amy Lu <amy.lu at oracle.com
>>> <mailto:amy.lu at oracle.com>> wrote:
>>>
>>> Thank you Joe for your review.
>>>
>>> The intent is to give it more chance "for the thread group
>>> stop to be issued", not to extend the whole test execution
>>> timeout.
>>>
>>> I updated the webrev to make this in a retry, limit to 5
>>> times of retry:
>>> http://cr.openjdk.java.net/~amlu/8132548/webrev.01/
>>> <http://cr.openjdk.java.net/%7Eamlu/8132548/webrev.01/>
>>>
>>> Thanks,
>>> Amy
>>>
>>>
>>> On 7/8/16 12:15 PM, joe darcy wrote:
>>>
>>> Hi Amy,
>>>
>>> I'm a bit uncomfortable with the fix as-is.
>>>
>>> Rather than hard-coding sleep values, if sleep values
>>> are needed I think it is a better practice to use ones
>>> that are scaled with the jtreg timeout factors, etc.
>>> used to run the tests. Please instead use something like
>>> the adjustTimeout method of
>>>
>>> $JDK_FOREST_ROOT/test/lib/share/classes/jdk/test/lib/Utils
>>>
>>> As a general comment, I'd prefer we don't just up
>>> timeout values for tests. That can cause the whole test
>>> suite run to slow down, which is undesirable especially
>>> if the condition in question may actually be satisfied
>>> in many cases much faster than the timeout value.
>>>
>>> Thanks,
>>>
>>> -Joe
>>>
>>>
>>> On 7/7/2016 7:01 PM, Amy Lu wrote:
>>>
>>> Please review this trivial fix for
>>> test:java/lang/ThreadGroup/Stop.java
>>>
>>> Though this is a test for a deprecated API, failed
>>> with very very low frequency and hard to reproduce
>>> (I got no luck to reproduce it), I’d like to patch
>>> it as suggested: extend the sleep in the main thread
>>> from one second to five seconds. Also added
>>> 'volatile' to the boolean variable 'groupStopped'.
>>>
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8132548
>>> webrev:
>>> http://cr.openjdk.java.net/~amlu/8132548/webrev.00/
>>> <http://cr.openjdk.java.net/%7Eamlu/8132548/webrev.00/>
>>>
>>> Thanks,
>>> Amy
>>>
>>>
>>> --- old/test/java/lang/ThreadGroup/Stop.java
>>> 2016-07-04 14:53:59.000000000 +0800
>>> +++ new/test/java/lang/ThreadGroup/Stop.java
>>> 2016-07-04 14:53:58.000000000 +0800
>>> @@ -1,5 +1,5 @@
>>> /*
>>> - * Copyright (c) 1999, 2011, Oracle and/or its
>>> affiliates. All rights reserved.
>>> + * Copyright (c) 1999, 2016, Oracle and/or its
>>> affiliates. All rights reserved.
>>> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS
>>> FILE HEADER.
>>> *
>>> * This code is free software; you can redistribute
>>> it and/or modify it
>>> @@ -29,7 +29,7 @@
>>> */
>>>
>>> public class Stop implements Runnable {
>>> - private static boolean groupStopped = false ;
>>> + private static volatile boolean groupStopped =
>>> false ;
>>> private static final Object lock = new Object();
>>>
>>> private static final ThreadGroup group = new
>>> ThreadGroup("");
>>> @@ -70,7 +70,7 @@
>>> while (!groupStopped) {
>>> lock.wait();
>>> // Give the other thread a chance
>>> to stop
>>> - Thread.sleep(1000);
>>> + Thread.sleep(5000);
>>> }
>>> }
>>>
>>>
>>>
>>>
>>>
>>>
>>
>
>
More information about the core-libs-dev
mailing list