You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by ma...@apache.org on 2014/03/23 18:12:35 UTC
svn commit: r1580542 -
/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java
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);
}
}