JDK 9 RFR of JDK-8132548: java/lang/ThreadGroup/Stop.java fails with "RuntimeException: Failure"

David Holmes david.holmes at oracle.com
Mon Jul 11 07:24:50 UTC 2016


My last email crossed with yours. Please see it.

David

On 11/07/2016 5:20 PM, Amy Lu wrote:
> Please review the updated webrev:
> http://cr.openjdk.java.net/~amlu/8132548/webrev.02/
>
> Thanks,
> Amy
>
> On 7/11/16 9:20 AM, Amy Lu wrote:
>> 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> 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