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 2023/01/26 04:46:24 UTC
[logging-log4j2] branch master updated (eb3ec0f1b8 -> ec35f53f3f)
This is an automated email from the ASF dual-hosted git repository.
pkarwasz pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
from eb3ec0f1b8 Fix (duplicated when exported) comment block in `.changelog-entries.adoc.ftl`
new f165c135e8 [LOG4J2-3615] Fix `log4j-api` OSGI encapsulation problem
new 635ca9b67a Make `AbstractBuilder.logBuilder` static
new ec35f53f3f Add changelog and test to `DefaultLogBuilder`
The 3 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:
.../log4j/internal/DefaultLogBuilderTest.java | 49 ++++++++++++++++++++++
.../logging/log4j/internal/DefaultLogBuilder.java | 15 ++++---
.../apache/logging/log4j/spi/AbstractLogger.java | 27 ++----------
...ttern.xml => LOG4J2-3615_Fix_OSGI_API_leak.xml} | 5 +--
4 files changed, 62 insertions(+), 34 deletions(-)
create mode 100644 log4j-api-test/src/test/java/org/apache/logging/log4j/internal/DefaultLogBuilderTest.java
copy src/changelog/.2.x.x/{LOG4J2-1631_Honor-timezone-in-file-name-pattern.xml => LOG4J2-3615_Fix_OSGI_API_leak.xml} (83%)
[logging-log4j2] 02/03: Make `AbstractBuilder.logBuilder` static
Posted by pk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
pkarwasz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 635ca9b67ab4df0b3a017c1fff5f5e7a08c2e2d8
Author: Piotr P. Karwasz <pi...@karwasz.org>
AuthorDate: Sat Jan 14 21:16:46 2023 +0100
Make `AbstractBuilder.logBuilder` static
A non-static `ThreadLocal` for hundreds of loggers creates thousands of
`LogBuilder` instances.
---
.../logging/log4j/internal/DefaultLogBuilder.java | 15 ++++++------
.../apache/logging/log4j/spi/AbstractLogger.java | 27 ++++------------------
2 files changed, 11 insertions(+), 31 deletions(-)
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/internal/DefaultLogBuilder.java b/log4j-api/src/main/java/org/apache/logging/log4j/internal/DefaultLogBuilder.java
index cd472c43e3..6a7a2f31cc 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/internal/DefaultLogBuilder.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/internal/DefaultLogBuilder.java
@@ -39,7 +39,7 @@ public class DefaultLogBuilder implements BridgeAware, LogBuilder {
private static final Logger LOGGER = StatusLogger.getLogger();
private static final Message EMPTY_MESSAGE = new SimpleMessage(Strings.EMPTY);
- private final Logger logger;
+ private Logger logger;
private Level level;
private Marker marker;
private Throwable throwable;
@@ -52,13 +52,11 @@ public class DefaultLogBuilder implements BridgeAware, LogBuilder {
this.logger = logger;
this.level = level;
this.threadId = Thread.currentThread().getId();
- this.inUse = true;
+ this.inUse = level != null;
}
- public DefaultLogBuilder(final Logger logger) {
- this.logger = logger;
- this.inUse = false;
- this.threadId = Thread.currentThread().getId();
+ public DefaultLogBuilder() {
+ this(null, null);
}
@Override
@@ -71,12 +69,13 @@ public class DefaultLogBuilder implements BridgeAware, LogBuilder {
* @param level The logging level for this event.
* @return This LogBuilder instance.
*/
- public LogBuilder reset(final Level level) {
- this.inUse = true;
+ public LogBuilder reset(Logger logger, Level level) {
+ this.logger = logger;
this.level = level;
this.marker = null;
this.throwable = null;
this.location = null;
+ this.inUse = true;
return this;
}
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 2f6ddd2262..dc76835569 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
@@ -84,7 +84,7 @@ public abstract class AbstractLogger implements ExtendedLogger {
private final MessageFactory messageFactory;
private final FlowMessageFactory flowMessageFactory;
private static final ThreadLocal<int[]> recursionDepthHolder = new ThreadLocal<>(); // LOG4J2-1518, LOG4J2-2031
- private final transient ThreadLocal<DefaultLogBuilder> logBuilder;
+ private static final ThreadLocal<DefaultLogBuilder> logBuilder = ThreadLocal.withInitial(DefaultLogBuilder::new);
/**
@@ -95,7 +95,6 @@ public abstract class AbstractLogger implements ExtendedLogger {
this.name = canonicalName != null ? canonicalName : getClass().getName();
this.messageFactory = LoggingSystem.getMessageFactory();
this.flowMessageFactory = LoggingSystem.getFlowMessageFactory();
- this.logBuilder = new LocalLogBuilder(this);
}
/**
@@ -117,7 +116,6 @@ public abstract class AbstractLogger implements ExtendedLogger {
this.name = name;
this.messageFactory = messageFactory == null ? LoggingSystem.getMessageFactory() : messageFactory;
this.flowMessageFactory = LoggingSystem.getFlowMessageFactory();
- this.logBuilder = new LocalLogBuilder(this);
}
/**
@@ -2742,11 +2740,7 @@ public abstract class AbstractLogger implements ExtendedLogger {
*/
@Override
public LogBuilder always() {
- final DefaultLogBuilder builder = logBuilder.get();
- if (builder.isInUse()) {
- return new DefaultLogBuilder(this);
- }
- return builder.reset(Level.OFF);
+ return getLogBuilder(Level.OFF);
}
/**
@@ -2757,26 +2751,13 @@ public abstract class AbstractLogger implements ExtendedLogger {
@Override
public LogBuilder atLevel(final Level level) {
if (isEnabled(level)) {
- return (getLogBuilder(level).reset(level));
- } else {
- return LogBuilder.NOOP;
+ return getLogBuilder(level).reset(this, level);
}
+ return LogBuilder.NOOP;
}
private DefaultLogBuilder getLogBuilder(final Level level) {
final DefaultLogBuilder builder = logBuilder.get();
return Constants.isThreadLocalsEnabled() && !builder.isInUse() ? builder : new DefaultLogBuilder(this, level);
}
-
- private static class LocalLogBuilder extends ThreadLocal<DefaultLogBuilder> {
- private final AbstractLogger logger;
- LocalLogBuilder(final AbstractLogger logger) {
- this.logger = logger;
- }
-
- @Override
- protected DefaultLogBuilder initialValue() {
- return new DefaultLogBuilder(logger);
- }
- }
}
[logging-log4j2] 01/03: [LOG4J2-3615] Fix `log4j-api` OSGI encapsulation problem
Posted by pk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
pkarwasz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit f165c135e8e8a4486114a3d023a1259b829a20a0
Author: Piotr P. Karwasz <pi...@karwasz.org>
AuthorDate: Fri Oct 7 00:08:28 2022 +0200
[LOG4J2-3615] Fix `log4j-api` OSGI encapsulation problem
The protected `AbstractLogger.logBuilder` field leaks the internal
`DefaultLogBuilder` class into the public API. This causes a warning in
the upgraded `maven-bundle-plugin`.
Since this is a breaking change, I think that a larger consensus is
necessary.
---
.../src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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 f6d5a22077..2f6ddd2262 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
@@ -84,7 +84,7 @@ public abstract class AbstractLogger implements ExtendedLogger {
private final MessageFactory messageFactory;
private final FlowMessageFactory flowMessageFactory;
private static final ThreadLocal<int[]> recursionDepthHolder = new ThreadLocal<>(); // LOG4J2-1518, LOG4J2-2031
- protected final transient ThreadLocal<DefaultLogBuilder> logBuilder;
+ private final transient ThreadLocal<DefaultLogBuilder> logBuilder;
/**
[logging-log4j2] 03/03: Add changelog and test to `DefaultLogBuilder`
Posted by pk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
pkarwasz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit ec35f53f3f3fe860319b345d5147a991929c43fe
Author: Piotr P. Karwasz <pi...@karwasz.org>
AuthorDate: Wed Jan 25 22:07:11 2023 +0100
Add changelog and test to `DefaultLogBuilder`
---
.../log4j/internal/DefaultLogBuilderTest.java | 49 ++++++++++++++++++++++
.../.2.x.x/LOG4J2-3615_Fix_OSGI_API_leak.xml | 24 +++++++++++
2 files changed, 73 insertions(+)
diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/internal/DefaultLogBuilderTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/internal/DefaultLogBuilderTest.java
new file mode 100644
index 0000000000..1a6a404d82
--- /dev/null
+++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/internal/DefaultLogBuilderTest.java
@@ -0,0 +1,49 @@
+/*
+ * 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.internal;
+
+import java.util.Arrays;
+import java.util.List;
+
+import org.apache.logging.log4j.LogBuilder;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.test.TestLogger;
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class DefaultLogBuilderTest {
+
+ private final TestLogger logger1 = (TestLogger) LogManager.getLogger(DefaultLogBuilderTest.class);
+ private final TestLogger logger2 = (TestLogger) LogManager.getLogger("second.logger");
+
+ @Test
+ public void testConcurrentUsage() {
+ logger1.getEntries().clear();
+ logger2.getEntries().clear();
+ final List<LogBuilder> logBuilders = Arrays.asList(
+ logger1.atDebug(),
+ logger1.atInfo(),
+ logger2.atDebug(),
+ logger2.atInfo());
+ logBuilders.forEach(logBuilder -> logBuilder.log("Hello LogBuilder!"));
+ assertThat(logger1.getEntries()).hasSize(2)
+ .containsExactly(" DEBUG Hello LogBuilder!", " INFO Hello LogBuilder!");
+ assertThat(logger2.getEntries()).hasSize(2)
+ .containsExactly(" DEBUG Hello LogBuilder!", " INFO Hello LogBuilder!");
+ }
+}
diff --git a/src/changelog/.2.x.x/LOG4J2-3615_Fix_OSGI_API_leak.xml b/src/changelog/.2.x.x/LOG4J2-3615_Fix_OSGI_API_leak.xml
new file mode 100644
index 0000000000..b0706f0cb6
--- /dev/null
+++ b/src/changelog/.2.x.x/LOG4J2-3615_Fix_OSGI_API_leak.xml
@@ -0,0 +1,24 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ 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.
+-->
+<entry type="added">
+ <issue id="LOG4J2-3615" link="https://issues.apache.org/jira/browse/LOG4J2-3615"/>
+ <author id="pkarwasz"/>
+ <description format="asciidoc">
+ Removes internal field that leaked into public API.
+ </description>
+</entry>