You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4j-dev@logging.apache.org by Gary Gregory <ga...@gmail.com> on 2016/04/16 16:26:04 UTC
Fwd: [2/5] logging-log4j2 git commit: LOG4J2-1334 MutableLogEvent
bugfix: don't clear thread name
1) I'm seeing some commits fly by sans unit tests. Are setting our selves
up for regressions?
2) Also, better comments would help (me). For example, this comments
states" THreadName should not be cleared" but why?
3) Typo in capitalization: THreadName
Gary
---------- Forwarded message ----------
From: <rp...@apache.org>
Date: Apr 16, 2016 5:55 AM
Subject: [2/5] logging-log4j2 git commit: LOG4J2-1334 MutableLogEvent
bugfix: don't clear thread name
To: <co...@logging.apache.org>
Cc:
LOG4J2-1334 MutableLogEvent bugfix: don't clear thread name
>
>
> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
> Commit:
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/07cd44a0
> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/07cd44a0
> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/07cd44a0
>
> Branch: refs/heads/master
> Commit: 07cd44a06e4b16d38330d443fc771ada187d537f
> Parents: dc9b6af
> Author: rpopma <rp...@apache.org>
> Authored: Sat Apr 16 21:40:29 2016 +0900
> Committer: rpopma <rp...@apache.org>
> Committed: Sat Apr 16 21:40:29 2016 +0900
>
> ----------------------------------------------------------------------
> .../apache/logging/log4j/core/impl/MutableLogEvent.java | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/07cd44a0/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/MutableLogEvent.java
> ----------------------------------------------------------------------
> diff --git
> a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/MutableLogEvent.java
> b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/MutableLogEvent.java
> index 5330c34..3dce409 100644
> ---
> a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/MutableLogEvent.java
> +++
> b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/MutableLogEvent.java
> @@ -17,6 +17,7 @@ import java.util.Map;
>
> /**
> * Mutable implementation of the {@code LogEvent} interface.
> + * @since 2.6
> */
> public class MutableLogEvent implements LogEvent, ReusableMessage {
> private static final int INITIAL_REUSABLE_MESSAGE_SIZE =
> size("log4j.initialReusableMsgSize", 128);
> @@ -29,18 +30,18 @@ public class MutableLogEvent implements LogEvent,
> ReusableMessage {
> private Level level;
> private String loggerName;
> private Message message;
> - private Throwable thrown;
> private long timeMillis;
> + private Throwable thrown;
> + private ThrowableProxy thrownProxy;
> private Map<String, String> contextMap;
> private ThreadContext.ContextStack contextStack;
> private long threadId;
> private String threadName;
> private int threadPriority;
> + private StackTraceElement source;
> private boolean includeLocation;
> private boolean endOfBatch = false;
> private long nanoTime;
> - private ThrowableProxy thrownProxy;
> - private StackTraceElement source;
> private StringBuilder messageText;
>
> private static int size(final String property, final int
> defaultValue) {
> @@ -87,7 +88,7 @@ public class MutableLogEvent implements LogEvent,
> ReusableMessage {
> source = null;
> contextMap = null;
> contextStack = null;
> - threadName = null;
> + // threadName = null; // THreadName should not be cleared
> // primitive fields that cannot be cleared:
> //timeMillis;
> //threadId;
> @@ -143,7 +144,7 @@ public class MutableLogEvent implements LogEvent,
> ReusableMessage {
>
> public void setMessage(final Message msg) {
> if (msg instanceof ReusableMessage) {
> - ((ReusableMessage) msg).formatTo(getMessageTextForWriting());
> + ((ReusableMessage) msg).formatTo(getMessageTextForWriting());
> // init messageText
> } else {
> // if the Message instance is reused, there is no point in
> freezing its message here
> if (!Constants.FORMAT_MESSAGES_IN_BACKGROUND && msg != null)
> { // LOG4J2-898: user may choose
>
>
Re: [2/5] logging-log4j2 git commit: LOG4J2-1334 MutableLogEvent bugfix: don't clear thread name
Posted by Remko Popma <re...@gmail.com>.
You are right. Unit tests are on the way. I started with fixing the issues brought to light by the existing unit tests, but I still need to add tests for MutableLogEvent and its factory.
About that comment: thread name is initialized once by the factory when the MutableLogEvent is created and stored in a ThreadLocal. It should not be cleared because it may not be set again (still need to implement ThreadName strategy in the factory, but the default is to set it only once).
Sent from my iPhone
> On 2016/04/16, at 23:26, Gary Gregory <ga...@gmail.com> wrote:
>
> 1) I'm seeing some commits fly by sans unit tests. Are setting our selves up for regressions?
>
> 2) Also, better comments would help (me). For example, this comments states" THreadName should not be cleared" but why?
>
> 3) Typo in capitalization: THreadName
>
> Gary
>
> ---------- Forwarded message ----------
> From: <rp...@apache.org>
> Date: Apr 16, 2016 5:55 AM
> Subject: [2/5] logging-log4j2 git commit: LOG4J2-1334 MutableLogEvent bugfix: don't clear thread name
> To: <co...@logging.apache.org>
> Cc:
>
>> LOG4J2-1334 MutableLogEvent bugfix: don't clear thread name
>>
>>
>> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/07cd44a0
>> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/07cd44a0
>> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/07cd44a0
>>
>> Branch: refs/heads/master
>> Commit: 07cd44a06e4b16d38330d443fc771ada187d537f
>> Parents: dc9b6af
>> Author: rpopma <rp...@apache.org>
>> Authored: Sat Apr 16 21:40:29 2016 +0900
>> Committer: rpopma <rp...@apache.org>
>> Committed: Sat Apr 16 21:40:29 2016 +0900
>>
>> ----------------------------------------------------------------------
>> .../apache/logging/log4j/core/impl/MutableLogEvent.java | 11 ++++++-----
>> 1 file changed, 6 insertions(+), 5 deletions(-)
>> ----------------------------------------------------------------------
>>
>>
>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/07cd44a0/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/MutableLogEvent.java
>> ----------------------------------------------------------------------
>> diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/MutableLogEvent.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/MutableLogEvent.java
>> index 5330c34..3dce409 100644
>> --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/MutableLogEvent.java
>> +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/MutableLogEvent.java
>> @@ -17,6 +17,7 @@ import java.util.Map;
>>
>> /**
>> * Mutable implementation of the {@code LogEvent} interface.
>> + * @since 2.6
>> */
>> public class MutableLogEvent implements LogEvent, ReusableMessage {
>> private static final int INITIAL_REUSABLE_MESSAGE_SIZE = size("log4j.initialReusableMsgSize", 128);
>> @@ -29,18 +30,18 @@ public class MutableLogEvent implements LogEvent, ReusableMessage {
>> private Level level;
>> private String loggerName;
>> private Message message;
>> - private Throwable thrown;
>> private long timeMillis;
>> + private Throwable thrown;
>> + private ThrowableProxy thrownProxy;
>> private Map<String, String> contextMap;
>> private ThreadContext.ContextStack contextStack;
>> private long threadId;
>> private String threadName;
>> private int threadPriority;
>> + private StackTraceElement source;
>> private boolean includeLocation;
>> private boolean endOfBatch = false;
>> private long nanoTime;
>> - private ThrowableProxy thrownProxy;
>> - private StackTraceElement source;
>> private StringBuilder messageText;
>>
>> private static int size(final String property, final int defaultValue) {
>> @@ -87,7 +88,7 @@ public class MutableLogEvent implements LogEvent, ReusableMessage {
>> source = null;
>> contextMap = null;
>> contextStack = null;
>> - threadName = null;
>> + // threadName = null; // THreadName should not be cleared
>> // primitive fields that cannot be cleared:
>> //timeMillis;
>> //threadId;
>> @@ -143,7 +144,7 @@ public class MutableLogEvent implements LogEvent, ReusableMessage {
>>
>> public void setMessage(final Message msg) {
>> if (msg instanceof ReusableMessage) {
>> - ((ReusableMessage) msg).formatTo(getMessageTextForWriting());
>> + ((ReusableMessage) msg).formatTo(getMessageTextForWriting()); // init messageText
>> } else {
>> // if the Message instance is reused, there is no point in freezing its message here
>> if (!Constants.FORMAT_MESSAGES_IN_BACKGROUND && msg != null) { // LOG4J2-898: user may choose