You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by ck...@apache.org on 2019/06/17 00:22:21 UTC
[logging-log4j2] branch release-2.x updated (1e5ae05 -> dd87a3a)
This is an automated email from the ASF dual-hosted git repository.
ckozak pushed a change to branch release-2.x
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git.
from 1e5ae05 Clarify Default Rollover Strategy
new 1e726c2 LOG4J2-2631: RoutingAppender PurgePolicy implementations don't remove referenced appenders
new dd87a3a LOG4J2-2629: Avoid losing log events when the PurgePolicy races a log event
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../core/appender/routing/IdlePurgePolicy.java | 7 +-
.../log4j/core/appender/routing/PurgePolicy.java | 6 +-
.../core/appender/routing/RoutingAppender.java | 169 ++++++++++++++++++---
.../routing/RoutingAppenderWithPurgingTest.java | 23 ++-
.../src/test/resources/log4j-routing-purge.xml | 6 +-
src/changes/changes.xml | 9 ++
6 files changed, 184 insertions(+), 36 deletions(-)
[logging-log4j2] 01/02: LOG4J2-2631: RoutingAppender PurgePolicy
implementations don't remove referenced appenders
Posted by ck...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
ckozak pushed a commit to branch release-2.x
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 1e726c20e71e9aca7b5c1f4d568bd3fabd189076
Author: Carter Kozak <ck...@apache.org>
AuthorDate: Sun Jun 16 11:27:29 2019 -0400
LOG4J2-2631: RoutingAppender PurgePolicy implementations don't remove referenced appenders
Note that this makes a behavior change to RoutingAppender.getAppenders
where Routes based on appender references are no longer included.
PurgePolicy implementations should be entirely unaware of the
existance of reference based routes because those appenders may
be used elsewhere in the configuration.
---
.../core/appender/routing/IdlePurgePolicy.java | 2 +-
.../core/appender/routing/RoutingAppender.java | 60 +++++++++++++++-------
.../routing/RoutingAppenderWithPurgingTest.java | 23 +++++++--
.../src/test/resources/log4j-routing-purge.xml | 6 ++-
src/changes/changes.xml | 5 ++
5 files changed, 70 insertions(+), 26 deletions(-)
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/IdlePurgePolicy.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/IdlePurgePolicy.java
index 1babc38..65a0f45 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/IdlePurgePolicy.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/IdlePurgePolicy.java
@@ -74,7 +74,7 @@ public class IdlePurgePolicy extends AbstractLifeCycle implements PurgePolicy, R
final long createTime = System.currentTimeMillis() - timeToLive;
for (final Entry<String, Long> entry : appendersUsage.entrySet()) {
if (entry.getValue() < createTime) {
- LOGGER.debug("Removing appender " + entry.getKey());
+ LOGGER.debug("Removing appender {}", entry.getKey());
if (appendersUsage.remove(entry.getKey(), entry.getValue())) {
routingAppender.deleteAppender(entry.getKey());
}
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/RoutingAppender.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/RoutingAppender.java
index 4fd50d2..b70877c 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/RoutingAppender.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/RoutingAppender.java
@@ -134,7 +134,10 @@ public final class RoutingAppender extends AbstractAppender {
private final Routes routes;
private Route defaultRoute;
private final Configuration configuration;
- private final ConcurrentMap<String, AppenderControl> appenders = new ConcurrentHashMap<>();
+ private final ConcurrentMap<String, AppenderControl> createdAppenders = new ConcurrentHashMap<>();
+ private final Map<String, AppenderControl> createdAppendersUnmodifiableView
+ = Collections.unmodifiableMap(createdAppenders);
+ private final ConcurrentMap<String, AppenderControl> referencedAppenders = new ConcurrentHashMap<>();
private final RewritePolicy rewritePolicy;
private final PurgePolicy purgePolicy;
private final AbstractScript defaultRouteScript;
@@ -188,7 +191,7 @@ public final class RoutingAppender extends AbstractAppender {
final Appender appender = configuration.getAppender(route.getAppenderRef());
if (appender != null) {
final String key = route == defaultRoute ? DEFAULT_KEY : route.getKey();
- appenders.put(key, new AppenderControl(appender, null, null));
+ referencedAppenders.put(key, new AppenderControl(appender, null, null));
} else {
error("Appender " + route.getAppenderRef() + " cannot be located. Route ignored");
}
@@ -201,15 +204,13 @@ public final class RoutingAppender extends AbstractAppender {
public boolean stop(final long timeout, final TimeUnit timeUnit) {
setStopping();
super.stop(timeout, timeUnit, false);
- final Map<String, Appender> map = configuration.getAppenders();
- for (final Map.Entry<String, AppenderControl> entry : appenders.entrySet()) {
+ // Only stop appenders that were created by this RoutingAppender
+ for (final Map.Entry<String, AppenderControl> entry : createdAppenders.entrySet()) {
final Appender appender = entry.getValue().getAppender();
- if (!map.containsKey(appender.getName())) {
- if (appender instanceof LifeCycle2) {
- ((LifeCycle2) appender).stop(timeout, timeUnit);
- } else {
- appender.stop();
- }
+ if (appender instanceof LifeCycle2) {
+ ((LifeCycle2) appender).stop(timeout, timeUnit);
+ } else {
+ appender.stop();
}
}
setStopped();
@@ -228,13 +229,16 @@ public final class RoutingAppender extends AbstractAppender {
control.callAppender(event);
}
- if (purgePolicy != null) {
+ if (purgePolicy != null
+ // LOG4J2-2631: PurgePolicy implementations do not need to be aware of appenders that
+ // were not created by this RoutingAppender.
+ && !referencedAppenders.containsKey(key)) {
purgePolicy.update(key, event);
}
}
private synchronized AppenderControl getControl(final String key, final LogEvent event) {
- AppenderControl control = appenders.get(key);
+ AppenderControl control = getAppender(key);
if (control != null) {
return control;
}
@@ -247,7 +251,7 @@ public final class RoutingAppender extends AbstractAppender {
}
if (route == null) {
route = defaultRoute;
- control = appenders.get(DEFAULT_KEY);
+ control = getAppender(DEFAULT_KEY);
if (control != null) {
return control;
}
@@ -258,12 +262,20 @@ public final class RoutingAppender extends AbstractAppender {
return null;
}
control = new AppenderControl(app, null, null);
- appenders.put(key, control);
+ createdAppenders.put(key, control);
}
return control;
}
+ private AppenderControl getAppender(final String key) {
+ final AppenderControl result = referencedAppenders.get(key);
+ if (result == null) {
+ return createdAppenders.get(key);
+ }
+ return result;
+ }
+
private Appender createAppender(final Route route, final LogEvent event) {
final Node routeNode = route.getNode();
for (final Node node : routeNode.getChildren()) {
@@ -283,8 +295,12 @@ public final class RoutingAppender extends AbstractAppender {
return null;
}
+ /**
+ * Returns an unmodifiable view of the appenders created by this {@link RoutingAppender}.
+ * Note that this map does not contain appenders that are routed by reference.
+ */
public Map<String, AppenderControl> getAppenders() {
- return Collections.unmodifiableMap(appenders);
+ return createdAppendersUnmodifiableView;
}
/**
@@ -293,13 +309,19 @@ public final class RoutingAppender extends AbstractAppender {
* @param key The appender's key
*/
public void deleteAppender(final String key) {
- LOGGER.debug("Deleting route with " + key + " key ");
- final AppenderControl control = appenders.remove(key);
+ LOGGER.debug("Deleting route with {} key ", key);
+ // LOG4J2-2631: Only appenders created by this RoutingAppender are eligible for deletion.
+ final AppenderControl control = createdAppenders.remove(key);
if (null != control) {
- LOGGER.debug("Stopping route with " + key + " key");
+ LOGGER.debug("Stopping route with {} key", key);
control.getAppender().stop();
} else {
- LOGGER.debug("Route with " + key + " key already deleted");
+ if (referencedAppenders.containsKey(key)) {
+ LOGGER.debug("Route {} using an appender reference may not be removed because " +
+ "the appender may be used outside of the RoutingAppender", key);
+ } else {
+ LOGGER.debug("Route with {} key already deleted", key);
+ }
}
}
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/routing/RoutingAppenderWithPurgingTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/routing/RoutingAppenderWithPurgingTest.java
index 7dce22d..82a571f 100644
--- a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/routing/RoutingAppenderWithPurgingTest.java
+++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/routing/RoutingAppenderWithPurgingTest.java
@@ -17,11 +17,14 @@
package org.apache.logging.log4j.core.appender.routing;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import java.io.File;
+import java.util.HashSet;
import java.util.List;
+import java.util.Set;
import org.apache.logging.log4j.EventLogger;
import org.apache.logging.log4j.core.LogEvent;
@@ -84,19 +87,26 @@ public class RoutingAppenderWithPurgingTest {
EventLogger.logEvent(msg);
msg = new StructuredDataMessage("3", "This is a test 3", "Service");
EventLogger.logEvent(msg);
- final String[] files = {IDLE_LOG_FILE1, IDLE_LOG_FILE2, IDLE_LOG_FILE3, MANUAL_LOG_FILE1, MANUAL_LOG_FILE2, MANUAL_LOG_FILE3};
+ // '2' is a referenced list appender
+ final String[] files = {IDLE_LOG_FILE1, IDLE_LOG_FILE3, MANUAL_LOG_FILE1, MANUAL_LOG_FILE3};
assertFileExistance(files);
+ Set<String> expectedAppenderKeys = new HashSet<>(2);
+ expectedAppenderKeys.add("1");
+ expectedAppenderKeys.add("3");
+ assertEquals(expectedAppenderKeys, routingAppenderManual.getAppenders().keySet());
- assertEquals("Incorrect number of appenders with IdlePurgePolicy.", 3, routingAppenderIdle.getAppenders().size());
+ assertFalse(((ListAppender) loggerContextRule.getAppender("ReferencedList")).getEvents().isEmpty());
+
+ assertEquals("Incorrect number of appenders with IdlePurgePolicy.", 2, routingAppenderIdle.getAppenders().size());
assertEquals("Incorrect number of appenders with IdlePurgePolicy with HangingAppender.",
- 3, routingAppenderIdleWithHangingAppender.getAppenders().size());
- assertEquals("Incorrect number of appenders manual purge.", 3, routingAppenderManual.getAppenders().size());
+ 2, routingAppenderIdleWithHangingAppender.getAppenders().size());
+ assertEquals("Incorrect number of appenders manual purge.", 2, routingAppenderManual.getAppenders().size());
Thread.sleep(3000);
EventLogger.logEvent(msg);
assertEquals("Incorrect number of appenders with IdlePurgePolicy.", 1, routingAppenderIdle.getAppenders().size());
- assertEquals("Incorrect number of appenders with manual purge.", 3, routingAppenderManual.getAppenders().size());
+ assertEquals("Incorrect number of appenders with manual purge.", 2, routingAppenderManual.getAppenders().size());
routingAppenderManual.deleteAppender("1");
routingAppenderManual.deleteAppender("2");
@@ -105,6 +115,9 @@ public class RoutingAppenderWithPurgingTest {
assertEquals("Incorrect number of appenders with IdlePurgePolicy.", 1, routingAppenderIdle.getAppenders().size());
assertEquals("Incorrect number of appenders with manual purge.", 0, routingAppenderManual.getAppenders().size());
+ assertFalse("Reference based routes should not be stoppable",
+ loggerContextRule.getAppender("ReferencedList").isStopped());
+
msg = new StructuredDataMessage("5", "This is a test 5", "Service");
EventLogger.logEvent(msg);
diff --git a/log4j-core/src/test/resources/log4j-routing-purge.xml b/log4j-core/src/test/resources/log4j-routing-purge.xml
index 0efc296..d1de288 100644
--- a/log4j-core/src/test/resources/log4j-routing-purge.xml
+++ b/log4j-core/src/test/resources/log4j-routing-purge.xml
@@ -30,6 +30,7 @@
<List name="List">
<ThresholdFilter level="debug"/>
</List>
+ <List name="ReferencedList"/>
<Routing name="RoutingPurgeIdle">
<Routes pattern="$${sd:id}">
<Route>
@@ -39,15 +40,17 @@
</PatternLayout>
</File>
</Route>
+ <Route ref="ReferencedList" key="2"/>
</Routes>
<IdlePurgePolicy timeToLive="2" timeUnit="seconds" />
</Routing>
-
+
<Routing name="RoutingPurgeIdleWithHangingAppender">
<Routes pattern="$${sd:id}">
<Route>
<Hanging name="Routing-${sd:id}" shutdownDelay="10000"/>
</Route>
+ <Route ref="ReferencedList" key="2"/>
</Routes>
<IdlePurgePolicy timeToLive="2" timeUnit="seconds" />
</Routing>
@@ -61,6 +64,7 @@
</PatternLayout>
</File>
</Route>
+ <Route ref="ReferencedList" key="2"/>
</Routes>
</Routing>
</Appenders>
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index fa9b13f..e058176 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -91,6 +91,11 @@
<action issue="LOG4J2-2619" dev="ggregory" type="update">
Update Jackson from 2.9.8 to 2.9.9.
</action>
+ <action issue="LOG4J2-2631" dev="ckozak" type="fix">
+ RoutingAppender PurgePolicy implementations no longer stop appenders referenced from the logger configuration,
+ only those that have been created by the RoutingAppender. Note that RoutingAppender.getAppenders no longer
+ includes entries for referenced appenders, only those which it has created.
+ </action>
</release>
<release version="2.11.2" date="2019-02-04" description="GA Release 2.11.2">
<action issue="LOG4J2-2500" dev="rgoers" type="fix">
[logging-log4j2] 02/02: LOG4J2-2629: Avoid losing log events when
the PurgePolicy races a log event
Posted by ck...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
ckozak pushed a commit to branch release-2.x
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit dd87a3a8993c992bbd06f3b18eebb1e0cd9c6793
Author: Carter Kozak <ck...@apache.org>
AuthorDate: Sun Jun 16 16:58:02 2019 -0400
LOG4J2-2629: Avoid losing log events when the PurgePolicy races a log event
---
.../core/appender/routing/IdlePurgePolicy.java | 7 +-
.../log4j/core/appender/routing/PurgePolicy.java | 6 +-
.../core/appender/routing/RoutingAppender.java | 127 ++++++++++++++++++---
src/changes/changes.xml | 4 +
4 files changed, 124 insertions(+), 20 deletions(-)
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/IdlePurgePolicy.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/IdlePurgePolicy.java
index 65a0f45..0f75ec3 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/IdlePurgePolicy.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/IdlePurgePolicy.java
@@ -73,9 +73,10 @@ public class IdlePurgePolicy extends AbstractLifeCycle implements PurgePolicy, R
public void purge() {
final long createTime = System.currentTimeMillis() - timeToLive;
for (final Entry<String, Long> entry : appendersUsage.entrySet()) {
- if (entry.getValue() < createTime) {
- LOGGER.debug("Removing appender {}", entry.getKey());
- if (appendersUsage.remove(entry.getKey(), entry.getValue())) {
+ long entryValue = entry.getValue();
+ if (entryValue < createTime) {
+ if (appendersUsage.remove(entry.getKey(), entryValue)) {
+ LOGGER.debug("Removing appender {}", entry.getKey());
routingAppender.deleteAppender(entry.getKey());
}
}
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/PurgePolicy.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/PurgePolicy.java
index f780b15..98af154 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/PurgePolicy.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/PurgePolicy.java
@@ -24,13 +24,13 @@ import org.apache.logging.log4j.core.LogEvent;
public interface PurgePolicy {
/**
- * Activates purging appenders
+ * Activates purging appenders. Note that {@link PurgePolicy} implementations are responsible for invoking
+ * this method themselves.
*/
void purge();
/**
- *
- * @param routed appender key
+ * @param key routed appender key
* @param event
*/
void update(String key, LogEvent event);
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/RoutingAppender.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/RoutingAppender.java
index b70877c..0ce2029 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/RoutingAppender.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/routing/RoutingAppender.java
@@ -22,6 +22,7 @@ import java.util.Objects;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
import javax.script.Bindings;
@@ -134,10 +135,10 @@ public final class RoutingAppender extends AbstractAppender {
private final Routes routes;
private Route defaultRoute;
private final Configuration configuration;
- private final ConcurrentMap<String, AppenderControl> createdAppenders = new ConcurrentHashMap<>();
+ private final ConcurrentMap<String, CreatedRouteAppenderControl> createdAppenders = new ConcurrentHashMap<>();
private final Map<String, AppenderControl> createdAppendersUnmodifiableView
= Collections.unmodifiableMap(createdAppenders);
- private final ConcurrentMap<String, AppenderControl> referencedAppenders = new ConcurrentHashMap<>();
+ private final ConcurrentMap<String, RouteAppenderControl> referencedAppenders = new ConcurrentHashMap<>();
private final RewritePolicy rewritePolicy;
private final PurgePolicy purgePolicy;
private final AbstractScript defaultRouteScript;
@@ -191,7 +192,7 @@ public final class RoutingAppender extends AbstractAppender {
final Appender appender = configuration.getAppender(route.getAppenderRef());
if (appender != null) {
final String key = route == defaultRoute ? DEFAULT_KEY : route.getKey();
- referencedAppenders.put(key, new AppenderControl(appender, null, null));
+ referencedAppenders.put(key, new ReferencedRouteAppenderControl(appender));
} else {
error("Appender " + route.getAppenderRef() + " cannot be located. Route ignored");
}
@@ -205,7 +206,7 @@ public final class RoutingAppender extends AbstractAppender {
setStopping();
super.stop(timeout, timeUnit, false);
// Only stop appenders that were created by this RoutingAppender
- for (final Map.Entry<String, AppenderControl> entry : createdAppenders.entrySet()) {
+ for (final Map.Entry<String, CreatedRouteAppenderControl> entry : createdAppenders.entrySet()) {
final Appender appender = entry.getValue().getAppender();
if (appender instanceof LifeCycle2) {
((LifeCycle2) appender).stop(timeout, timeUnit);
@@ -224,11 +225,18 @@ public final class RoutingAppender extends AbstractAppender {
}
final String pattern = routes.getPattern(event, scriptStaticVariables);
final String key = pattern != null ? configuration.getStrSubstitutor().replace(event, pattern) : defaultRoute.getKey();
- final AppenderControl control = getControl(key, event);
+ final RouteAppenderControl control = getControl(key, event);
if (control != null) {
- control.callAppender(event);
+ try {
+ control.callAppender(event);
+ } finally {
+ control.release();
+ }
}
+ updatePurgePolicy(key, event);
+ }
+ private void updatePurgePolicy(final String key, final LogEvent event) {
if (purgePolicy != null
// LOG4J2-2631: PurgePolicy implementations do not need to be aware of appenders that
// were not created by this RoutingAppender.
@@ -237,9 +245,10 @@ public final class RoutingAppender extends AbstractAppender {
}
}
- private synchronized AppenderControl getControl(final String key, final LogEvent event) {
- AppenderControl control = getAppender(key);
+ private synchronized RouteAppenderControl getControl(final String key, final LogEvent event) {
+ RouteAppenderControl control = getAppender(key);
if (control != null) {
+ control.checkout();
return control;
}
Route route = null;
@@ -253,6 +262,7 @@ public final class RoutingAppender extends AbstractAppender {
route = defaultRoute;
control = getAppender(DEFAULT_KEY);
if (control != null) {
+ control.checkout();
return control;
}
}
@@ -261,15 +271,19 @@ public final class RoutingAppender extends AbstractAppender {
if (app == null) {
return null;
}
- control = new AppenderControl(app, null, null);
- createdAppenders.put(key, control);
+ CreatedRouteAppenderControl created = new CreatedRouteAppenderControl(app);
+ control = created;
+ createdAppenders.put(key, created);
}
+ if (control != null) {
+ control.checkout();
+ }
return control;
}
- private AppenderControl getAppender(final String key) {
- final AppenderControl result = referencedAppenders.get(key);
+ private RouteAppenderControl getAppender(final String key) {
+ final RouteAppenderControl result = referencedAppenders.get(key);
if (result == null) {
return createdAppenders.get(key);
}
@@ -311,10 +325,17 @@ public final class RoutingAppender extends AbstractAppender {
public void deleteAppender(final String key) {
LOGGER.debug("Deleting route with {} key ", key);
// LOG4J2-2631: Only appenders created by this RoutingAppender are eligible for deletion.
- final AppenderControl control = createdAppenders.remove(key);
+ final CreatedRouteAppenderControl control = createdAppenders.remove(key);
if (null != control) {
LOGGER.debug("Stopping route with {} key", key);
- control.getAppender().stop();
+ // Synchronize with getControl to avoid triggering stopAppender before RouteAppenderControl.checkout
+ // can be invoked.
+ synchronized (this) {
+ control.pendingDeletion = true;
+ }
+ // Don't attempt to stop the appender in a synchronized block, since it may block flushing events
+ // to disk.
+ control.tryStopAppender();
} else {
if (referencedAppenders.containsKey(key)) {
LOGGER.debug("Route {} using an appender reference may not be removed because " +
@@ -386,4 +407,82 @@ public final class RoutingAppender extends AbstractAppender {
public ConcurrentMap<Object, Object> getScriptStaticVariables() {
return scriptStaticVariables;
}
+
+ /**
+ * LOG4J2-2629: PurgePolicy implementations can invoke {@link #deleteAppender(String)} after we have looked up
+ * an instance of a target appender but before events are appended, which could result in events not being
+ * recorded to any appender.
+ * This extension of {@link AppenderControl} allows to to mark usage of an appender, allowing deferral of
+ * {@link Appender#stop()} until events have successfully been recorded.
+ * Alternative approaches considered:
+ * - More aggressive synchronization: Appenders may do expensive I/O that shouldn't block routing.
+ * - Move the 'updatePurgePolicy' invocation before appenders are called: Unfortunately this approach doesn't work
+ * if we consider an ImmediatePurgePolicy (or IdlePurgePolicy with a very small timeout) because it may attempt
+ * to remove an appender that doesn't exist yet. It's counterintuitive to get an event that a route has been
+ * used at a point when we expect the route doesn't exist in {@link #getAppenders()}.
+ */
+ private static abstract class RouteAppenderControl extends AppenderControl {
+
+ RouteAppenderControl(Appender appender) {
+ super(appender, null, null);
+ }
+
+ abstract void checkout();
+
+ abstract void release();
+ }
+
+ private static final class CreatedRouteAppenderControl extends RouteAppenderControl {
+
+ private volatile boolean pendingDeletion = false;
+ private final AtomicInteger depth = new AtomicInteger();
+
+ CreatedRouteAppenderControl(Appender appender) {
+ super(appender);
+ }
+
+ @Override
+ void checkout() {
+ if (pendingDeletion) {
+ LOGGER.warn("CreatedRouteAppenderControl.checkout invoked on a " +
+ "RouteAppenderControl that is pending deletion");
+ }
+ depth.incrementAndGet();
+ }
+
+ @Override
+ void release() {
+ depth.decrementAndGet();
+ tryStopAppender();
+ }
+
+ void tryStopAppender() {
+ if (pendingDeletion
+ // Only attempt to stop the appender if we can CaS the depth away from zero, otherwise either
+ // 1. Another invocation of tryStopAppender has succeeded, or
+ // 2. Events are being appended, and will trigger stop when they complete
+ && depth.compareAndSet(0, -100_000)) {
+ Appender appender = getAppender();
+ LOGGER.debug("Stopping appender {}", appender);
+ appender.stop();
+ }
+ }
+ }
+
+ private static final class ReferencedRouteAppenderControl extends RouteAppenderControl {
+
+ ReferencedRouteAppenderControl(Appender appender) {
+ super(appender);
+ }
+
+ @Override
+ void checkout() {
+ // nop
+ }
+
+ @Override
+ void release() {
+ // nop
+ }
+ }
}
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index e058176..f45e2fa 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -96,6 +96,10 @@
only those that have been created by the RoutingAppender. Note that RoutingAppender.getAppenders no longer
includes entries for referenced appenders, only those which it has created.
</action>
+ <action issue="LOG4J2-2629" dev="ckozak" type="fix">
+ Fix a race allowing events not to be recorded when a RoutingAppender purge policy attempts to delete an idle
+ appender at exactly the same time as a new event is recorded.
+ </action>
</release>
<release version="2.11.2" date="2019-02-04" description="GA Release 2.11.2">
<action issue="LOG4J2-2500" dev="rgoers" type="fix">