You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by rg...@apache.org on 2019/07/29 02:08:00 UTC
[logging-log4j2] branch master updated: LOG4J2-2366 - Remove
references to LoggerContext when it is shutdown
This is an automated email from the ASF dual-hosted git repository.
rgoers pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
The following commit(s) were added to refs/heads/master by this push:
new 2b2b85d LOG4J2-2366 - Remove references to LoggerContext when it is shutdown
2b2b85d is described below
commit 2b2b85d8ed3005520f0d8e464c33c1007df43918
Author: Ralph Goers <rg...@apache.org>
AuthorDate: Sun Jul 28 19:07:47 2019 -0700
LOG4J2-2366 - Remove references to LoggerContext when it is shutdown
---
.../logging/log4j/spi/AbstractLoggerAdapter.java | 28 ++++++--
.../log4j/spi/LoggerContextShutdownAware.java | 26 ++++++++
.../log4j/spi/LoggerContextShutdownEnabled.java | 29 +++++++++
.../logging/log4j/spi/LoggerAdapterTest.java | 75 ++++++++++++++++++++--
.../apache/logging/log4j/core/LoggerContext.java | 33 +++++++++-
.../core/selector/ClassLoaderContextSelector.java | 21 ++++--
.../apache/logging/slf4j/LoggerContextTest.java | 44 +++++++++++++
.../apache/logging/slf4j/LoggerContextTest.java | 44 +++++++++++++
src/changes/changes.xml | 3 +
9 files changed, 285 insertions(+), 18 deletions(-)
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLoggerAdapter.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLoggerAdapter.java
index e86f990..17defb4 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLoggerAdapter.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLoggerAdapter.java
@@ -16,28 +16,29 @@
*/
package org.apache.logging.log4j.spi;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.util.LoaderUtil;
+
+import java.util.HashSet;
import java.util.Map;
-import java.util.WeakHashMap;
+import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.util.LoaderUtil;
-
/**
* Provides an abstract base class to use for implementing LoggerAdapter.
*
* @param <L> the Logger class to adapt
* @since 2.1
*/
-public abstract class AbstractLoggerAdapter<L> implements LoggerAdapter<L> {
+public abstract class AbstractLoggerAdapter<L> implements LoggerAdapter<L>, LoggerContextShutdownAware {
/**
* A map to store loggers for their given LoggerContexts.
*/
- protected final Map<LoggerContext, ConcurrentMap<String, L>> registry = new WeakHashMap<>();
+ protected final Map<LoggerContext, ConcurrentMap<String, L>> registry = new ConcurrentHashMap<>();
private final ReadWriteLock lock = new ReentrantReadWriteLock (true);
@@ -53,6 +54,11 @@ public abstract class AbstractLoggerAdapter<L> implements LoggerAdapter<L> {
return loggers.get(name);
}
+ @Override
+ public void contextShutdown(LoggerContext loggerContext) {
+ registry.remove(loggerContext);
+ }
+
/**
* Gets or creates the ConcurrentMap of named loggers for a given LoggerContext.
*
@@ -77,6 +83,9 @@ public abstract class AbstractLoggerAdapter<L> implements LoggerAdapter<L> {
if (loggers == null) {
loggers = new ConcurrentHashMap<> ();
registry.put (context, loggers);
+ if (context instanceof LoggerContextShutdownEnabled) {
+ ((LoggerContextShutdownEnabled) context).addShutdownListener(this);
+ }
}
return loggers;
} finally {
@@ -85,6 +94,13 @@ public abstract class AbstractLoggerAdapter<L> implements LoggerAdapter<L> {
}
/**
+ * For unit testing. Consider to be private.
+ */
+ public Set<LoggerContext> getLoggerContexts() {
+ return new HashSet<>(registry.keySet());
+ }
+
+ /**
* Creates a new named logger for a given {@link LoggerContext}.
*
* @param name the name of the logger to create
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerContextShutdownAware.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerContextShutdownAware.java
new file mode 100644
index 0000000..d31c583
--- /dev/null
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerContextShutdownAware.java
@@ -0,0 +1,26 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache license, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the license for the specific language governing permissions and
+ * limitations under the license.
+ */
+package org.apache.logging.log4j.spi;
+
+/**
+ * Interface allowing interested classes to know when a LoggerContext has shutdown - if the LoggerContext
+ * implementation provides a way to register listeners.
+ */
+public interface LoggerContextShutdownAware {
+
+ void contextShutdown(LoggerContext loggerContext);
+}
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerContextShutdownEnabled.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerContextShutdownEnabled.java
new file mode 100644
index 0000000..f3d59c1
--- /dev/null
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerContextShutdownEnabled.java
@@ -0,0 +1,29 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache license, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the license for the specific language governing permissions and
+ * limitations under the license.
+ */
+package org.apache.logging.log4j.spi;
+
+import java.util.List;
+
+/**
+ * LoggerContexts implementing this are able register LoggerContextShutdownAware classes.
+ */
+public interface LoggerContextShutdownEnabled {
+
+ void addShutdownListener(LoggerContextShutdownAware listener);
+
+ List<LoggerContextShutdownAware> getListeners();
+}
diff --git a/log4j-api/src/test/java/org/apache/logging/log4j/spi/LoggerAdapterTest.java b/log4j-api/src/test/java/org/apache/logging/log4j/spi/LoggerAdapterTest.java
index c2b1290..c65d748 100644
--- a/log4j-api/src/test/java/org/apache/logging/log4j/spi/LoggerAdapterTest.java
+++ b/log4j-api/src/test/java/org/apache/logging/log4j/spi/LoggerAdapterTest.java
@@ -16,12 +16,18 @@
*/
package org.apache.logging.log4j.spi;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.TestLogger;
+import org.apache.logging.log4j.TestLoggerContext;
+import org.apache.logging.log4j.TestLoggerContextFactory;
import org.apache.logging.log4j.simple.SimpleLoggerContext;
import org.junit.Test;
+import java.util.HashSet;
import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CountDownLatch;
-import java.util.logging.Logger;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertSame;
@@ -68,12 +74,6 @@ public class LoggerAdapterTest {
}
- private static class TestLogger extends Logger {
- public TestLogger() {
- super("test", null);
- }
- }
-
private static class TestLoggerAdapter extends AbstractLoggerAdapter<Logger> {
@Override
@@ -87,6 +87,67 @@ public class LoggerAdapterTest {
}
}
+ private static class TestLoggerAdapter2 extends AbstractLoggerAdapter<Logger> {
+
+ @Override
+ protected Logger newLogger(String name, LoggerContext context) {
+ return context.getLogger(name);
+ }
+
+ @Override
+ protected LoggerContext getContext() {
+ return null;
+ }
+
+ public LoggerContext getContext(String fqcn) {
+ for (LoggerContext lc : registry.keySet()) {
+ TestLoggerContext2 context = (TestLoggerContext2) lc;
+ if (fqcn.equals(context.getName())) {
+ return context;
+ }
+ }
+ LoggerContext lc = new TestLoggerContext2(fqcn, this);
+ registry.put(lc, new ConcurrentHashMap<>());
+ return lc;
+ }
+ }
+
+ private static class TestLoggerContext2 extends TestLoggerContext {
+ private final String name;
+ private final LoggerContextShutdownAware listener;
+
+ public TestLoggerContext2(String name, LoggerContextShutdownAware listener) {
+ this.name = name;
+ this.listener = listener;
+ }
+
+ public String getName() {
+ return name;
+ }
+
+ public void shutdown() {
+ listener.contextShutdown(this);
+ }
+ }
+
+ @Test
+ public void testCleanup() throws Exception {
+ final LoggerContextFactory factory = new TestLoggerContextFactory();
+ final TestLoggerAdapter2 adapter = new TestLoggerAdapter2();
+ for (int i = 0; i < 5; ++i) {
+ LoggerContext lc = adapter.getContext(Integer.toString(i));
+ lc.getLogger(Integer.toString(i));
+ }
+ assertEquals("Expected 5 LoggerContexts", 5, adapter.registry.size());
+ Set<LoggerContext> contexts = new HashSet<>(adapter.registry.keySet());
+ for (LoggerContext context : contexts) {
+ ((TestLoggerContext2) context).shutdown();
+ }
+ assertEquals("Expected 0 LoggerContexts", 0, adapter.registry.size());
+ }
+
+
+
/**
* Testing synchronization in the getLoggersInContext() method
*/
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java
index 9d12421..d84ab2a 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java
@@ -22,7 +22,10 @@ import java.beans.PropertyChangeEvent;
import java.beans.PropertyChangeListener;
import java.io.File;
import java.net.URI;
+import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
import java.util.Objects;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.CopyOnWriteArrayList;
@@ -48,6 +51,8 @@ import org.apache.logging.log4j.core.util.ShutdownCallbackRegistry;
import org.apache.logging.log4j.message.MessageFactory;
import org.apache.logging.log4j.spi.AbstractLogger;
import org.apache.logging.log4j.spi.LoggerContextFactory;
+import org.apache.logging.log4j.spi.LoggerContextShutdownAware;
+import org.apache.logging.log4j.spi.LoggerContextShutdownEnabled;
import org.apache.logging.log4j.spi.LoggerRegistry;
import org.apache.logging.log4j.spi.Terminable;
import org.apache.logging.log4j.spi.ThreadContextMapFactory;
@@ -60,7 +65,8 @@ import org.apache.logging.log4j.util.PropertiesUtil;
* filters, etc and will be atomically updated whenever a reconfigure occurs.
*/
public class LoggerContext extends AbstractLifeCycle
- implements org.apache.logging.log4j.spi.LoggerContext, AutoCloseable, Terminable, ConfigurationListener {
+ implements org.apache.logging.log4j.spi.LoggerContext, AutoCloseable, Terminable, ConfigurationListener,
+ LoggerContextShutdownEnabled {
static {
try {
@@ -80,6 +86,7 @@ public class LoggerContext extends AbstractLifeCycle
private final LoggerRegistry<Logger> loggerRegistry = new LoggerRegistry<>();
private final CopyOnWriteArrayList<PropertyChangeListener> propertyChangeListeners = new CopyOnWriteArrayList<>();
+ private volatile List<LoggerContextShutdownAware> listeners = null;
/**
* The Configuration is volatile to guarantee that initialization of the Configuration has completed before the
@@ -149,6 +156,21 @@ public class LoggerContext extends AbstractLifeCycle
}
}
+ public void addShutdownListener(LoggerContextShutdownAware listener) {
+ if (listeners == null) {
+ synchronized(this) {
+ if (listeners == null) {
+ listeners = Collections.synchronizedList(new ArrayList<>());
+ }
+ }
+ }
+ listeners.add(listener);
+ }
+
+ public List<LoggerContextShutdownAware> getListeners() {
+ return listeners;
+ }
+
/**
* Returns the current LoggerContext.
* <p>
@@ -356,6 +378,15 @@ public class LoggerContext extends AbstractLifeCycle
configLock.unlock();
this.setStopped();
}
+ if (listeners != null) {
+ for (LoggerContextShutdownAware listener : listeners) {
+ try {
+ listener.contextShutdown(this);
+ } catch (Exception ex) {
+ // Ignore the exception.
+ }
+ }
+ }
LOGGER.debug("Stopped LoggerContext[name={}, {}] with status {}", getName(), this, true);
return true;
}
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/ClassLoaderContextSelector.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/ClassLoaderContextSelector.java
index 3204e7e..b2c7282 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/ClassLoaderContextSelector.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/ClassLoaderContextSelector.java
@@ -30,6 +30,7 @@ import java.util.concurrent.atomic.AtomicReference;
import org.apache.logging.log4j.core.LoggerContext;
import org.apache.logging.log4j.core.impl.ContextAnchor;
+import org.apache.logging.log4j.spi.LoggerContextShutdownAware;
import org.apache.logging.log4j.status.StatusLogger;
import org.apache.logging.log4j.util.StackLocatorUtil;
@@ -44,7 +45,7 @@ import org.apache.logging.log4j.util.StackLocatorUtil;
*
* This ContextSelector should not be used with a Servlet Filter such as the Log4jServletFilter.
*/
-public class ClassLoaderContextSelector implements ContextSelector {
+public class ClassLoaderContextSelector implements ContextSelector, LoggerContextShutdownAware {
private static final AtomicReference<LoggerContext> DEFAULT_CONTEXT = new AtomicReference<>();
@@ -74,6 +75,13 @@ public class ClassLoaderContextSelector implements ContextSelector {
}
@Override
+ public void contextShutdown(org.apache.logging.log4j.spi.LoggerContext loggerContext) {
+ if (loggerContext instanceof LoggerContext) {
+ removeContext((LoggerContext) loggerContext);
+ }
+ }
+
+ @Override
public boolean hasContext(final String fqcn, final ClassLoader loader, final boolean currentContext) {
if (currentContext) {
return ContextAnchor.THREAD_CONTEXT.get() != null;
@@ -193,8 +201,11 @@ public class ClassLoaderContextSelector implements ContextSelector {
final AtomicReference<WeakReference<LoggerContext>> r = new AtomicReference<>();
r.set(new WeakReference<>(ctx));
CONTEXT_MAP.putIfAbsent(name, r);
- ctx = CONTEXT_MAP.get(name).get().get();
- return ctx;
+ LoggerContext newContext = CONTEXT_MAP.get(name).get().get();
+ if (newContext != null && newContext == ctx) {
+ newContext.addShutdownListener(this);
+ }
+ return newContext;
}
final WeakReference<LoggerContext> weakRef = ref.get();
LoggerContext ctx = weakRef.get();
@@ -210,7 +221,9 @@ public class ClassLoaderContextSelector implements ContextSelector {
return ctx;
}
ctx = createContext(name, configLocation);
- ref.compareAndSet(weakRef, new WeakReference<>(ctx));
+ if (ref.compareAndSet(weakRef, new WeakReference<>(ctx))) {
+ ctx.addShutdownListener(this);
+ }
return ctx;
}
diff --git a/log4j-slf4j-impl/src/test/java/org/apache/logging/slf4j/LoggerContextTest.java b/log4j-slf4j-impl/src/test/java/org/apache/logging/slf4j/LoggerContextTest.java
new file mode 100644
index 0000000..742fc9b
--- /dev/null
+++ b/log4j-slf4j-impl/src/test/java/org/apache/logging/slf4j/LoggerContextTest.java
@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache license, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the license for the specific language governing permissions and
+ * limitations under the license.
+ */
+package org.apache.logging.slf4j;
+
+import org.apache.logging.log4j.core.LifeCycle;
+import org.apache.logging.log4j.spi.LoggerContext;
+import org.junit.Test;
+import org.slf4j.impl.StaticLoggerBinder;
+
+import java.util.Set;
+
+import static org.junit.Assert.*;
+
+/**
+ * Tests cleanup of the LoggerContexts.
+ */
+public class LoggerContextTest {
+
+ @Test
+ public void testCleanup() throws Exception {
+ Log4jLoggerFactory factory = (Log4jLoggerFactory) StaticLoggerBinder.getSingleton().getLoggerFactory();
+ factory.getLogger("test");
+ Set<LoggerContext> set = factory.getLoggerContexts();
+ LoggerContext ctx1 = set.toArray(new LoggerContext[0])[0];
+ assertTrue("LoggerContext is not enabled for shutdown", ctx1 instanceof LifeCycle);
+ ((LifeCycle) ctx1).stop();
+ set = factory.getLoggerContexts();
+ assertTrue("Expected no LoggerContexts", set.isEmpty());
+ }
+}
diff --git a/log4j-slf4j18-impl/src/test/java/org/apache/logging/slf4j/LoggerContextTest.java b/log4j-slf4j18-impl/src/test/java/org/apache/logging/slf4j/LoggerContextTest.java
new file mode 100644
index 0000000..3c6403c
--- /dev/null
+++ b/log4j-slf4j18-impl/src/test/java/org/apache/logging/slf4j/LoggerContextTest.java
@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache license, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the license for the specific language governing permissions and
+ * limitations under the license.
+ */
+package org.apache.logging.slf4j;
+
+import org.apache.logging.log4j.core.LifeCycle;
+import org.apache.logging.log4j.spi.LoggerContext;
+import org.junit.Test;
+import org.slf4j.LoggerFactory;
+
+import java.util.Set;
+
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Tests cleanup of the LoggerContexts.
+ */
+public class LoggerContextTest {
+
+ @Test
+ public void testCleanup() throws Exception {
+ Log4jLoggerFactory factory = (Log4jLoggerFactory) LoggerFactory.getILoggerFactory();
+ factory.getLogger("test");
+ Set<LoggerContext> set = factory.getLoggerContexts();
+ LoggerContext ctx1 = set.toArray(new LoggerContext[0])[0];
+ assertTrue("LoggerContext is not enabled for shutdown", ctx1 instanceof LifeCycle);
+ ((LifeCycle) ctx1).stop();
+ set = factory.getLoggerContexts();
+ assertTrue("Expected no LoggerContexts", set.isEmpty());
+ }
+}
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 5b2444b..2df88f7 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -142,6 +142,9 @@
</action>
</release>
<release version="2.12.1" date="2019-MM-DD" description="GA Release 2.12.1">
+ <action issue="LOG4J2-2366" dev="rgoers" type="fix">
+ Remove references to LoggerContext when it is shutdown.
+ </action>
<action issue="LOG4J2-2556" dev="rgoers" type="update">
Make Log4j Core optional for Log4j 1.2 API.
</action>