src/share/classes/java/util/logging/LogManager.java

Print this page

        

@@ -144,11 +144,18 @@
 
 public class LogManager {
     // The global LogManager object
     private static final LogManager manager;
 
-    private Properties props = new Properties();
+    // 'props' is assigned within a lock but accessed without it.
+    // Declaring it volatile makes sure that another thread will not
+    // be able to see a partially constructed 'props' object.
+    // (seeing a partially constructed 'props' object can result in
+    // NPE being thrown in Hashtable.get(), because it leaves the door
+    // open for props.getProperties() to be called before the construcor
+    // of Hashtable is actually completed).
+    private volatile Properties props = new Properties();
     private final static Level defaultLevel = Level.INFO;
 
     // The map of the registered listeners. The map value is the registration
     // count to allow for cases where the same listener is registered many times.
     private final Map<Object,Integer> listenerMap = new HashMap<>();

@@ -668,11 +675,11 @@
             }
             Logger logger = ref.get();
             if (logger == null) {
                 // Hashtable holds stale weak reference
                 // to a logger which has been GC-ed.
-                removeLogger(name);
+                ref.dispose();
             }
             return logger;
         }
 
         // This method is called before adding a logger to the

@@ -754,11 +761,11 @@
             if (ref != null) {
                 if (ref.get() == null) {
                     // It's possible that the Logger was GC'ed after a
                     // drainLoggerRefQueueBounded() call above so allow
                     // a new one to be registered.
-                    removeLogger(name);
+                    ref.dispose();
                 } else {
                     // We already have a registered logger with the given name.
                     return false;
                 }
             }

@@ -806,14 +813,12 @@
             // new LogNode is ready so tell the LoggerWeakRef about it
             ref.setNode(node);
             return true;
         }
 
-        // note: all calls to removeLogger are synchronized on LogManager's
-        // intrinsic lock
-        void removeLogger(String name) {
-            namedLoggers.remove(name);
+        synchronized void removeLoggerRef(String name, LoggerWeakRef ref) {
+            namedLoggers.remove(name, ref);
         }
 
         synchronized Enumeration<String> getLoggerNames() {
             // ensure that this context is properly initialized before
             // returning logger names.

@@ -991,28 +996,60 @@
     //
     final class LoggerWeakRef extends WeakReference<Logger> {
         private String                name;       // for namedLoggers cleanup
         private LogNode               node;       // for loggerRef cleanup
         private WeakReference<Logger> parentRef;  // for kids cleanup
+        private boolean disposed = false;         // avoid calling dispose twice
 
         LoggerWeakRef(Logger logger) {
             super(logger, loggerRefQueue);
 
             name = logger.getName();  // save for namedLoggers cleanup
         }
 
         // dispose of this LoggerWeakRef object
         void dispose() {
-            if (node != null) {
+            // Avoid calling dispose twice. When a Logger is gc'ed, its
+            // LoggerWeakRef will be enqueued.
+            // However, a new logger of the same name may be added (or looked
+            // up) before the queue is drained. When that happens, dispose()
+            // will be called by addLocalLogger() or findLogger().
+            // Later when the queue is drained, dispose() will be called again
+            // for the same LoggerWeakRef. Marking LoggerWeakRef as disposed
+            // avoids processing the data twice (even though the code should
+            // now be reentrant).
+            synchronized(this) {
+                // Note to maintainers:
+                // Be careful not to call any method that tries to acquire
+                // another lock from within this block - as this would surely
+                // lead to deadlocks, given that dispose() can be called by
+                // multiple threads, and from within different synchronized
+                // methods/blocks.
+                if (disposed) return;
+                disposed = true;
+            }
+
+            final LogNode n = node;
+            if (n != null) {
+                // n.loggerRef can only be safely modified from within
+                // a lock on LoggerContext. removeLoggerRef is already
+                // synchronized on LoggerContext so calling
+                // n.context.removeLoggerRef from within this lock is safe.
+                synchronized (n.context) {
                 // if we have a LogNode, then we were a named Logger
                 // so clear namedLoggers weak ref to us
-                node.context.removeLogger(name);
+                    n.context.removeLoggerRef(name, this);
                 name = null;  // clear our ref to the Logger's name
 
-                node.loggerRef = null;  // clear LogNode's weak ref to us
+                    // LogNode may have been reused - so only clear
+                    // LogNode.loggerRef if LogNode.loggerRef == this
+                    if (n.loggerRef == this) {
+                        n.loggerRef = null;  // clear LogNode's weak ref to us
+                    }
                 node = null;            // clear our ref to LogNode
             }
+             }
 
             if (parentRef != null) {
                 // this LoggerWeakRef has or had a parent Logger
                 Logger parent = parentRef.get();
                 if (parent != null) {

@@ -1060,11 +1097,11 @@
     //   - average: 0.57 ms
     //   - minimum: 0.02 ms
     //   - maximum: 10.9 ms
     //
     private final static int MAX_ITERATIONS = 400;
-    final synchronized void drainLoggerRefQueueBounded() {
+    final void drainLoggerRefQueueBounded() {
         for (int i = 0; i < MAX_ITERATIONS; i++) {
             if (loggerRefQueue == null) {
                 // haven't finished loading LogManager yet
                 break;
             }