You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by pk...@apache.org on 2024/04/30 09:53:19 UTC
(logging-log4j2) branch feature/2.x/enable-thread-local updated (8ec8f6cd41 -> a86abbd6c4)
This is an automated email from the ASF dual-hosted git repository.
pkarwasz pushed a change to branch feature/2.x/enable-thread-local
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
discard 8ec8f6cd41 Enforce `log4j2.enableThreadlocals` system property
new a86abbd6c4 Enforce `log4j2.enableThreadlocals` system property
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (8ec8f6cd41)
\
N -- N -- N refs/heads/feature/2.x/enable-thread-local (a86abbd6c4)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 1 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:
.../src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
(logging-log4j2) 01/01: Enforce `log4j2.enableThreadlocals` system property
Posted by pk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
pkarwasz pushed a commit to branch feature/2.x/enable-thread-local
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit a86abbd6c499e595a938a3b5a4a56555d9bba57e
Author: Piotr P. Karwasz <pi...@karwasz.org>
AuthorDate: Mon Apr 29 11:52:14 2024 +0200
Enforce `log4j2.enableThreadlocals` system property
If the `log4j2.enableThreadlocals` property is set to `false` Log4j
should clear all `ThreadLocal`s from the calling thread before execution
finishes.
A similar rule is also applied to `ThreadContext`: assuming the user takes
care of leaving an empty map and stack at the end of the execution, all
the `ThreadLocal`s should be cleared.
---
log4j-api-test/pom.xml | 37 +++++++++
.../apache/logging/log4j/test/ThreadLocalUtil.java | 57 ++++++++++++++
.../apache/logging/log4j/test/package-info.java | 2 +-
.../map/StringArrayThreadContextMapTest.java | 29 ++++++-
.../log4j/spi/DefaultThreadContextStackTest.java | 27 +++++++
.../logging/log4j/spi/ThreadContextMapTest.java | 36 +++++++++
.../internal/map/StringArrayThreadContextMap.java | 11 ++-
.../apache/logging/log4j/spi/AbstractLogger.java | 29 ++++---
.../CopyOnWriteSortedArrayThreadContextMap.java | 16 +++-
.../logging/log4j/spi/DefaultThreadContextMap.java | 12 ++-
.../log4j/spi/DefaultThreadContextStack.java | 26 +++++--
.../org/apache/logging/log4j/util/Strings.java | 8 +-
.../java/org/apache/logging/log4j/util/Timer.java | 19 ++---
.../log4j/core/async/AsyncLoggerConfig.java | 36 ++++-----
.../logging/log4j/core/config/AppenderControl.java | 2 +-
.../log4j/core/filter/StructuredDataFilter.java | 20 +++--
.../org/apache/logging/log4j/jul/LogManager.java | 5 ++
log4j-slf4j2-impl/pom.xml | 37 +++++++++
.../org/apache/logging/slf4j/Log4jMDCAdapter.java | 90 +++++++++++++++++-----
.../apache/logging/slf4j/Log4jMDCAdapterTest.java | 20 ++++-
20 files changed, 425 insertions(+), 94 deletions(-)
diff --git a/log4j-api-test/pom.xml b/log4j-api-test/pom.xml
index 40d4dc2525..1673c66928 100644
--- a/log4j-api-test/pom.xml
+++ b/log4j-api-test/pom.xml
@@ -175,4 +175,41 @@
</plugins>
</build>
+
+ <profiles>
+
+ <!-- Fixes incompatible with Java 8 -->
+ <profile>
+
+ <id>java8-incompat-fixes</id>
+
+ <!-- CI uses Java 8 for running tests.
+ Hence, we assume CI=Java8 and apply our changes elsewhere.
+
+ One might think why not activate using `<jdk>[16,)` instead?
+ This doesn't work, since the match is not against "the JDK running tests", but "the JDK running Maven".
+ These two JDKs can differ due to Maven Toolchains.
+ See `java8-tests` profile in `/pom.xml` for details. -->
+ <activation>
+ <property>
+ <name>!env.CI</name>
+ </property>
+ </activation>
+
+ <!-- Illegal access is disabled by default in Java 16 due to JEP-396.
+ We are relaxing it for tests. -->
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <configuration>
+ <argLine>--add-opens java.base/java.lang=ALL-UNNAMED</argLine>
+ </configuration>
+ </plugin>
+ </plugins>
+ </build>
+
+ </profile>
+ </profiles>
</project>
diff --git a/log4j-api-test/src/main/java/org/apache/logging/log4j/test/ThreadLocalUtil.java b/log4j-api-test/src/main/java/org/apache/logging/log4j/test/ThreadLocalUtil.java
new file mode 100644
index 0000000000..a316ed379f
--- /dev/null
+++ b/log4j-api-test/src/main/java/org/apache/logging/log4j/test/ThreadLocalUtil.java
@@ -0,0 +1,57 @@
+/*
+ * 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.test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
+
+/**
+ * Utility class to test {@link ThreadLocal} fields.
+ */
+public final class ThreadLocalUtil {
+
+ private ThreadLocalUtil() {}
+
+ /**
+ * Returns the number of {@link ThreadLocal} objects that have a value for the current thread.
+ * <p>
+ * <strong>WARNING:</strong> The {@link ThreadLocal#get()} method is <strong>not</strong> side-effect free.
+ * If the thread local is not initialized, it initializes it with a value of {@code null}.
+ * </p>
+ * @return The number of {@code ThreadLocal} objects with a value in the current thread.
+ */
+ public static int getThreadLocalCount() {
+ return assertDoesNotThrow(() -> {
+ final Field threadLocalsField = Thread.class.getDeclaredField("threadLocals");
+ threadLocalsField.setAccessible(true);
+ final Object threadLocalMap = threadLocalsField.get(Thread.currentThread());
+ final Method expungeStaleEntries = threadLocalMap.getClass().getDeclaredMethod("expungeStaleEntries");
+ expungeStaleEntries.setAccessible(true);
+ expungeStaleEntries.invoke(threadLocalMap);
+ final Field sizeField = threadLocalMap.getClass().getDeclaredField("size");
+ sizeField.setAccessible(true);
+ return (int) sizeField.get(threadLocalMap);
+ });
+ }
+
+ public static void assertThreadLocalCount(final int expexted) {
+ assertThat(getThreadLocalCount()).as("Count ThreadLocals with value").isEqualTo(expexted);
+ }
+}
diff --git a/log4j-api-test/src/main/java/org/apache/logging/log4j/test/package-info.java b/log4j-api-test/src/main/java/org/apache/logging/log4j/test/package-info.java
index 5a0ebc4510..9f17256730 100644
--- a/log4j-api-test/src/main/java/org/apache/logging/log4j/test/package-info.java
+++ b/log4j-api-test/src/main/java/org/apache/logging/log4j/test/package-info.java
@@ -15,7 +15,7 @@
* limitations under the license.
*/
@Export
-@Version("2.21.1")
+@Version("2.24.0")
package org.apache.logging.log4j.test;
import org.osgi.annotation.bundle.Export;
diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/internal/map/StringArrayThreadContextMapTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/internal/map/StringArrayThreadContextMapTest.java
index 2f845d8bf7..3e6a94aa5b 100644
--- a/log4j-api-test/src/test/java/org/apache/logging/log4j/internal/map/StringArrayThreadContextMapTest.java
+++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/internal/map/StringArrayThreadContextMapTest.java
@@ -16,16 +16,19 @@
*/
package org.apache.logging.log4j.internal.map;
+import static org.apache.logging.log4j.test.ThreadLocalUtil.assertThreadLocalCount;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
+import org.apache.logging.log4j.test.ThreadLocalUtil;
import org.apache.logging.log4j.test.junit.UsingThreadContextMap;
import org.apache.logging.log4j.util.TriConsumer;
import org.junit.jupiter.api.Test;
@@ -34,7 +37,9 @@ import org.junit.jupiter.api.Test;
* Tests the {@code StringArrayThreadContextMap} class.
*/
@UsingThreadContextMap
-public class StringArrayThreadContextMapTest {
+class StringArrayThreadContextMapTest {
+
+ private static final String KEY = "StringArrayThreadContextMapTest";
@Test
public void testEqualsVsSameKind() {
@@ -284,4 +289,26 @@ public class StringArrayThreadContextMapTest {
map.forEach(triConsumer, iterationResultMap);
assertTrue(iterationResultMap.isEmpty());
}
+
+ @Test
+ void threadLocalsRemovedWhenMapEmpty() {
+ final StringArrayThreadContextMap contextMap = new StringArrayThreadContextMap();
+ final int threadLocalCount = ThreadLocalUtil.getThreadLocalCount();
+
+ contextMap.put(KEY, "threadLocalsRemovedWhenMapEmpty");
+ assertThreadLocalCount(threadLocalCount + 1);
+ contextMap.remove(KEY);
+ assertThreadLocalCount(threadLocalCount);
+
+ contextMap.put("key1", "value1");
+ contextMap.put("key2", "value2");
+ assertThreadLocalCount(threadLocalCount + 1);
+ contextMap.removeAll(Arrays.asList("key1", "key2"));
+ assertThreadLocalCount(threadLocalCount);
+
+ contextMap.put(KEY, "threadLocalsRemovedWhenMapEmpty");
+ assertThreadLocalCount(threadLocalCount + 1);
+ contextMap.clear();
+ assertThreadLocalCount(threadLocalCount);
+ }
}
diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/spi/DefaultThreadContextStackTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/spi/DefaultThreadContextStackTest.java
index 4778ec63c1..e94ed9914e 100644
--- a/log4j-api-test/src/test/java/org/apache/logging/log4j/spi/DefaultThreadContextStackTest.java
+++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/spi/DefaultThreadContextStackTest.java
@@ -16,6 +16,7 @@
*/
package org.apache.logging.log4j.spi;
+import static org.apache.logging.log4j.test.ThreadLocalUtil.assertThreadLocalCount;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -28,7 +29,9 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
import java.util.Arrays;
import java.util.Collections;
import java.util.Iterator;
+import java.util.List;
import org.apache.logging.log4j.ThreadContext.ContextStack;
+import org.apache.logging.log4j.test.ThreadLocalUtil;
import org.apache.logging.log4j.test.junit.UsingAnyThreadContext;
import org.junit.jupiter.api.Test;
@@ -339,4 +342,28 @@ public class DefaultThreadContextStackTest {
stack.retainAll(Arrays.asList("msg1", "msg3"));
assertEquals("[msg1, msg3]", stack.toString());
}
+
+ @Test
+ void threadLocalUnsetWhenStackEmpty() throws Exception {
+ final DefaultThreadContextStack stack = new DefaultThreadContextStack(true);
+ final int threadLocalCount = ThreadLocalUtil.getThreadLocalCount();
+ // push and pop
+ stack.push("push and pop");
+ assertThreadLocalCount(threadLocalCount + 1);
+ stack.pop();
+ assertThreadLocalCount(threadLocalCount);
+
+ // push and remove
+ stack.push("push and remove");
+ assertThreadLocalCount(threadLocalCount + 1);
+ stack.remove("push and remove");
+ assertThreadLocalCount(threadLocalCount);
+
+ // addAll and removeAll
+ final List<String> data = Arrays.asList("addAll", "removeAll");
+ stack.addAll(data);
+ assertThreadLocalCount(threadLocalCount + 1);
+ stack.removeAll(data);
+ assertThreadLocalCount(threadLocalCount);
+ }
}
diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/spi/ThreadContextMapTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/spi/ThreadContextMapTest.java
index f394e71bea..97c3cdf03b 100644
--- a/log4j-api-test/src/test/java/org/apache/logging/log4j/spi/ThreadContextMapTest.java
+++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/spi/ThreadContextMapTest.java
@@ -16,13 +16,16 @@
*/
package org.apache.logging.log4j.spi;
+import static org.apache.logging.log4j.test.ThreadLocalUtil.assertThreadLocalCount;
import static org.assertj.core.api.Assertions.assertThat;
import java.time.Duration;
+import java.util.Arrays;
import java.util.Properties;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.stream.Stream;
+import org.apache.logging.log4j.test.ThreadLocalUtil;
import org.apache.logging.log4j.util.PropertiesUtil;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
@@ -73,4 +76,37 @@ class ThreadContextMapTest {
executorService.shutdown();
}
}
+
+ static Stream<ThreadContextMap> threadLocalsRemovedWhenMapEmpty() {
+ return Stream.of(new DefaultThreadContextMap(), new CopyOnWriteSortedArrayThreadContextMap());
+ }
+
+ @ParameterizedTest
+ @MethodSource
+ void threadLocalsRemovedWhenMapEmpty(final ThreadContextMap contextMap) {
+ // JUnit calls contextMap#toString() and sets the `ThreadLocal`.
+ contextMap.clear();
+ final int threadLocalCount = ThreadLocalUtil.getThreadLocalCount();
+
+ contextMap.put(KEY, "threadLocalsRemovedWhenMapEmpty");
+ assertThreadLocalCount(threadLocalCount + 1);
+ contextMap.remove(KEY);
+ assertThreadLocalCount(threadLocalCount);
+
+ contextMap.put("key1", "value1");
+ contextMap.put("key2", "value2");
+ assertThreadLocalCount(threadLocalCount + 1);
+ if (contextMap instanceof DefaultThreadContextMap) {
+ ((DefaultThreadContextMap) contextMap).removeAll(Arrays.asList("key1", "key2"));
+ }
+ if (contextMap instanceof CleanableThreadContextMap) {
+ ((CleanableThreadContextMap) contextMap).removeAll(Arrays.asList("key1", "key2"));
+ }
+ assertThreadLocalCount(threadLocalCount);
+
+ contextMap.put(KEY, "threadLocalsRemovedWhenMapEmpty");
+ assertThreadLocalCount(threadLocalCount + 1);
+ contextMap.clear();
+ assertThreadLocalCount(threadLocalCount);
+ }
}
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/internal/map/StringArrayThreadContextMap.java b/log4j-api/src/main/java/org/apache/logging/log4j/internal/map/StringArrayThreadContextMap.java
index 608ca77ffb..de837a9353 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/internal/map/StringArrayThreadContextMap.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/internal/map/StringArrayThreadContextMap.java
@@ -79,7 +79,11 @@ public class StringArrayThreadContextMap implements ThreadContextMap, ReadOnlySt
if (state != null) {
final UnmodifiableArrayBackedMap modifiedMap =
UnmodifiableArrayBackedMap.getInstance(state).copyAndRemove(key);
- threadLocalMapState.set(modifiedMap.getBackingArray());
+ if (modifiedMap.isEmpty()) {
+ threadLocalMapState.remove();
+ } else {
+ threadLocalMapState.set(modifiedMap.getBackingArray());
+ }
}
}
@@ -89,6 +93,11 @@ public class StringArrayThreadContextMap implements ThreadContextMap, ReadOnlySt
final UnmodifiableArrayBackedMap modifiedMap =
UnmodifiableArrayBackedMap.getInstance(state).copyAndRemoveAll(keys);
threadLocalMapState.set(modifiedMap.getBackingArray());
+ if (modifiedMap.isEmpty()) {
+ threadLocalMapState.remove();
+ } else {
+ threadLocalMapState.set(modifiedMap.getBackingArray());
+ }
}
}
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java
index ecc499ac58..058ce0bc47 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java
@@ -102,7 +102,12 @@ public abstract class AbstractLogger implements ExtendedLogger, LocationAwareLog
protected final String name;
private final MessageFactory2 messageFactory;
private final FlowMessageFactory flowMessageFactory;
- private static final ThreadLocal<int[]> recursionDepthHolder = new ThreadLocal<>(); // LOG4J2-1518, LOG4J2-2031
+ /**
+ * See <a href="https://issues.apache.org/jira/browse/LOG4J2-1518">LOG4J2-1518</a>,
+ * <a href="https://issues.apache.org/jira/browse/LOG4J2-2031">LOG4J2-2031</a>.
+ */
+ private static final ThreadLocal<Integer> recursionDepthHolder = new ThreadLocal<>();
+
private static final ThreadLocal<DefaultLogBuilder> logBuilder = ThreadLocal.withInitial(DefaultLogBuilder::new);
/**
@@ -2860,23 +2865,16 @@ public abstract class AbstractLogger implements ExtendedLogger, LocationAwareLog
}
}
- private static int[] getRecursionDepthHolder() {
- int[] result = recursionDepthHolder.get();
- if (result == null) {
- result = new int[1];
- recursionDepthHolder.set(result);
- }
- return result;
- }
-
private static void incrementRecursionDepth() {
- getRecursionDepthHolder()[0]++;
+ recursionDepthHolder.set(getRecursionDepth() + 1);
}
private static void decrementRecursionDepth() {
- final int newDepth = --getRecursionDepthHolder()[0];
- if (newDepth < 0) {
- throw new IllegalStateException("Recursion depth became negative: " + newDepth);
+ final int newDepth = getRecursionDepth() - 1;
+ if (newDepth == 0) {
+ recursionDepthHolder.remove();
+ } else {
+ recursionDepthHolder.set(newDepth);
}
}
@@ -2887,7 +2885,8 @@ public abstract class AbstractLogger implements ExtendedLogger, LocationAwareLog
* @return the depth of the nested logging calls in the current Thread
*/
public static int getRecursionDepth() {
- return getRecursionDepthHolder()[0];
+ final Integer recursionDepth = recursionDepthHolder.get();
+ return recursionDepth != null ? recursionDepth : 0;
}
@PerformanceSensitive
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/CopyOnWriteSortedArrayThreadContextMap.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/CopyOnWriteSortedArrayThreadContextMap.java
index ca9f501c21..9377d5e16f 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/CopyOnWriteSortedArrayThreadContextMap.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/CopyOnWriteSortedArrayThreadContextMap.java
@@ -165,8 +165,12 @@ class CopyOnWriteSortedArrayThreadContextMap implements ReadOnlyThreadContextMap
if (map != null) {
final StringMap copy = createStringMap(map);
copy.remove(key);
- copy.freeze();
- localMap.set(copy);
+ if (copy.isEmpty()) {
+ localMap.remove();
+ } else {
+ copy.freeze();
+ localMap.set(copy);
+ }
}
}
@@ -178,8 +182,12 @@ class CopyOnWriteSortedArrayThreadContextMap implements ReadOnlyThreadContextMap
for (final String key : keys) {
copy.remove(key);
}
- copy.freeze();
- localMap.set(copy);
+ if (copy.isEmpty()) {
+ localMap.remove();
+ } else {
+ copy.freeze();
+ localMap.set(copy);
+ }
}
}
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextMap.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextMap.java
index a07992d371..e5b658976a 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextMap.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextMap.java
@@ -104,7 +104,11 @@ public class DefaultThreadContextMap implements ThreadContextMap, ReadOnlyString
if (map != null) {
final Map<String, String> copy = new HashMap<>(map);
copy.remove(key);
- localMap.set(Collections.unmodifiableMap(copy));
+ if (copy.isEmpty()) {
+ localMap.remove();
+ } else {
+ localMap.set(Collections.unmodifiableMap(copy));
+ }
}
}
@@ -115,7 +119,11 @@ public class DefaultThreadContextMap implements ThreadContextMap, ReadOnlyString
for (final String key : keys) {
copy.remove(key);
}
- localMap.set(Collections.unmodifiableMap(copy));
+ if (copy.isEmpty()) {
+ localMap.remove();
+ } else {
+ localMap.set(Collections.unmodifiableMap(copy));
+ }
}
}
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextStack.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextStack.java
index 5ed3fb41ea..7846f67f1a 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextStack.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/DefaultThreadContextStack.java
@@ -35,7 +35,7 @@ public class DefaultThreadContextStack implements ThreadContextStack, StringBuil
private static final long serialVersionUID = 5050501L;
- private static final ThreadLocal<MutableThreadContextStack> STACK = new ThreadLocal<>();
+ private final ThreadLocal<MutableThreadContextStack> STACK = new ThreadLocal<>();
private final boolean useStack;
@@ -197,8 +197,12 @@ public class DefaultThreadContextStack implements ThreadContextStack, StringBuil
}
final MutableThreadContextStack copy = (MutableThreadContextStack) values.copy();
final String result = copy.pop();
- copy.freeze();
- STACK.set(copy);
+ if (copy.isEmpty()) {
+ STACK.remove();
+ } else {
+ copy.freeze();
+ STACK.set(copy);
+ }
return result;
}
@@ -221,8 +225,12 @@ public class DefaultThreadContextStack implements ThreadContextStack, StringBuil
}
final MutableThreadContextStack copy = (MutableThreadContextStack) values.copy();
final boolean result = copy.remove(o);
- copy.freeze();
- STACK.set(copy);
+ if (copy.isEmpty()) {
+ STACK.remove();
+ } else {
+ copy.freeze();
+ STACK.set(copy);
+ }
return result;
}
@@ -237,8 +245,12 @@ public class DefaultThreadContextStack implements ThreadContextStack, StringBuil
}
final MutableThreadContextStack copy = (MutableThreadContextStack) values.copy();
final boolean result = copy.removeAll(objects);
- copy.freeze();
- STACK.set(copy);
+ if (copy.isEmpty()) {
+ STACK.remove();
+ } else {
+ copy.freeze();
+ STACK.set(copy);
+ }
return result;
}
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java
index 91057b53d6..09f596a578 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java
@@ -311,6 +311,10 @@ public final class Strings {
return str.toUpperCase(Locale.ROOT);
}
+ private static StringBuilder getStringBuilder() {
+ return Constants.ENABLE_THREADLOCALS ? FORMAT_BUFFER_REF.get() : new StringBuilder();
+ }
+
/**
* Concatenates 2 Strings without allocation.
* @param str1 the first string.
@@ -323,7 +327,7 @@ public final class Strings {
} else if (isEmpty(str2)) {
return str1;
}
- final StringBuilder sb = FORMAT_BUFFER_REF.get();
+ final StringBuilder sb = getStringBuilder();
try {
return sb.append(str1).append(str2).toString();
} finally {
@@ -344,7 +348,7 @@ public final class Strings {
if (count < 0) {
throw new IllegalArgumentException("count");
}
- final StringBuilder sb = FORMAT_BUFFER_REF.get();
+ final StringBuilder sb = getStringBuilder();
try {
for (int index = 0; index < count; index++) {
sb.append(str);
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/Timer.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/Timer.java
index f85fb1a1c0..cbb2841a7d 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/util/Timer.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/Timer.java
@@ -38,15 +38,10 @@ public class Timer implements Serializable, StringBuilderFormattable {
private Status status; // The timer's status
private long elapsedTime; // The elapsed time
private final int iterations;
- private static long NANO_PER_SECOND = 1000000000L;
- private static long NANO_PER_MINUTE = NANO_PER_SECOND * 60;
- private static long NANO_PER_HOUR = NANO_PER_MINUTE * 60;
- private ThreadLocal<Long> startTime = new ThreadLocal<Long>() {
- @Override
- protected Long initialValue() {
- return 0L;
- }
- };
+ private static final long NANO_PER_SECOND = 1000000000L;
+ private static final long NANO_PER_MINUTE = NANO_PER_SECOND * 60;
+ private static final long NANO_PER_HOUR = NANO_PER_MINUTE * 60;
+ private final ThreadLocal<Long> startTime = ThreadLocal.withInitial(() -> 0L);
/**
* Constructor.
@@ -64,7 +59,7 @@ public class Timer implements Serializable, StringBuilderFormattable {
public Timer(final String name, final int iterations) {
this.name = name;
status = Status.Stopped;
- this.iterations = (iterations > 0) ? iterations : 0;
+ this.iterations = Math.max(iterations, 0);
}
/**
@@ -89,7 +84,7 @@ public class Timer implements Serializable, StringBuilderFormattable {
*/
public synchronized String stop() {
elapsedTime += System.nanoTime() - startTime.get();
- startTime.set(0L);
+ startTime.remove();
status = Status.Stopped;
return toString();
}
@@ -99,7 +94,7 @@ public class Timer implements Serializable, StringBuilderFormattable {
*/
public synchronized void pause() {
elapsedTime += System.nanoTime() - startTime.get();
- startTime.set(0L);
+ startTime.remove();
status = Status.Paused;
}
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLoggerConfig.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLoggerConfig.java
index b3c11bb5d6..d181000800 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLoggerConfig.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLoggerConfig.java
@@ -94,12 +94,7 @@ public class AsyncLoggerConfig extends LoggerConfig {
}
}
- private static final ThreadLocal<Boolean> ASYNC_LOGGER_ENTERED = new ThreadLocal<Boolean>() {
- @Override
- protected Boolean initialValue() {
- return Boolean.FALSE;
- }
- };
+ private static final ThreadLocal<Boolean> ASYNC_LOGGER_ENTERED = new ThreadLocal<>();
private final AsyncLoggerConfigDelegate delegate;
@@ -126,29 +121,30 @@ public class AsyncLoggerConfig extends LoggerConfig {
protected void log(final LogEvent event, final LoggerConfigPredicate predicate) {
// See LOG4J2-2301
if (predicate == LoggerConfigPredicate.ALL
- && ASYNC_LOGGER_ENTERED.get() == Boolean.FALSE
&&
// Optimization: AsyncLoggerConfig is identical to LoggerConfig
// when no appenders are present. Avoid splitting for synchronous
// and asynchronous execution paths until encountering an
// AsyncLoggerConfig with appenders.
hasAppenders()) {
- // This is the first AsnycLoggerConfig encountered by this LogEvent
- ASYNC_LOGGER_ENTERED.set(Boolean.TRUE);
try {
- if (!isFiltered(event)) {
- // Detect the first time we encounter an AsyncLoggerConfig. We must log
- // to all non-async loggers first.
- processLogEvent(event, LoggerConfigPredicate.SYNCHRONOUS_ONLY);
- // Then pass the event to the background thread where
- // all async logging is executed. It is important this
- // happens at most once and after all synchronous loggers
- // have been invoked, because we lose parameter references
- // from reusable messages.
- logToAsyncDelegate(event);
+ if (ASYNC_LOGGER_ENTERED.get() == null) {
+ // This is the first AsyncLoggerConfig encountered by this LogEvent
+ ASYNC_LOGGER_ENTERED.set(Boolean.TRUE);
+ if (!isFiltered(event)) {
+ // Detect the first time we encounter an AsyncLoggerConfig. We must log
+ // to all non-async loggers first.
+ processLogEvent(event, LoggerConfigPredicate.SYNCHRONOUS_ONLY);
+ // Then pass the event to the background thread where
+ // all async logging is executed. It is important this
+ // happens at most once and after all synchronous loggers
+ // have been invoked, because we lose parameter references
+ // from reusable messages.
+ logToAsyncDelegate(event);
+ }
}
} finally {
- ASYNC_LOGGER_ENTERED.set(Boolean.FALSE);
+ ASYNC_LOGGER_ENTERED.remove();
}
} else {
super.log(event, predicate);
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AppenderControl.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AppenderControl.java
index 2dfd13b2a8..660abac909 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AppenderControl.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AppenderControl.java
@@ -123,7 +123,7 @@ public class AppenderControl extends AbstractFilterable {
recursive.set(this);
callAppender0(event);
} finally {
- recursive.set(null);
+ recursive.remove();
}
}
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/StructuredDataFilter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/StructuredDataFilter.java
index d9c991d6b6..6a105fff54 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/StructuredDataFilter.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/StructuredDataFilter.java
@@ -30,6 +30,7 @@ import org.apache.logging.log4j.core.config.plugins.Plugin;
import org.apache.logging.log4j.core.config.plugins.PluginAttribute;
import org.apache.logging.log4j.core.config.plugins.PluginElement;
import org.apache.logging.log4j.core.config.plugins.PluginFactory;
+import org.apache.logging.log4j.core.util.Constants;
import org.apache.logging.log4j.core.util.KeyValuePair;
import org.apache.logging.log4j.message.Message;
import org.apache.logging.log4j.message.StructuredDataMessage;
@@ -45,7 +46,7 @@ import org.apache.logging.log4j.util.StringBuilders;
public final class StructuredDataFilter extends MapFilter {
private static final int MAX_BUFFER_SIZE = 2048;
- private static ThreadLocal<StringBuilder> threadLocalStringBuilder = new ThreadLocal<>();
+ private static final ThreadLocal<StringBuilder> threadLocalStringBuilder = new ThreadLocal<>();
private StructuredDataFilter(
final Map<String, List<String>> map, final boolean oper, final Result onMatch, final Result onMismatch) {
@@ -105,14 +106,17 @@ public final class StructuredDataFilter extends MapFilter {
}
private StringBuilder getStringBuilder() {
- StringBuilder result = threadLocalStringBuilder.get();
- if (result == null) {
- result = new StringBuilder();
- threadLocalStringBuilder.set(result);
+ if (Constants.ENABLE_THREADLOCALS) {
+ StringBuilder result = threadLocalStringBuilder.get();
+ if (result == null) {
+ result = new StringBuilder();
+ threadLocalStringBuilder.set(result);
+ }
+ StringBuilders.trimToMaxSize(result, MAX_BUFFER_SIZE);
+ result.setLength(0);
+ return result;
}
- StringBuilders.trimToMaxSize(result, MAX_BUFFER_SIZE);
- result.setLength(0);
- return result;
+ return new StringBuilder();
}
private StringBuilder appendOrNull(final String value, final StringBuilder sb) {
diff --git a/log4j-jul/src/main/java/org/apache/logging/log4j/jul/LogManager.java b/log4j-jul/src/main/java/org/apache/logging/log4j/jul/LogManager.java
index bb8f6e737e..4f396d13b4 100644
--- a/log4j-jul/src/main/java/org/apache/logging/log4j/jul/LogManager.java
+++ b/log4j-jul/src/main/java/org/apache/logging/log4j/jul/LogManager.java
@@ -16,6 +16,8 @@
*/
package org.apache.logging.log4j.jul;
+import static org.apache.logging.log4j.util.Constants.ENABLE_THREADLOCALS;
+
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashSet;
@@ -94,6 +96,9 @@ public class LogManager extends java.util.logging.LogManager {
return loggerAdapter.getLogger(name);
} finally {
activeRequests.remove(name);
+ if (!ENABLE_THREADLOCALS && activeRequests.isEmpty()) {
+ recursive.remove();
+ }
}
}
LOGGER.warn("Recursive call to getLogger for {} ignored.", name);
diff --git a/log4j-slf4j2-impl/pom.xml b/log4j-slf4j2-impl/pom.xml
index f6011d3948..67f4c01bed 100644
--- a/log4j-slf4j2-impl/pom.xml
+++ b/log4j-slf4j2-impl/pom.xml
@@ -157,4 +157,41 @@
</plugins>
</build>
+
+ <profiles>
+
+ <!-- Fixes incompatible with Java 8 -->
+ <profile>
+
+ <id>java8-incompat-fixes</id>
+
+ <!-- CI uses Java 8 for running tests.
+ Hence, we assume CI=Java8 and apply our changes elsewhere.
+
+ One might think why not activate using `<jdk>[16,)` instead?
+ This doesn't work, since the match is not against "the JDK running tests", but "the JDK running Maven".
+ These two JDKs can differ due to Maven Toolchains.
+ See `java8-tests` profile in `/pom.xml` for details. -->
+ <activation>
+ <property>
+ <name>!env.CI</name>
+ </property>
+ </activation>
+
+ <!-- Illegal access is disabled by default in Java 16 due to JEP-396.
+ We are relaxing it for tests. -->
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <configuration>
+ <argLine>--add-opens java.base/java.lang=ALL-UNNAMED</argLine>
+ </configuration>
+ </plugin>
+ </plugins>
+ </build>
+
+ </profile>
+ </profiles>
</project>
diff --git a/log4j-slf4j2-impl/src/main/java/org/apache/logging/slf4j/Log4jMDCAdapter.java b/log4j-slf4j2-impl/src/main/java/org/apache/logging/slf4j/Log4jMDCAdapter.java
index bae69e961d..ab1184e5bb 100644
--- a/log4j-slf4j2-impl/src/main/java/org/apache/logging/slf4j/Log4jMDCAdapter.java
+++ b/log4j-slf4j2-impl/src/main/java/org/apache/logging/slf4j/Log4jMDCAdapter.java
@@ -25,11 +25,9 @@ import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.ThreadContext;
import org.apache.logging.log4j.ThreadContext.ContextStack;
import org.apache.logging.log4j.status.StatusLogger;
+import org.apache.logging.log4j.util.Constants;
import org.slf4j.spi.MDCAdapter;
-/**
- *
- */
public class Log4jMDCAdapter implements MDCAdapter {
private static final Logger LOGGER = StatusLogger.getLogger();
@@ -90,7 +88,12 @@ public class Log4jMDCAdapter implements MDCAdapter {
if (!Objects.equals(ThreadContext.get(key), value)) {
LOGGER.warn("The key {} was used in both the string and stack-valued MDC.", key);
}
- ThreadContext.put(key, mapOfStacks.peekByKey(key));
+ final String oldValue = mapOfStacks.peekByKey(key);
+ if (oldValue != null) {
+ ThreadContext.put(key, oldValue);
+ } else {
+ ThreadContext.remove(key);
+ }
return value;
}
@@ -111,41 +114,92 @@ public class Log4jMDCAdapter implements MDCAdapter {
ThreadContext.clearStack();
} else {
mapOfStacks.clearByKey(key);
- ThreadContext.put(key, null);
+ ThreadContext.remove(key);
}
}
+ // Used by tests
+ void clearDeque() {
+ mapOfStacks.clear();
+ }
+
private static class ThreadLocalMapOfStacks {
private final ThreadLocal<Map<String, Deque<String>>> tlMapOfStacks = ThreadLocal.withInitial(HashMap::new);
+ private Map<String, Deque<String>> getMapOfStacks() {
+ Map<String, Deque<String>> localMap = tlMapOfStacks.get();
+ if (localMap == null) {
+ tlMapOfStacks.set(localMap = new HashMap<>());
+ }
+ return localMap;
+ }
+
+ private void removeIfEmpty(final Map<String, Deque<String>> mapOfStacks) {
+ if (!Constants.ENABLE_THREADLOCALS && mapOfStacks != null && mapOfStacks.isEmpty()) {
+ tlMapOfStacks.remove();
+ }
+ }
+
public void pushByKey(final String key, final String value) {
- tlMapOfStacks
- .get()
- .computeIfAbsent(key, ignored -> new ArrayDeque<>())
- .push(value);
+ getMapOfStacks().computeIfAbsent(key, ignored -> new ArrayDeque<>()).push(value);
}
public String popByKey(final String key) {
- final Deque<String> deque = tlMapOfStacks.get().get(key);
- return deque != null ? deque.poll() : null;
+ final Map<String, Deque<String>> mapOfStacks = tlMapOfStacks.get();
+ if (mapOfStacks != null) {
+ final Deque<String> deque = mapOfStacks.get(key);
+ if (deque != null) {
+ final String result = deque.poll();
+ if (!Constants.ENABLE_THREADLOCALS && deque.isEmpty()) {
+ mapOfStacks.remove(key);
+ removeIfEmpty(mapOfStacks);
+ }
+ return result;
+ }
+ }
+ return null;
}
public Deque<String> getCopyOfDequeByKey(final String key) {
- final Deque<String> deque = tlMapOfStacks.get().get(key);
- return deque != null ? new ArrayDeque<>(deque) : null;
+ final Map<String, Deque<String>> mapOfStacks = tlMapOfStacks.get();
+ if (mapOfStacks != null) {
+ final Deque<String> deque = mapOfStacks.get(key);
+ return deque != null ? new ArrayDeque<>(deque) : null;
+ }
+ return null;
+ }
+
+ public void clear() {
+ final Map<String, Deque<String>> mapOfStacks = tlMapOfStacks.get();
+ if (Constants.ENABLE_THREADLOCALS) {
+ mapOfStacks.clear();
+ } else {
+ tlMapOfStacks.remove();
+ }
}
public void clearByKey(final String key) {
- final Deque<String> deque = tlMapOfStacks.get().get(key);
- if (deque != null) {
- deque.clear();
+ final Map<String, Deque<String>> mapOfStacks = tlMapOfStacks.get();
+ if (mapOfStacks != null) {
+ mapOfStacks.computeIfPresent(key, (ignored, deque) -> {
+ if (Constants.ENABLE_THREADLOCALS) {
+ deque.clear();
+ return deque;
+ }
+ return null;
+ });
}
+ removeIfEmpty(mapOfStacks);
}
public String peekByKey(final String key) {
- final Deque<String> deque = tlMapOfStacks.get().get(key);
- return deque != null ? deque.peek() : null;
+ final Map<String, Deque<String>> mapOfStacks = tlMapOfStacks.get();
+ if (mapOfStacks != null) {
+ final Deque<String> deque = mapOfStacks.get(key);
+ return deque != null ? deque.peek() : null;
+ }
+ return null;
}
}
}
diff --git a/log4j-slf4j2-impl/src/test/java/org/apache/logging/slf4j/Log4jMDCAdapterTest.java b/log4j-slf4j2-impl/src/test/java/org/apache/logging/slf4j/Log4jMDCAdapterTest.java
index 0559608f46..3d38f5ce9f 100644
--- a/log4j-slf4j2-impl/src/test/java/org/apache/logging/slf4j/Log4jMDCAdapterTest.java
+++ b/log4j-slf4j2-impl/src/test/java/org/apache/logging/slf4j/Log4jMDCAdapterTest.java
@@ -16,16 +16,21 @@
*/
package org.apache.logging.slf4j;
+import static org.apache.logging.log4j.test.ThreadLocalUtil.assertThreadLocalCount;
import static org.assertj.core.api.Assertions.assertThat;
import java.util.ArrayDeque;
import java.util.Deque;
import java.util.stream.IntStream;
import java.util.stream.Stream;
+import org.apache.logging.log4j.test.ThreadLocalUtil;
+import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
+import org.junitpioneer.jupiter.SetSystemProperty;
-public class Log4jMDCAdapterTest {
+@SetSystemProperty(key = "log4j2.enableThreadLocals", value = "false")
+class Log4jMDCAdapterTest {
private static final Log4jMDCAdapter MDC_ADAPTER = new Log4jMDCAdapter();
private static final String KEY = "Log4j2";
@@ -51,11 +56,22 @@ public class Log4jMDCAdapterTest {
@ParameterizedTest
@MethodSource("keys")
- public void testPushPopByKey(final String key) {
+ void testPushPopByKey(final String key) {
MDC_ADAPTER.clearDequeByKey(key);
final Deque<String> expectedValues = createDeque(100);
expectedValues.descendingIterator().forEachRemaining(v -> MDC_ADAPTER.pushByKey(key, v));
assertThat(MDC_ADAPTER.getCopyOfDequeByKey(key)).containsExactlyElementsOf(expectedValues);
assertThat(popDeque(key)).containsExactlyElementsOf(expectedValues);
}
+
+ @Test
+ void threadLocalClearedWhenMapOfStacksEmpty() {
+ MDC_ADAPTER.clear();
+ MDC_ADAPTER.clearDeque();
+ final int threadLocalCount = ThreadLocalUtil.getThreadLocalCount();
+ MDC_ADAPTER.pushByKey(KEY, "threadLocalClearedWhenMapOfStacksEmpty");
+ assertThreadLocalCount(threadLocalCount + 1);
+ assertThat(MDC_ADAPTER.popByKey(KEY)).isEqualTo("threadLocalClearedWhenMapOfStacksEmpty");
+ assertThreadLocalCount(threadLocalCount);
+ }
}