Repository: logging-log4j2
Updated Branches:
  refs/heads/master 6467586f8 -> 38e59a119


[LOG4J2-2368] Recursive logging doesn't clobber cached StringBuidlers

This closes #189


Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/38e59a11
Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/38e59a11
Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/38e59a11

Branch: refs/heads/master
Commit: 38e59a1191b8d50af416e0428addeabcfc145d61
Parents: 6467586
Author: Carter Kozak <[email protected]>
Authored: Mon Jul 9 17:13:20 2018 -0400
Committer: Carter Kozak <[email protected]>
Committed: Mon Jul 9 20:19:17 2018 -0400

----------------------------------------------------------------------
 .../log4j/core/layout/AbstractStringLayout.java |   5 +
 .../NestedLoggingFromThrowableMessageTest.java  | 101 +++++++++++++++++++
 .../log4j-nested-logging-throwable-message.xml  |  43 ++++++++
 src/changes/changes.xml                         |   6 ++
 4 files changed, 155 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/38e59a11/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java
----------------------------------------------------------------------
diff --git 
a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java
 
b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java
index 88e541c..d8403b6 100644
--- 
a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java
+++ 
b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java
@@ -29,6 +29,7 @@ import 
org.apache.logging.log4j.core.config.plugins.PluginElement;
 import org.apache.logging.log4j.core.impl.DefaultLogEventFactory;
 import org.apache.logging.log4j.core.util.Constants;
 import org.apache.logging.log4j.core.util.StringEncoder;
+import org.apache.logging.log4j.spi.AbstractLogger;
 import org.apache.logging.log4j.util.PropertiesUtil;
 import org.apache.logging.log4j.util.StringBuilders;
 import org.apache.logging.log4j.util.Strings;
