PATCH: ShutdownHooks and LoggingDeadlock2
Martin Buchholz
martinrb at google.com
Mon Apr 13 20:28:50 UTC 2009
I've put the patch up on
http://cr.openjdk.java.net/~martin/LoggingDeadlock2
(my first use (or abuse? I didn't use webrev) of cr.openjdk.java.net)
I am not a shutdown hook expert - everything I know about shutdown hooks
I learned while tracking down LoggingDeadlock2 failures.
So I agree it would be good for you (Mandy) to pick this up.
One way to separate the work would be for you to file a bug for bug #1,
I check in only the fix for the test case for bug #1,
intentionally leaving it still flaky,
but serving as a regression test for your followon fix for bug #2.
You just need to add the @bug for bug #2 to my test.
Deal?
Martin
On Mon, Apr 13, 2009 at 10:56, Mandy Chung <Mandy.Chung at sun.com> wrote:
> 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