LOG4J2-1489 (GC) Fixed %date conversion patterns with a timezone parameter are now garbage free.
This closes #36 (https://github.com/apache/logging-log4j2/pull/36). Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/0a85a774 Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/0a85a774 Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/0a85a774 Branch: refs/heads/LOG4J-1181 Commit: 0a85a77481e8ac289d36befb16828a99dd47cb96 Parents: 2bca903 Author: rpopma <[email protected]> Authored: Sun Jul 31 15:39:43 2016 +0900 Committer: rpopma <[email protected]> Committed: Sun Jul 31 15:39:43 2016 +0900 ---------------------------------------------------------------------- .../core/pattern/DatePatternConverter.java | 2 +- .../core/util/datetime/FixedDateFormat.java | 65 +++++++++++++++++--- .../core/util/datetime/FixedDateFormatTest.java | 35 ++++++++--- log4j-core/src/test/resources/gcFreeLogging.xml | 9 +-- .../resources/gcFreeMixedSyncAsyncLogging.xml | 10 +-- src/changes/changes.xml | 3 + 6 files changed, 94 insertions(+), 30 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0a85a774/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/DatePatternConverter.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/DatePatternConverter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/DatePatternConverter.java index 499f0d9..19edde5 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/DatePatternConverter.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/DatePatternConverter.java @@ -224,7 +224,7 @@ public final class DatePatternConverter extends LogEventPatternConverter impleme LOGGER.warn("Could not instantiate FastDateFormat with pattern " + pattern, e); // default to the DEFAULT format - return createFixedFormatter(FixedDateFormat.create(FixedFormat.DEFAULT)); + return createFixedFormatter(FixedDateFormat.create(FixedFormat.DEFAULT, tz)); } } http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0a85a774/log4j-core/src/main/java/org/apache/logging/log4j/core/util/datetime/FixedDateFormat.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/datetime/FixedDateFormat.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/datetime/FixedDateFormat.java index 25a5127..3b1f102 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/datetime/FixedDateFormat.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/datetime/FixedDateFormat.java @@ -19,6 +19,7 @@ package org.apache.logging.log4j.core.util.datetime; import java.util.Calendar; import java.util.Objects; +import java.util.TimeZone; /** * Custom time formatter that trades flexibility for performance. This formatter only supports the date patterns defined @@ -156,11 +157,23 @@ public class FixedDateFormat { * @return the {@code FastDateFormat} object for formatting the date part of the pattern or {@code null} */ public FastDateFormat getFastDateFormat() { - return getDatePattern() == null ? null : FastDateFormat.getInstance(getDatePattern()); + return getFastDateFormat(null); + } + + /** + * Returns the {@code FastDateFormat} object for formatting the date part of the pattern or {@code null} if the + * pattern does not have a date part. + * + * @param tz the time zone to use + * @return the {@code FastDateFormat} object for formatting the date part of the pattern or {@code null} + */ + public FastDateFormat getFastDateFormat(TimeZone tz) { + return getDatePattern() == null ? null : FastDateFormat.getInstance(getDatePattern(), tz); } } private final FixedFormat fixedFormat; + private final TimeZone timeZone; private final int length; private final FastDateFormat fastDateFormat; // may be null private final char timeSeparatorChar; @@ -186,35 +199,57 @@ public class FixedDateFormat { * * @param fixedFormat the fixed format */ - FixedDateFormat(final FixedFormat fixedFormat) { + FixedDateFormat(final FixedFormat fixedFormat, final TimeZone tz) { this.fixedFormat = Objects.requireNonNull(fixedFormat); + this.timeZone = Objects.requireNonNull(tz); this.timeSeparatorChar = fixedFormat.timeSeparatorChar; this.timeSeparatorLength = fixedFormat.timeSeparatorLength; this.millisSeparatorChar = fixedFormat.millisSeparatorChar; this.millisSeparatorLength = fixedFormat.millisSeparatorLength; this.length = fixedFormat.getLength(); - this.fastDateFormat = fixedFormat.getFastDateFormat(); + this.fastDateFormat = fixedFormat.getFastDateFormat(tz); } public static FixedDateFormat createIfSupported(final String... options) { if (options == null || options.length == 0 || options[0] == null) { - return new FixedDateFormat(FixedFormat.DEFAULT); + return new FixedDateFormat(FixedFormat.DEFAULT, TimeZone.getDefault()); } + final TimeZone tz; if (options.length > 1) { - return null; // time zone not supported + if (options[1] != null){ + tz = TimeZone.getTimeZone(options[1]); + } else { + tz = TimeZone.getDefault(); + } + } else if (options.length > 2) { + return null; + } else { + tz = TimeZone.getDefault(); } + final FixedFormat type = FixedFormat.lookup(options[0]); - return type == null ? null : new FixedDateFormat(type); + return type == null ? null : new FixedDateFormat(type, tz); } /** - * Returns a new {@code FixedDateFormat} object for the specified {@code FixedFormat} and a {@code null} TimeZone. + * Returns a new {@code FixedDateFormat} object for the specified {@code FixedFormat} and a {@code TimeZone.getDefault()} TimeZone. * * @param format the format to use * @return a new {@code FixedDateFormat} object */ public static FixedDateFormat create(final FixedFormat format) { - return new FixedDateFormat(format); + return new FixedDateFormat(format, TimeZone.getDefault()); + } + + /** + * Returns a new {@code FixedDateFormat} object for the specified {@code FixedFormat} and TimeZone. + * + * @param format the format to use + * @param tz the time zone to use + * @return a new {@code FixedDateFormat} object + */ + public static FixedDateFormat create(final FixedFormat format, final TimeZone tz) { + return new FixedDateFormat(format, tz != null ? tz : TimeZone.getDefault()); } /** @@ -226,6 +261,16 @@ public class FixedDateFormat { return fixedFormat.getPattern(); } + /** + * Returns the time zone. + * + * @return the time zone + */ + + public TimeZone getTimeZone() { + return timeZone; + } + // Profiling showed this method is important to log4j performance. Modify with care! // 30 bytes (allows immediate JVM inlining: <= -XX:MaxInlineSize=35 bytes) private long millisSinceMidnight(final long now) { @@ -243,8 +288,8 @@ public class FixedDateFormat { midnightTomorrow = calcMidnightMillis(now, 1); } - static long calcMidnightMillis(final long time, final int addDays) { - final Calendar cal = Calendar.getInstance(); + private long calcMidnightMillis(final long time, final int addDays) { + final Calendar cal = Calendar.getInstance(timeZone); cal.setTimeInMillis(time); cal.set(Calendar.HOUR_OF_DAY, 0); cal.set(Calendar.MINUTE, 0); http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0a85a774/log4j-core/src/test/java/org/apache/logging/log4j/core/util/datetime/FixedDateFormatTest.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/util/datetime/FixedDateFormatTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/util/datetime/FixedDateFormatTest.java index 0946d73..571e901 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/util/datetime/FixedDateFormatTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/util/datetime/FixedDateFormatTest.java @@ -20,6 +20,7 @@ package org.apache.logging.log4j.core.util.datetime; import java.text.SimpleDateFormat; import java.util.Date; import java.util.Locale; +import java.util.TimeZone; import java.util.concurrent.TimeUnit; import org.apache.logging.log4j.core.util.datetime.FixedDateFormat.FixedFormat; @@ -114,22 +115,36 @@ public class FixedDateFormatTest { public void testCreateIfSupported_defaultIfOptionsArrayWithSingleNullElement() { final FixedDateFormat fmt = FixedDateFormat.createIfSupported(new String[1]); assertEquals(FixedFormat.DEFAULT.getPattern(), fmt.getFormat()); + assertEquals(TimeZone.getDefault(), fmt.getTimeZone()); } @Test - public void testCreateIfSupported_nullIfOptionsArrayHasTwoElements() { - final String[] options = {FixedDateFormat.FixedFormat.ABSOLUTE.getPattern(), "+08:00"}; - assertNull("timezone", FixedDateFormat.createIfSupported(options)); + public void testCreateIfSupported_defaultTimeZoneIfOptionsArrayWithSecondNullElement() { + final FixedDateFormat fmt = FixedDateFormat.createIfSupported(new String[] {FixedFormat.DEFAULT.getPattern(), null, ""}); + assertEquals(FixedFormat.DEFAULT.getPattern(), fmt.getFormat()); + assertEquals(TimeZone.getDefault(), fmt.getTimeZone()); + } + + @Test + public void testCreateIfSupported_customTimeZoneIfOptionsArrayWithTimeZoneElement() { + final FixedDateFormat fmt = FixedDateFormat.createIfSupported(new String[] {FixedFormat.DEFAULT.getPattern(), "+08:00", ""}); + assertEquals(FixedFormat.DEFAULT.getPattern(), fmt.getFormat()); + assertEquals(TimeZone.getTimeZone("+08:00"), fmt.getTimeZone()); + } + + @Test(expected = NullPointerException.class) + public void testConstructorDisallowsNullFormat() { + new FixedDateFormat(null, TimeZone.getDefault()); } @Test(expected = NullPointerException.class) - public void testConstructorDisallowsNull() { - new FixedDateFormat(null); + public void testConstructorDisallowsNullTimeZone() { + new FixedDateFormat(FixedFormat.ABSOLUTE, null); } @Test public void testGetFormatReturnsConstructorFixedFormatPattern() { - final FixedDateFormat format = new FixedDateFormat(FixedDateFormat.FixedFormat.ABSOLUTE); + final FixedDateFormat format = new FixedDateFormat(FixedDateFormat.FixedFormat.ABSOLUTE, TimeZone.getDefault()); assertSame(FixedDateFormat.FixedFormat.ABSOLUTE.getPattern(), format.getFormat()); } @@ -140,7 +155,7 @@ public class FixedDateFormatTest { final long end = now + TimeUnit.HOURS.toMillis(25); for (final FixedFormat format : FixedFormat.values()) { final SimpleDateFormat simpleDF = new SimpleDateFormat(format.getPattern(), Locale.getDefault()); - final FixedDateFormat customTF = new FixedDateFormat(format); + final FixedDateFormat customTF = new FixedDateFormat(format, TimeZone.getDefault()); for (long time = start; time < end; time += 12345) { final String actual = customTF.format(time); final String expected = simpleDF.format(new Date(time)); @@ -156,7 +171,7 @@ public class FixedDateFormatTest { final long end = now + TimeUnit.HOURS.toMillis(25); for (final FixedFormat format : FixedFormat.values()) { final SimpleDateFormat simpleDF = new SimpleDateFormat(format.getPattern(), Locale.getDefault()); - final FixedDateFormat customTF = new FixedDateFormat(format); + final FixedDateFormat customTF = new FixedDateFormat(format, TimeZone.getDefault()); for (long time = end; time > start; time -= 12345) { final String actual = customTF.format(time); final String expected = simpleDF.format(new Date(time)); @@ -173,7 +188,7 @@ public class FixedDateFormatTest { final char[] buffer = new char[128]; for (final FixedFormat format : FixedFormat.values()) { final SimpleDateFormat simpleDF = new SimpleDateFormat(format.getPattern(), Locale.getDefault()); - final FixedDateFormat customTF = new FixedDateFormat(format); + final FixedDateFormat customTF = new FixedDateFormat(format, TimeZone.getDefault()); for (long time = start; time < end; time += 12345) { final int length = customTF.format(time, buffer, 23); final String actual = new String(buffer, 23, length); @@ -191,7 +206,7 @@ public class FixedDateFormatTest { final char[] buffer = new char[128]; for (final FixedFormat format : FixedFormat.values()) { final SimpleDateFormat simpleDF = new SimpleDateFormat(format.getPattern(), Locale.getDefault()); - final FixedDateFormat customTF = new FixedDateFormat(format); + final FixedDateFormat customTF = new FixedDateFormat(format, TimeZone.getDefault()); for (long time = end; time > start; time -= 12345) { final int length = customTF.format(time, buffer, 23); final String actual = new String(buffer, 23, length); http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0a85a774/log4j-core/src/test/resources/gcFreeLogging.xml ---------------------------------------------------------------------- diff --git a/log4j-core/src/test/resources/gcFreeLogging.xml b/log4j-core/src/test/resources/gcFreeLogging.xml index 8893466..97fc345 100644 --- a/log4j-core/src/test/resources/gcFreeLogging.xml +++ b/log4j-core/src/test/resources/gcFreeLogging.xml @@ -6,13 +6,13 @@ </Console> <File name="File" fileName="target/gcfreefile.log" bufferedIO="false"> <PatternLayout> - <Pattern>%highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %m%n}</Pattern> + <Pattern>%d{DEFAULT}{UTC} %p %c{1.} [%t] %m%n %highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %m%n}</Pattern> </PatternLayout> </File> <RollingFile name="RollingFile" fileName="target/gcfreeRollingFile.log" filePattern="target/gcfree-%d{MM-dd-yy-HH-mm-ss}.log.gz"> <PatternLayout> - <Pattern>%highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %m%n}</Pattern> + <Pattern>%d{DEFAULT}{UTC} %p %c{1.} [%t] %m%n %highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %m%n}</Pattern> </PatternLayout> <Policies> <SizeBasedTriggeringPolicy size="50M" /> @@ -20,7 +20,7 @@ </RollingFile> <RandomAccessFile name="RandomAccessFile" fileName="target/gcfreeRAF.log" immediateFlush="false" append="false"> <PatternLayout> - <Pattern>%highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %X{aKey} %m %ex%n}</Pattern> + <Pattern>%d{DEFAULT}{UTC} %p %c{1.} [%t] %X{aKey} %m %ex%n %highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %X{aKey} %m %ex%n}</Pattern> </PatternLayout> </RandomAccessFile> <RollingRandomAccessFile name="RollingRandomAccessFile" @@ -28,7 +28,7 @@ filePattern="target/afterRollover-%i.log" append="false" immediateFlush="false"> <PatternLayout> - <Pattern>%highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %X{aKey} %m %location %ex%n}</Pattern> + <Pattern>%d{DEFAULT}{UTC} %p %c{1.} [%t] %X{aKey} %m %location %ex%n %highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %X{aKey} %m %location %ex%n}</Pattern> </PatternLayout> <Policies> <SizeBasedTriggeringPolicy size="50 M"/> @@ -38,6 +38,7 @@ fileName="target/gcfreemmap.log" immediateFlush="false" append="false"> <PatternLayout> + <Pattern>%d{DEFAULT}{UTC} %p %c{1.} [%t] %X{aKey} %m%ex%n</Pattern> <Pattern>%highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %X{aKey} %m%ex%n}</Pattern> </PatternLayout> </MemoryMappedFile> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0a85a774/log4j-core/src/test/resources/gcFreeMixedSyncAsyncLogging.xml ---------------------------------------------------------------------- diff --git a/log4j-core/src/test/resources/gcFreeMixedSyncAsyncLogging.xml b/log4j-core/src/test/resources/gcFreeMixedSyncAsyncLogging.xml index 50bff57..6944e36 100644 --- a/log4j-core/src/test/resources/gcFreeMixedSyncAsyncLogging.xml +++ b/log4j-core/src/test/resources/gcFreeMixedSyncAsyncLogging.xml @@ -6,13 +6,13 @@ </Console> <File name="File" fileName="target/gcfreefileMixed.log" bufferedIO="false"> <PatternLayout> - <Pattern>%highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %m%n}</Pattern> + <Pattern>%d{DEFAULT}{UTC} %p %c{1.} [%t] %m%n %highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %m%n}</Pattern> </PatternLayout> </File> <RollingFile name="RollingFile" fileName="target/gcfreeRollingFileMixed.log" filePattern="target/gcfree-%d{MM-dd-yy-HH-mm-ss}.log.gz"> <PatternLayout> - <Pattern>%highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %m%n}</Pattern> + <Pattern>%d{DEFAULT}{UTC} %p %c{1.} [%t] %m%n %highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %m%n}</Pattern> </PatternLayout> <Policies> <SizeBasedTriggeringPolicy size="50M" /> @@ -20,7 +20,7 @@ </RollingFile> <RandomAccessFile name="RandomAccessFile" fileName="target/gcfreeRAFMixed.log" immediateFlush="false" append="false"> <PatternLayout> - <Pattern>%highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %X{aKey} %m %ex%n}</Pattern> + <Pattern>%d{DEFAULT}{UTC} %p %c{1.} [%t] %X{aKey} %m %ex%n %highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %X{aKey} %m %ex%n}</Pattern> </PatternLayout> </RandomAccessFile> <RollingRandomAccessFile name="RollingRandomAccessFile" @@ -28,7 +28,7 @@ filePattern="target/afterRollover-%i.log" append="false" immediateFlush="false"> <PatternLayout> - <Pattern>%highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %X{aKey} %m %location %ex%n}</Pattern> + <Pattern>%d{DEFAULT}{UTC} %p %c{1.} [%t] %X{aKey} %m %location %ex%n %highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %X{aKey} %m %location %ex%n}</Pattern> </PatternLayout> <Policies> <SizeBasedTriggeringPolicy size="50 M"/> @@ -38,7 +38,7 @@ fileName="target/gcfreemmapMixed.log" immediateFlush="false" append="false"> <PatternLayout> - <Pattern>%highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %X{aKey} %m%ex%n}</Pattern> + <Pattern>%d{DEFAULT}{UTC} %p %c{1.} [%t] %X{aKey} %m%ex%n %highlight{%style{%d}{bright,cyan} %p %c{1.} [%t] %X{aKey} %m%ex%n}</Pattern> </PatternLayout> </MemoryMappedFile> <RandomAccessFile name="RandomAccessFileGelf" fileName="target/gcfreeMixed.json" immediateFlush="false" append="false"> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0a85a774/src/changes/changes.xml ---------------------------------------------------------------------- diff --git a/src/changes/changes.xml b/src/changes/changes.xml index cbb8466..7e44b1b 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -24,6 +24,9 @@ </properties> <body> <release version="2.7" date="2016-MM-DD" description="GA Release 2.7"> + <action issue="LOG4J2-1489" dev="rpopma" type="fix" due-to="Richard Zschech"> + (GC) Fixed %date conversion patterns with a timezone parameter are now garbage free. + </action> <action issue="LOG4J2-1279" dev="rpopma" type="fix" due-to="Tony Baines"> Prevent NullPointerException in FastDateParser$TimeZoneStrategy. </action>
