LOG4J2-1142 Fix potential memory leak in web applications by using a straight ThreadLocal field instead of subclassing ThreadLocal.
Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/8086bdc6 Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/8086bdc6 Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/8086bdc6 Branch: refs/heads/LOG4J2-1136 Commit: 8086bdc67ec20bf3e7d09a69a889905ad665a707 Parents: dc3f626 Author: rpopma <[email protected]> Authored: Sun Oct 4 01:33:18 2015 +0200 Committer: Ralph Goers <[email protected]> Committed: Sat Oct 3 23:08:06 2015 -0700 ---------------------------------------------------------------------- .../log4j/core/layout/AbstractStringLayout.java | 29 +++++++++++--------- .../log4j/core/layout/CsvLogEventLayout.java | 13 ++++----- .../log4j/core/layout/CsvParameterLayout.java | 13 ++++----- .../logging/log4j/core/layout/GelfLayout.java | 4 +-- .../logging/log4j/core/layout/HtmlLayout.java | 4 +-- .../log4j/core/layout/PatternLayout.java | 8 ++---- .../log4j/core/layout/Rfc5424Layout.java | 5 +--- .../logging/log4j/core/layout/SyslogLayout.java | 4 +-- src/changes/changes.xml | 9 ++++-- 9 files changed, 38 insertions(+), 51 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/8086bdc6/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 a21d11c..cd10881 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 @@ -30,6 +30,9 @@ public abstract class AbstractStringLayout extends AbstractLayout<String> { * Default length for new StringBuilder instances: {@value} . */ protected static final int DEFAULT_STRING_BUILDER_SIZE = 1024; + + private final static ThreadLocal<StringBuilder> threadLocal = new ThreadLocal<>(); + private static final long serialVersionUID = 1L; /** @@ -61,19 +64,19 @@ public abstract class AbstractStringLayout extends AbstractLayout<String> { return null; } - protected static ThreadLocal<StringBuilder> newStringBuilderThreadLocal() { - return new ThreadLocal<StringBuilder>() { - @Override - protected StringBuilder initialValue() { - return new StringBuilder(DEFAULT_STRING_BUILDER_SIZE); - } - }; - } - - protected static StringBuilder prepareStringBuilder(ThreadLocal<StringBuilder> threadLocal) { - final StringBuilder buf = threadLocal.get(); - buf.setLength(0); - return buf; + /** + * Returns a {@code StringBuilder} that this Layout implementation can use to write the formatted log event to. + * + * @return a {@code StringBuilder} + */ + protected StringBuilder getStringBuilder() { + StringBuilder result = threadLocal.get(); + if (result == null) { + result = new StringBuilder(DEFAULT_STRING_BUILDER_SIZE); + threadLocal.set(result); + } + result.setLength(0); + return result; } protected byte[] getBytes(final String s) { http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/8086bdc6/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/CsvLogEventLayout.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/CsvLogEventLayout.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/CsvLogEventLayout.java index 9fb36b0..9b737fe 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/CsvLogEventLayout.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/CsvLogEventLayout.java @@ -41,8 +41,6 @@ import org.apache.logging.log4j.status.StatusLogger; public class CsvLogEventLayout extends AbstractCsvLayout { private static final long serialVersionUID = 1L; - - private static ThreadLocal<StringBuilder> strBuilder = newStringBuilderThreadLocal(); public static CsvLogEventLayout createDefaultLayout() { return new CsvLogEventLayout(Charset.forName(DEFAULT_CHARSET), CSVFormat.valueOf(DEFAULT_FORMAT), null, null); @@ -78,12 +76,11 @@ public class CsvLogEventLayout extends AbstractCsvLayout { @Override public String toSerializable(final LogEvent event) { - final StringBuilder buffer = prepareStringBuilder(strBuilder); - try { - // Revisit when 1.3 is out so that we do not need to create a new - // printer for each event. - // No need to close the printer. - final CSVPrinter printer = new CSVPrinter(buffer, getFormat()); + final StringBuilder buffer = getStringBuilder(); + // Revisit when 1.3 is out so that we do not need to create a new + // printer for each event. + // No need to close the printer. + try (final CSVPrinter printer = new CSVPrinter(buffer, getFormat())) { printer.print(event.getNanoTime()); printer.print(event.getTimeMillis()); printer.print(event.getLevel()); http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/8086bdc6/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/CsvParameterLayout.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/CsvParameterLayout.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/CsvParameterLayout.java index bb0bc6e..fb43964 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/CsvParameterLayout.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/CsvParameterLayout.java @@ -50,8 +50,6 @@ import org.apache.logging.log4j.status.StatusLogger; public class CsvParameterLayout extends AbstractCsvLayout { private static final long serialVersionUID = 1L; - - private static ThreadLocal<StringBuilder> strBuilder = newStringBuilderThreadLocal(); public static AbstractCsvLayout createDefaultLayout() { return new CsvParameterLayout(Charset.forName(DEFAULT_CHARSET), CSVFormat.valueOf(DEFAULT_FORMAT), null, null); @@ -89,12 +87,11 @@ public class CsvParameterLayout extends AbstractCsvLayout { public String toSerializable(final LogEvent event) { final Message message = event.getMessage(); final Object[] parameters = message.getParameters(); - final StringBuilder buffer = prepareStringBuilder(strBuilder); - try { - // Revisit when 1.3 is out so that we do not need to create a new - // printer for each event. - // No need to close the printer. - final CSVPrinter printer = new CSVPrinter(buffer, getFormat()); + final StringBuilder buffer = getStringBuilder(); + // Revisit when 1.3 is out so that we do not need to create a new + // printer for each event. + // No need to close the printer. + try (final CSVPrinter printer = new CSVPrinter(buffer, getFormat())) { printer.printRecord(parameters); return buffer.toString(); } catch (final IOException e) { http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/8086bdc6/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/GelfLayout.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/GelfLayout.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/GelfLayout.java index be71c29..a19af64 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/GelfLayout.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/GelfLayout.java @@ -104,8 +104,6 @@ public final class GelfLayout extends AbstractStringLayout { private static final long serialVersionUID = 1L; private static final BigDecimal TIME_DIVISOR = new BigDecimal(1000); - private static ThreadLocal<StringBuilder> strBuilder = newStringBuilderThreadLocal(); - private final KeyValuePair[] additionalFields; private final int compressionThreshold; private final CompressionType compressionType; @@ -188,7 +186,7 @@ public final class GelfLayout extends AbstractStringLayout { @Override public String toSerializable(final LogEvent event) { - final StringBuilder builder = prepareStringBuilder(strBuilder); + final StringBuilder builder = getStringBuilder(); final JsonStringEncoder jsonEncoder = JsonStringEncoder.getInstance(); builder.append('{'); builder.append("\"version\":\"1.1\","); http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/8086bdc6/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/HtmlLayout.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/HtmlLayout.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/HtmlLayout.java index c80d560..a05f85e 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/HtmlLayout.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/HtmlLayout.java @@ -61,8 +61,6 @@ public final class HtmlLayout extends AbstractStringLayout { private static final String DEFAULT_TITLE = "Log4j Log Messages"; private static final String DEFAULT_CONTENT_TYPE = "text/html"; - private static ThreadLocal<StringBuilder> strBuilder = newStringBuilderThreadLocal(); - private final long jvmStartTime = ManagementFactory.getRuntimeMXBean().getStartTime(); // Print no location info by default @@ -128,7 +126,7 @@ public final class HtmlLayout extends AbstractStringLayout { */ @Override public String toSerializable(final LogEvent event) { - final StringBuilder sbuf = prepareStringBuilder(strBuilder); + final StringBuilder sbuf = getStringBuilder(); sbuf.append(Constants.LINE_SEPARATOR).append("<tr>").append(Constants.LINE_SEPARATOR); http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/8086bdc6/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/PatternLayout.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/PatternLayout.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/PatternLayout.java index 12207f4..9874af0 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/PatternLayout.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/PatternLayout.java @@ -81,8 +81,6 @@ public final class PatternLayout extends AbstractStringLayout { private static final long serialVersionUID = 1L; - private static ThreadLocal<StringBuilder> strBuilder = newStringBuilderThreadLocal(); - /** * Initial converter for pattern. */ @@ -284,8 +282,7 @@ public final class PatternLayout extends AbstractStringLayout { private class PatternSerializer implements Serializer { @Override public String toSerializable(final LogEvent event) { - final StringBuilder buf = strBuilder.get(); - buf.setLength(0); + final StringBuilder buf = getStringBuilder(); final int len = formatters.length; for (int i = 0; i < len; i++) { formatters[i].format(event, buf); @@ -301,8 +298,7 @@ public final class PatternLayout extends AbstractStringLayout { private class PatternSelectorSerializer implements Serializer { @Override public String toSerializable(final LogEvent event) { - final StringBuilder buf = strBuilder.get(); - buf.setLength(0); + final StringBuilder buf = getStringBuilder(); PatternFormatter[] formatters = patternSelector.getFormatters(event); final int len = formatters.length; for (int i = 0; i < len; i++) { http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/8086bdc6/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/Rfc5424Layout.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/Rfc5424Layout.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/Rfc5424Layout.java index 5775a86..2009161 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/Rfc5424Layout.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/Rfc5424Layout.java @@ -93,11 +93,8 @@ public final class Rfc5424Layout extends AbstractStringLayout { private static final int THREE_DIGITS = 100; private static final int MILLIS_PER_MINUTE = 60000; private static final int MINUTES_PER_HOUR = 60; - private static final String COMPONENT_KEY = "RFC5424-Converter"; - private static ThreadLocal<StringBuilder> strBuilder = newStringBuilderThreadLocal(); - private final Facility facility; private final String defaultId; private final int enterpriseNumber; @@ -267,7 +264,7 @@ public final class Rfc5424Layout extends AbstractStringLayout { */ @Override public String toSerializable(final LogEvent event) { - final StringBuilder buf = prepareStringBuilder(strBuilder); + final StringBuilder buf = getStringBuilder(); appendPriority(buf, event.getLevel()); appendTimestamp(buf, event.getTimeMillis()); appendSpace(buf); http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/8086bdc6/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/SyslogLayout.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/SyslogLayout.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/SyslogLayout.java index b1d961d..67c797d 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/SyslogLayout.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/SyslogLayout.java @@ -50,8 +50,6 @@ public final class SyslogLayout extends AbstractStringLayout { private static final long serialVersionUID = 1L; - private static ThreadLocal<StringBuilder> strBuilder = newStringBuilderThreadLocal(); - private final Facility facility; private final boolean includeNewLine; private final String escapeNewLine; @@ -80,7 +78,7 @@ public final class SyslogLayout extends AbstractStringLayout { */ @Override public String toSerializable(final LogEvent event) { - final StringBuilder buf = prepareStringBuilder(strBuilder); + final StringBuilder buf = getStringBuilder(); buf.append('<'); buf.append(Priority.getPriority(facility, event.getLevel())); http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/8086bdc6/src/changes/changes.xml ---------------------------------------------------------------------- diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 0fa5c6e..d6fad6e 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -36,14 +36,17 @@ <action issue="LOG4J2-1146" dev="ggregory" type="add"> Add %notEmpty to PatternLayout to avoid output of patterns where all variables are empty. </action> - <action issue="LOG4J2-1020" dev="mikes" type="fix"> - Possibility to set shutdown timeout on AsyncAppender + <action issue="LOG4J2-1020" dev="mikes" type="add"> + Add possibility to set shutdown timeout on AsyncAppender. + </action> + <action issue="LOG4J2-1142" dev="rpopma" type="fix"> + Fix potential memory leak in web applications by using a straight ThreadLocal field instead of subclassing ThreadLocal. </action> <action issue="LOG4J2-1135" dev="rpopma" type="fix"> Compression on rollover was broken: log file was renamed to .zip but not compressed. </action> <action issue="LOG4J2-1127" dev="ggregory" type="fix"> - log4j2.xml cannot be parsed on Oracle Weblogic 12c + log4j2.xml cannot be parsed on Oracle Weblogic 12c. </action> <action issue="LOG4J2-1132" dev="ggregory" type="fix"> Do not use MongoDB driver 2.13.3 deprecated methods.
