PATCH: ShutdownHooks and LoggingDeadlock2
Mandy Chung
Mandy.Chung at Sun.COM
Mon Apr 13 20:54:28 UTC 2009
Sounds good.
The CR for bug #1 is 6829636. I'll let Swamy to review the test fix.
Mandy
On 04/13/09 13:28, Martin Buchholz wrote:
> 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