@@ -114,6 +115,10 @@ public abstract class AbstractStringLayout extends 
AbstractLayout<String> implem
      * @return a {@code StringBuilder}
      */
     protected static StringBuilder getStringBuilder() {
+        if (AbstractLogger.getRecursionDepth() > 1) { // LOG4J2-2368
+            // Recursive logging may clobber the cached StringBuilder.
+            return new StringBuilder(DEFAULT_STRING_BUILDER_SIZE);
+        }
         StringBuilder result = threadLocal.get();
         if (result == null) {
             result = new StringBuilder(DEFAULT_STRING_BUILDER_SIZE);

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/38e59a11/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/NestedLoggingFromThrowableMessageTest.java
----------------------------------------------------------------------
diff --git 
a/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/NestedLoggingFromThrowableMessageTest.java
 
b/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/NestedLoggingFromThrowableMessageTest.java
new file mode 100644
index 0000000..912f1a8
--- /dev/null
+++ 
b/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/NestedLoggingFromThrowableMessageTest.java
@@ -0,0 +1,101 @@
+/*
+ * 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.core.impl;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.core.CoreLoggerContexts;
+import org.apache.logging.log4j.junit.LoggerContextRule;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.Test;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.util.HashSet;
+import java.util.Set;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Test for LOG4J2-2368.
+ */
+public class NestedLoggingFromThrowableMessageTest {
+
+    private static File file1 = new File("target/NestedLoggerTest1.log");
+    private static File file2 = new File("target/NestedLoggerTest2.log");
+
+    @BeforeClass
+    public static void beforeClass() {
+        file1.delete();
+        file2.delete();
+        System.setProperty("log4j2.is.webapp", "false");
+    }
+
+    @Rule
+    public LoggerContextRule context = new 
LoggerContextRule("log4j-nested-logging-throwable-message.xml");
+    private Logger logger;
+
+    @Before
+    public void before() {
+        logger = 
LogManager.getLogger(NestedLoggingFromThrowableMessageTest.class);
+    }
+
+    class ThrowableLogsInGetMessage extends RuntimeException {
+
+        @Override
+        public String getMessage() {
+            logger.info("Logging in getMessage");
+            return "message";
+        }
+    }
+
+    @Test
+    public void testNestedLoggingInLastArgument() throws Exception {
+        logger.error("Test", new ThrowableLogsInGetMessage());
+        // stop async thread
+        CoreLoggerContexts.stopLoggerContext(false, file1);
+        CoreLoggerContexts.stopLoggerContext(false, file2);
+
+        Set<String> lines1 = readUniqueLines(file1);
+        Set<String> lines2 = readUniqueLines(file2);
+
+        assertEquals("Expected the same data from both appenders", lines1, 
lines2);
+        assertEquals(2, lines1.size());
+        assertTrue(lines1.contains("INFO NestedLoggingFromThrowableMessageTest 
Logging in getMessage "));
+        assertTrue(lines1.contains("ERROR 
NestedLoggingFromThrowableMessageTest Test message"));
+    }
+
+    private static Set<String> readUniqueLines(File input) throws IOException {
+        Set<String> lines = new HashSet<>();
+        BufferedReader reader = new BufferedReader(new InputStreamReader(new 
FileInputStream(input)));
+        try {
+            String line;
+            while ((line = reader.readLine()) != null) {
+                assertTrue("Read duplicate line: " + line, lines.add(line));
+            }
+        } finally {
+            reader.close();
+        }
+        return lines;
+    }
+}

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/38e59a11/log4j-core/src/test/resources/log4j-nested-logging-throwable-message.xml
----------------------------------------------------------------------
diff --git 
a/log4j-core/src/test/resources/log4j-nested-logging-throwable-message.xml 
b/log4j-core/src/test/resources/log4j-nested-logging-throwable-message.xml
new file mode 100644
index 0000000..42ca9e5
--- /dev/null
+++ b/log4j-core/src/test/resources/log4j-nested-logging-throwable-message.xml
@@ -0,0 +1,43 @@
+<?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.
+
+-->
+<Configuration status="OFF" name="NestedLoggingFromThrowableMessageTest">
+
+  <Appenders>
+    <RandomAccessFile name="File1" fileName="target/NestedLoggerTest1.log"
+                      immediateFlush="false" append="false">
+      <PatternLayout>
+        <Pattern>%level %logger{1} %m %throwable{short.message}%n</Pattern>
+      </PatternLayout>
+    </RandomAccessFile>
+    <RandomAccessFile name="File2" fileName="target/NestedLoggerTest2.log"
+                      immediateFlush="false" append="false">
+      <PatternLayout>
+        <Pattern>%level %logger{1} %m %throwable{short.message}%n</Pattern>
+      </PatternLayout>
+    </RandomAccessFile>
+  </Appenders>
+  <Loggers>
+    <Root level="trace">
+      <AppenderRef ref="File1"/>
+    </Root>
+    <Logger name="org"  level="trace">
+      <AppenderRef ref="File2"/>
+    </Logger>
+  </Loggers>
+</Configuration>
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/38e59a11/src/changes/changes.xml
----------------------------------------------------------------------
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 72f1b85..2bbb677 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -204,6 +204,9 @@
         Fixed a memory leak in which ReusableParameterizedMessage would hold a 
reference to the most recently
         logged throwable and provided varargs array.
       </action>
+      <action issue="LOG4J2-2368" dev="ckozak" type="fix">
+        Nested logging doesn't clobber AbstractStringLayout cached 
StringBuidlers
+      </action>
     </release>
     <release version="2.11.1" date="2018-MM-DD" description="GA Release 
2.11.1">
       <action issue="LOG4J2-1721" dev="rgoers" type="update" due-to="Phokham 
Nonava">
@@ -316,6 +319,9 @@
         Fixed a memory leak in which ReusableParameterizedMessage would hold a 
reference to the most recently
         logged throwable and provided varargs array.
       </action>
+      <action issue="LOG4J2-2368" dev="ckozak" type="fix">
+        Nested logging doesn't clobber AbstractStringLayout cached 
StringBuidlers
+      </action>
     </release>
     <release version="2.11.0" date="2018-xx-xx" description="GA Release 
2.11.0">
       <action issue="LOG4J2-2104" dev="rgoers" type="fix">

Reply via email to