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 <piotr.git...@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);
+ }
}
