PATCH: ShutdownHooks and LoggingDeadlock2
Mandy Chung
Mandy.Chung at Sun.COM
Mon Apr 13 17:56:56 UTC 2009
Hi Martin,
Thanks for catching the regression. I created a Sun bug # 6829503 for
bug 2.
With the fix for 6829503, ApplicationShutdownHook and DeleteOnExitHook
are both lazily initialized. Since it is possible that a file could be
added to the list for DeletedOnExit during shutdown, perhaps we also
need to change the Shutdown.add method to allow DeleteOnExitHook to be
added during shutdown.
I should pick up this bug since my fix caused this regression. Or do
you want to revise the patch?
Thanks
Mandy
Martin Buchholz wrote:
> 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