PATCH: ShutdownHooks and LoggingDeadlock2
Martin Buchholz
martinrb at google.com
Sun Apr 12 20:48:56 UTC 2009
Hi ShutdownHooks/Logging maintainers (especially Mandy),
This is a bug report for *two* bugs, with fix.
Bug 1:
Synopsis: test/java/util/logging/LoggingDeadlock2.java is flaky
Description:
The reg test java/util/logging/LoggingDeadlock2.java
has been failing intermittently for quite a while, at least since jdk7-b44.
It calls System.exit, which a jtreg test is not allowed to do (at
least not directly;
it must run the test in a subprocess). jtreg notices this if the
timing is just right.
Evaluation:
Yup. It's tedious to create a java subprocess, but we've done it
before and we know how.
Bug 2:
Synopsis: addShutdownHook fails if called after shutdown has commenced.
Description:
If you call addShutdownHook after shutdown has already started,
you should get IllegalStateException, as specified.
If the call to addShutdownHook is the first one in this JDK,
you will get an ExceptionInInitializerError instead, as below
Exception in thread "main" java.lang.ExceptionInInitializerError
at java.lang.Runtime.addShutdownHook(Runtime.java:209)
at java.util.logging.LogManager.<init>(LogManager.java:246)
at java.util.logging.LogManager$1.run(LogManager.java:194)
at java.security.AccessController.doPrivileged(Native Method)
at java.util.logging.LogManager.<clinit>(LogManager.java:175)
at LoggingDeadlock2$JavaChild.main(LoggingDeadlock2.java:106)
Caused by: java.lang.IllegalStateException: Shutdown in progress
at java.lang.Shutdown.add(Shutdown.java:77)
at java.lang.ApplicationShutdownHooks.<clinit>(ApplicationShutdownHooks.java:39)
This appears to be due to changes in
# HG changeset patch
# User mchung
# Date 1236878842 25200
# Node ID e1064300e0f6948e3ba820fab7a4040beeed930c
# Parent aa48deaf9af44bffce04a80ea1ea6a61901fd286
6810254: Lazily instantiate the shared secret access objects
Summary: Register the shutdown hooks only when needed and remove
JavaIODeleteOnExitAccess
Reviewed-by: alanb
--- a/src/share/classes/java/lang/ApplicationShutdownHooks.java
+++ b/src/share/classes/java/lang/ApplicationShutdownHooks.java
@@ -34,18 +34,18 @@
* @see java.lang.Runtime#removeShutdownHook
*/
-class ApplicationShutdownHooks implements Runnable {
- private static ApplicationShutdownHooks instance = null;
+class ApplicationShutdownHooks {
+ static {
+ Shutdown.add(1 /* shutdown hook invocation order */,
+ new Runnable() {
+ public void run() {
+ runHooks();
+ }
+ });
+ }
Evaluation:
ApplicationShutdownHooks.<clinit> needs to handle ISE from
Shutdown.add
PATCH: (The rewritten test is a regtest for bug 2)
# HG changeset patch
# User martin
# Date 1239567519 25200
# Node ID 8603336c1cfcb914cbb41deb6a2c8e141e96a803
# Parent 22b6e09960c153788717121f24ede64d845e8095
[mq]: LoggingDeadlock2
diff --git a/src/share/classes/java/lang/ApplicationShutdownHooks.java
b/src/share/classes/java/lang/ApplicationShutdownHooks.java
--- a/src/share/classes/java/lang/ApplicationShutdownHooks.java
+++ b/src/share/classes/java/lang/ApplicationShutdownHooks.java
@@ -26,34 +26,38 @@
import java.util.*;
-/*
+/**
* Class to track and run user level shutdown hooks registered through
* <tt>{@link Runtime#addShutdownHook Runtime.addShutdownHook}</tt>.
*
* @see java.lang.Runtime#addShutdownHook
* @see java.lang.Runtime#removeShutdownHook
*/
-
class ApplicationShutdownHooks {
static {
- Shutdown.add(1 /* shutdown hook invocation order */,
- new Runnable() {
- public void run() {
- runHooks();
- }
- });
+ try {
+ Shutdown.add(1 /* shutdown hook invocation order */,
+ new Runnable() {
+ public void run() {
+ runHooks();
+ }
+ });
+ } catch (IllegalStateException e) {
+ // shutdown already in progress
+ }
}
- /* The set of registered hooks */
+ /** The set of registered hooks */
private static IdentityHashMap<Thread, Thread> hooks = new
IdentityHashMap<Thread, Thread>();
private ApplicationShutdownHooks() {}
- /* Add a new shutdown hook. Checks the shutdown state and the hook itself,
+ /**
+ * Adds a new shutdown hook. Checks the shutdown state and the
hook itself,
* but does not do any security checks.
*/
static synchronized void add(Thread hook) {
- if(hooks == null)
+ if (hooks == null)
throw new IllegalStateException("Shutdown in progress");
if (hook.isAlive())
@@ -65,11 +69,12 @@
hooks.put(hook, hook);
}
- /* Remove a previously-registered hook. Like the add method, this method
+ /**
+ * Removes a previously-registered hook. Like the add method, this method
* does not do any security checks.
*/
static synchronized boolean remove(Thread hook) {
- if(hooks == null)
+ if (hooks == null)
throw new IllegalStateException("Shutdown in progress");
if (hook == null)
@@ -78,7 +83,8 @@
return hooks.remove(hook) != null;
}
- /* Iterates over all application hooks creating a new thread for each
+ /**
+ * Iterates over all application hooks creating a new thread for each
* to run in. Hooks are run concurrently and this method waits for
* them to finish.
*/
diff --git a/test/java/util/logging/LoggingDeadlock2.java
b/test/java/util/logging/LoggingDeadlock2.java
--- a/test/java/util/logging/LoggingDeadlock2.java
+++ b/test/java/util/logging/LoggingDeadlock2.java
@@ -26,7 +26,7 @@
* @bug 6467152
*
* @summary deadlock occurs in LogManager initialization and JVM termination
- * @author Serguei Spitsyn / Hittachi
+ * @author Serguei Spitsyn / Hitachi
*
* @build LoggingDeadlock2
* @run main/timeout=15 LoggingDeadlock2
@@ -47,43 +47,195 @@
* This is a regression test for this bug.
*/
+import java.util.Arrays;
+import java.util.List;
+import java.util.Random;
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.atomic.AtomicInteger;
import java.util.logging.LogManager;
+import java.io.File;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.Reader;
-public class LoggingDeadlock2 implements Runnable {
- static final java.io.PrintStream out = System.out;
- static Object lock = new Object();
- static int c = 0;
- public static void main(String arg[]) {
- out.println("\nThis test checks that there is no deadlock.");
- out.println("If not crashed or timed-out then it is passed.");
+public class LoggingDeadlock2 {
+
+ public static void realMain(String arg[]) throws Throwable {
try {
- new Thread(new LoggingDeadlock2()).start();
- synchronized(lock) {
- c++;
- if (c == 2) lock.notify();
- else lock.wait();
+ System.out.println(javaChildArgs);
+ ProcessBuilder pb = new ProcessBuilder(javaChildArgs);
+ ProcessResults r = run(pb.start());
+ equal(r.exitValue(), 99);
+ equal(r.out(), "");
+ equal(r.err(), "");
+ } catch (Throwable t) { unexpected(t); }
+ }
+
+ public static class JavaChild {
+ public static void main(String args[]) throws Throwable {
+ final CyclicBarrier startingGate = new CyclicBarrier(2);
+ final Throwable[] thrown = new Throwable[1];
+
+ // Some random variation, to help tickle races.
+ final Random rnd = new Random();
+ final boolean dojoin = rnd.nextBoolean();
+ final int JITTER = 1024;
+ final int iters1 = rnd.nextInt(JITTER);
+ final int iters2 = JITTER - iters1;
+ final AtomicInteger counter = new AtomicInteger(0);
+
+ Thread exiter = new Thread() {
+ public void run() {
+ try {
+ startingGate.await();
+ for (int i = 0; i < iters1; i++)
+ counter.getAndIncrement();
+ System.exit(99);
+ } catch (Throwable t) {
+ t.printStackTrace();
+ System.exit(86);
+ }
+ }};
+ exiter.start();
+
+ startingGate.await();
+ for (int i = 0; i < iters2; i++)
+ counter.getAndIncrement();
+ // This may or may not result in a first call to
+ // Runtime.addShutdownHook after shutdown has already
+ // commenced.
+ LogManager log = LogManager.getLogManager();
+
+ if (dojoin) {
+ exiter.join();
+ if (thrown[0] != null)
+ throw new Error(thrown[0]);
+ check(counter.get() == JITTER);
}
- LogManager log = LogManager.getLogManager();
- out.println("Test passed");
- }
- catch(Exception e) {
- e.printStackTrace();
- out.println("Test FAILED"); // Not expected
}
}
- public void run() {
- try {
- synchronized(lock) {
- c++;
- if (c == 2) lock.notify();
- else lock.wait();
- }
- System.exit(1);
+ //----------------------------------------------------------------
+ // The rest of this test is copied from ProcessBuilder/Basic.java
+ //----------------------------------------------------------------
+ private static final String javaExe =
+ System.getProperty("java.home") +
+ File.separator + "bin" + File.separator + "java";
+
+ private static final String classpath =
+ System.getProperty("java.class.path");
+
+ private static final List<String> javaChildArgs =
+ Arrays.asList(new String[]
+ { javaExe, "-classpath", classpath,
+ "LoggingDeadlock2$JavaChild"});
+
+ private static class ProcessResults {
+ private final String out;
+ private final String err;
+ private final int exitValue;
+ private final Throwable throwable;
+
+ public ProcessResults(String out,
+ String err,
+ int exitValue,
+ Throwable throwable) {
+ this.out = out;
+ this.err = err;
+ this.exitValue = exitValue;
+ this.throwable = throwable;
}
- catch(Exception e) {
- e.printStackTrace();
- out.println("Test FAILED"); // Not expected
+
+ public String out() { return out; }
+ public String err() { return err; }
+ public int exitValue() { return exitValue; }
+ public Throwable throwable() { return throwable; }
+
+ public String toString() {
+ StringBuilder sb = new StringBuilder();
+ sb.append("<STDOUT>\n" + out() + "</STDOUT>\n")
+ .append("<STDERR>\n" + err() + "</STDERR>\n")
+ .append("exitValue = " + exitValue + "\n");
+ if (throwable != null)
+ sb.append(throwable.getStackTrace());
+ return sb.toString();
}
}
+
+ private static class StreamAccumulator extends Thread {
+ private final InputStream is;
+ private final StringBuilder sb = new StringBuilder();
+ private Throwable throwable = null;
+
+ public String result () throws Throwable {
+ if (throwable != null)
+ throw throwable;
+ return sb.toString();
+ }
+
+ StreamAccumulator (InputStream is) {
+ this.is = is;
+ }
+
+ public void run() {
+ try {
+ Reader r = new InputStreamReader(is);
+ char[] buf = new char[4096];
+ int n;
+ while ((n = r.read(buf)) > 0) {
+ sb.append(buf,0,n);
+ }
+ } catch (Throwable t) {
+ throwable = t;
+ } finally {
+ try { is.close(); }
+ catch (Throwable t) { throwable = t; }
+ }
+ }
+ }
+
+ private static ProcessResults run(Process p) {
+ Throwable throwable = null;
+ int exitValue = -1;
+ String out = "";
+ String err = "";
+
+ StreamAccumulator outAccumulator =
+ new StreamAccumulator(p.getInputStream());
+ StreamAccumulator errAccumulator =
+ new StreamAccumulator(p.getErrorStream());
+
+ try {
+ outAccumulator.start();
+ errAccumulator.start();
+
+ exitValue = p.waitFor();
+
+ outAccumulator.join();
+ errAccumulator.join();
+
+ out = outAccumulator.result();
+ err = errAccumulator.result();
+ } catch (Throwable t) {
+ throwable = t;
+ }
+
+ return new ProcessResults(out, err, exitValue, throwable);
+ }
+
+ //--------------------- Infrastructure ---------------------------
+ static volatile int passed = 0, failed = 0;
+ static void pass() {passed++;}
+ static void fail() {failed++; Thread.dumpStack();}
+ static void fail(String msg) {System.out.println(msg); fail();}
+ static void unexpected(Throwable t) {failed++; t.printStackTrace();}
+ static void check(boolean cond) {if (cond) pass(); else fail();}
+ static void check(boolean cond, String m) {if (cond) pass(); else fail(m);}
+ static void equal(Object x, Object y) {
+ if (x == null ? y == null : x.equals(y)) pass();
+ else fail(x + " not equal to " + y);}
+ public static void main(String[] args) throws Throwable {
+ try {realMain(args);} catch (Throwable t) {unexpected(t);}
+ System.out.printf("%nPassed = %d, failed = %d%n%n", passed, failed);
+ if (failed > 0) throw new AssertionError("Some tests failed");}
}
More information about the core-libs-dev
mailing list