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

[Amy Lu]

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