From 1adad4f6a1b55a786841a7a17db8cc294ad5b823 Mon Sep 17 00:00:00 2001 From: jgish Date: Fri, 19 Apr 2013 16:50:10 -0700 Subject: [PATCH] 8010939: Deadlock in LogManager Summary: re-order locks to avoid deadlock Reviewed-by: mchung --- .../classes/java/util/logging/LogManager.java | 13 +- .../util/logging/DrainFindDeadlockTest.java | 196 ++++++++++++++++++ 2 files changed, 201 insertions(+), 8 deletions(-) create mode 100644 test/java/util/logging/DrainFindDeadlockTest.java diff --git a/src/share/classes/java/util/logging/LogManager.java b/src/share/classes/java/util/logging/LogManager.java index 3cb35f584..8545e47c2 100644 --- a/src/share/classes/java/util/logging/LogManager.java +++ b/src/share/classes/java/util/logging/LogManager.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2012, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 2013, 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 @@ -35,10 +35,8 @@ import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.beans.PropertyChangeListener; -import java.net.URL; import sun.misc.JavaAWTAccess; import sun.misc.SharedSecrets; -import sun.security.action.GetPropertyAction; /** * There is a single global LogManager object that is used to @@ -148,7 +146,6 @@ public class LogManager { // The global LogManager object private static LogManager manager; - private final static Handler[] emptyHandlers = { }; private Properties props = new Properties(); private final static Level defaultLevel = Level.INFO; @@ -547,13 +544,10 @@ public class LogManager { throw new NullPointerException(); } - // cleanup some Loggers that have been GC'ed - manager.drainLoggerRefQueueBounded(); - LoggerWeakRef ref = namedLoggers.get(name); if (ref != null) { if (ref.get() == null) { - // It's possible that the Logger was GC'ed after the + // It's possible that the Logger was GC'ed after a // drainLoggerRefQueueBounded() call above so allow // a new one to be registered. removeLogger(name); @@ -605,6 +599,8 @@ public class LogManager { return true; } + // note: all calls to removeLogger are synchronized on LogManager's + // intrinsic lock void removeLogger(String name) { namedLoggers.remove(name); } @@ -887,6 +883,7 @@ public class LogManager { if (name == null) { throw new NullPointerException(); } + drainLoggerRefQueueBounded(); LoggerContext cx = getUserContext(); if (cx.addLocalLogger(logger)) { // Do we have a per logger handler too? diff --git a/test/java/util/logging/DrainFindDeadlockTest.java b/test/java/util/logging/DrainFindDeadlockTest.java new file mode 100644 index 000000000..13a959c07 --- /dev/null +++ b/test/java/util/logging/DrainFindDeadlockTest.java @@ -0,0 +1,196 @@ +/* + * Copyright (c) 2013, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ +import java.lang.management.ThreadInfo; +import java.lang.management.ThreadMXBean; +import java.lang.Thread.State; +import java.io.IOException; +import java.lang.management.ManagementFactory; +import java.util.logging.LogManager; +import java.util.logging.Logger; +import java.util.Map; + +/** + * @test + * @bug 8010939 + * @summary check for deadlock between findLogger() and drainLoggerRefQueueBounded() + * @author jim.gish@oracle.com + * @build DrainFindDeadlockTest + * @run main/othervm/timeout=10 DrainFindDeadlockTest + */ + +/** + * This test is checking for a deadlock between + * LogManager$LoggerContext.findLogger() and + * LogManager.drainLoggerRefQueueBounded() (which could happen by calling + * Logger.getLogger() and LogManager.readConfiguration() in different threads) + */ +public class DrainFindDeadlockTest { + private LogManager mgr = LogManager.getLogManager(); + private final static int MAX_ITERATIONS = 100; + + // Get a ThreadMXBean so we can check for deadlock. N.B. this may + // not be supported on all platforms, which means we will have to + // resort to the traditional test timeout method. However, if + // we have the support we'll get the deadlock details if one + // is detected. + private final static ThreadMXBean threadMXBean = + ManagementFactory.getThreadMXBean(); + private final boolean threadMXBeanDeadlockSupported = + threadMXBean.isSynchronizerUsageSupported(); + + public static void main(String... args) throws IOException, Exception { + new DrainFindDeadlockTest().testForDeadlock(); + } + + public static void randomDelay() { + int runs = (int) Math.random() * 1000000; + int c = 0; + + for (int i=0; i threadMap = + Thread.getAllStackTraces(); + dumpStack(threadMap.get(x), x); + dumpStack(threadMap.get(y), y); + } + } + + private void dumpStack(StackTraceElement[] aStackElt, Thread aThread) { + if (aStackElt != null) { + System.out.println("Thread:" + aThread.getName() + ": " + + aThread.getState()); + for (StackTraceElement element: aStackElt) { + System.out.println(" " + element); + } + } + } + + @Override + public void run() { + System.out.println("Running " + Thread.currentThread().getName()); + for (int i=0; i < MAX_ITERATIONS*2; i++) { + checkState(t1, t2); + try { + Thread.sleep(10); + } catch (InterruptedException ex) { + }; + } + } + } +} -- GitLab