logging-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mattsic...@apache.org
Subject svn commit: r1580542 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java
Date Sun, 23 Mar 2014 17:12:35 GMT
Author: mattsicker
Date: Sun Mar 23 17:12:35 2014
New Revision: 1580542

URL: http://svn.apache.org/r1580542
Log:
Fix up circular references in LoggerContext.

  - This doesn't fix LOG4J2-570, but it does help in related memory 
  leaks.
  - Using a PhantomReference to hold the shutdown thread (which can 
  be enqueue()'d upon stopping).
  - No need to call Runtime.getRuntime().removeShutdownHook() when 
  it's already running!
  - Converted shutdown hook to a static inner class implementing 
  Runnable (as per the recommended practice of not extending Thread).
  - Also cleaned up some logging statements here.

Modified:
    logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java

Modified: logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java
URL: http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java?rev=1580542&r1=1580541&r2=1580542&view=diff
==============================================================================
--- logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java
(original)
+++ logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java
Sun Mar 23 17:12:35 2014
@@ -19,6 +19,9 @@ package org.apache.logging.log4j.core;
 import java.beans.PropertyChangeEvent;
 import java.beans.PropertyChangeListener;
 import java.io.File;
+import java.lang.ref.PhantomReference;
+import java.lang.ref.Reference;
+import java.lang.ref.ReferenceQueue;
 import java.net.URI;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
@@ -27,6 +30,8 @@ import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
 
 import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Marker;
+import org.apache.logging.log4j.MarkerManager;
 import org.apache.logging.log4j.core.config.Configuration;
 import org.apache.logging.log4j.core.config.ConfigurationFactory;
 import org.apache.logging.log4j.core.config.ConfigurationListener;
@@ -51,6 +56,7 @@ public class LoggerContext implements or
 
     public static final String PROPERTY_CONFIG = "config";
     private static final StatusLogger LOGGER = StatusLogger.getLogger();
+    private static final Marker SHUTDOWN_HOOK = MarkerManager.getMarker("SHUTDOWN HOOK");
     private static final Configuration NULL_CONFIGURATION = new NullConfiguration();
 
     private final ConcurrentMap<String, Logger> loggers = new ConcurrentHashMap<String,
Logger>();
@@ -65,7 +71,12 @@ public class LoggerContext implements or
     private final String name;
     private URI configLocation;
 
-    private ShutdownThread shutdownThread = null;
+    /**
+     * Once a shutdown hook thread executes, we can't remove it from the runtime (throws
an exception). Therefore,
+     * it's really pointless to keep a strongly accessible reference to said thread. Thus,
please use a
+     * PhantomReference here to prevent possible cyclic memory references.
+     */
+    private Reference<Thread> shutdownThread;
 
     /**
      * Status of the LoggerContext.
@@ -147,18 +158,7 @@ public class LoggerContext implements or
                 if (status == Status.INITIALIZED || status == Status.STOPPED) {
                     status = Status.STARTING;
                     reconfigure();
-                    if (config.isShutdownHookEnabled()) {
-                        shutdownThread = new ShutdownThread(this);
-                        try {
-                            Runtime.getRuntime().addShutdownHook(shutdownThread);
-                        } catch (final IllegalStateException ise) {
-                            LOGGER.warn("Unable to register shutdown hook due to JVM state");
-                            shutdownThread = null;
-                        } catch (final SecurityException se) {
-                            LOGGER.warn("Unable to register shutdown hook due to security
restrictions");
-                            shutdownThread = null;
-                        }
-                    }
+                    registerShutdownHookIfEnabled();
                     status = Status.STARTED;
                 }
             } finally {
@@ -174,17 +174,8 @@ public class LoggerContext implements or
     public void start(final Configuration config) {
         if (configLock.tryLock()) {
             try {
-                if ((status == Status.INITIALIZED || status == Status.STOPPED) &&
config.isShutdownHookEnabled() ) {
-                    shutdownThread = new ShutdownThread(this);
-                    try {
-                        Runtime.getRuntime().addShutdownHook(shutdownThread);
-                    } catch (final IllegalStateException ise) {
-                        LOGGER.warn("Unable to register shutdown hook due to JVM state");
-                        shutdownThread = null;
-                    } catch (final SecurityException se) {
-                        LOGGER.warn("Unable to register shutdown hook due to security restrictions");
-                        shutdownThread = null;
-                    }
+                if (status == Status.INITIALIZED || status == Status.STOPPED) {
+                    registerShutdownHookIfEnabled();
                     status = Status.STARTED;
                 }
             } finally {
@@ -194,6 +185,21 @@ public class LoggerContext implements or
         setConfiguration(config);
     }
 
+    private void registerShutdownHookIfEnabled() {
+        if (config.isShutdownHookEnabled()) {
+            LOGGER.debug(SHUTDOWN_HOOK, "Shutdown hook enabled. Registering a new one.");
+            final Thread thread = new Thread(new ShutdownThread(this), "log4j-shutdown");
+            shutdownThread = new PhantomReference<Thread>(thread, new ReferenceQueue<Thread>());
+            try {
+                Runtime.getRuntime().addShutdownHook(thread);
+            } catch (final IllegalStateException ise) {
+                LOGGER.warn(SHUTDOWN_HOOK, "Unable to register shutdown hook due to JVM state");
+            } catch (final SecurityException se) {
+                LOGGER.warn(SHUTDOWN_HOOK, "Unable to register shutdown hook due to security
restrictions");
+            }
+        }
+    }
+
     @Override
     public void stop() {
         configLock.lock();
@@ -202,10 +208,7 @@ public class LoggerContext implements or
                 return;
             }
             status = Status.STOPPING;
-            if (shutdownThread != null) {
-                Runtime.getRuntime().removeShutdownHook(shutdownThread);
-                shutdownThread = null;
-            }
+            unregisterShutdownHook();
             final Configuration prev = config;
             config = NULL_CONFIGURATION;
             updateLoggers();
@@ -215,12 +218,19 @@ public class LoggerContext implements or
             status = Status.STOPPED;
         } finally {
             configLock.unlock();
-            
-            // in finally: unregister MBeans even if an exception occurred while stopping

+
+            // in finally: unregister MBeans even if an exception occurred while stopping
             Server.unregisterLoggerContext(getName()); // LOG4J2-406, LOG4J2-500
         }
     }
 
+    private void unregisterShutdownHook() {
+        if (shutdownThread != null) {
+            LOGGER.debug(SHUTDOWN_HOOK, "Enqueue shutdown hook for garbage collection.");
+            shutdownThread.enqueue();
+        }
+    }
+
     /**
      * Gets the name.
      *
@@ -346,11 +356,7 @@ public class LoggerContext implements or
             prev.stop();
         }
 
-        // notify listeners
-        final PropertyChangeEvent evt = new PropertyChangeEvent(this, PROPERTY_CONFIG, prev,
config);
-        for (final PropertyChangeListener listener : propertyChangeListeners) {
-            listener.propertyChange(evt);
-        }
+        firePropertyChangeEvent(new PropertyChangeEvent(this, PROPERTY_CONFIG, prev, config));
 
         try {
             Server.reregisterMBeansAfterReconfigure();
@@ -360,6 +366,12 @@ public class LoggerContext implements or
         return prev;
     }
 
+    private void firePropertyChangeEvent(final PropertyChangeEvent event) {
+        for (final PropertyChangeListener listener : propertyChangeListeners) {
+            listener.propertyChange(event);
+        }
+    }
+
     public void addPropertyChangeListener(final PropertyChangeListener listener) {
         propertyChangeListeners.add(Assert.isNotNull(listener, "listener"));
     }
@@ -381,7 +393,7 @@ public class LoggerContext implements or
      * Reconfigure the context.
      */
     public synchronized void reconfigure() {
-        LOGGER.debug("Reconfiguration started for context " + name);
+        LOGGER.debug("Reconfiguration started for context {}", name);
         final Configuration instance = ConfigurationFactory.getInstance().getConfiguration(name,
configLocation);
         setConfiguration(instance);
         /*
@@ -417,7 +429,7 @@ public class LoggerContext implements or
      */
     @Override
     public synchronized void onChange(final Reconfigurable reconfigurable) {
-        LOGGER.debug("Reconfiguration started for context " + name);
+        LOGGER.debug("Reconfiguration started for context {}", name);
         final Configuration config = reconfigurable.reconfigure();
         if (config != null) {
             setConfiguration(config);
@@ -432,7 +444,7 @@ public class LoggerContext implements or
         return new Logger(ctx, name, messageFactory);
     }
 
-    private class ShutdownThread extends Thread {
+    private static class ShutdownThread implements Runnable {
 
         private final LoggerContext context;
 
@@ -442,8 +454,9 @@ public class LoggerContext implements or
 
         @Override
         public void run() {
-            context.shutdownThread = null;
+            LOGGER.debug("Stopping LoggerContext [{}]", context);
             context.stop();
+            LOGGER.debug("LoggerContext [{}] stopped.", context);
         }
     }
 



Mime
View raw message