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

[joe darcy]

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);
>>                          }
>>                      }
>>
>>
>>
>>
>>
>>
>