You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by rp...@apache.org on 2016/04/16 14:55:02 UTC
[3/5] logging-log4j2 git commit: LOG4J2-1334 ListAppender must add
snapshot of MutableLogEvent to the list,
not the MutableLogEvent itself (since it will change)
LOG4J2-1334 ListAppender must add snapshot of MutableLogEvent to the list, not the MutableLogEvent itself (since it will change)
Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/3f395f63
Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/3f395f63
Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/3f395f63
Branch: refs/heads/master
Commit: 3f395f63d03b603ef628e214d3f01dd226793baf
Parents: 07cd44a
Author: rpopma <rp...@apache.org>
Authored: Sat Apr 16 21:41:40 2016 +0900
Committer: rpopma <rp...@apache.org>
Committed: Sat Apr 16 21:41:40 2016 +0900
----------------------------------------------------------------------
.../apache/logging/log4j/test/appender/ListAppender.java | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/3f395f63/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/ListAppender.java
----------------------------------------------------------------------
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/ListAppender.java b/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/ListAppender.java
index 97ca15d..19aeaee 100644
--- a/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/ListAppender.java
+++ b/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/ListAppender.java
@@ -31,6 +31,8 @@ import org.apache.logging.log4j.core.config.plugins.PluginAttribute;
import org.apache.logging.log4j.core.config.plugins.PluginElement;
import org.apache.logging.log4j.core.config.plugins.PluginFactory;
import org.apache.logging.log4j.core.config.plugins.validation.constraints.Required;
+import org.apache.logging.log4j.core.impl.Log4jLogEvent;
+import org.apache.logging.log4j.core.impl.MutableLogEvent;
import org.apache.logging.log4j.core.layout.SerializedLayout;
/**
@@ -81,7 +83,12 @@ public class ListAppender extends AbstractAppender {
public synchronized void append(final LogEvent event) {
final Layout<? extends Serializable> layout = getLayout();
if (layout == null) {
- events.add(event);
+ if (event instanceof MutableLogEvent) {
+ // must take snapshot or subsequent calls to logger.log() will modify this event
+ events.add(Log4jLogEvent.deserialize(Log4jLogEvent.serialize(event, event.isIncludeLocation())));
+ } else {
+ events.add(event);
+ }
} else if (layout instanceof SerializedLayout) {
final byte[] header = layout.getHeader();
final byte[] content = layout.toByteArray(event);
Re: [3/5] logging-log4j2 git commit: LOG4J2-1334 ListAppender must add snapshot of MutableLogEvent to the list, not the MutableLogEvent itself (since it will change)
Posted by Remko Popma <re...@gmail.com>.
I hadn't thought of that. In this case though not all attributes are immutable so going through Log4jLogEvent.Proxy may be prudent. (In spite of the name, Java io serialization is not involved here. )
Sent from my iPhone
> On 2016/04/18, at 19:54, Mikael Ståldal <mi...@magine.com> wrote:
>
> Isn't this what clone() should be used for?
>
>> On Sat, Apr 16, 2016 at 10:51 PM, Remko Popma <re...@gmail.com> wrote:
>> Makes sense.
>>
>> Sent from my iPhone
>>
>>> On 2016/04/16, at 23:20, Gary Gregory <ga...@gmail.com> wrote:
>>>
>>> Hi,
>>>
>>> The ser+deser sequence feels like something that should be refactored into a serializedCopy() or deepCopy() method depending on whether or not you want to publicize the copying technique.
>>>
>>> Gary
>>>
>>> ---------- Forwarded message ----------
>>> From: <rp...@apache.org>
>>> Date: Apr 16, 2016 5:55 AM
>>> Subject: [3/5] logging-log4j2 git commit: LOG4J2-1334 ListAppender must add snapshot of MutableLogEvent to the list, not the MutableLogEvent itself (since it will change)
>>> To: <co...@logging.apache.org>
>>> Cc:
>>>
>>>> LOG4J2-1334 ListAppender must add snapshot of MutableLogEvent to the list, not the MutableLogEvent itself (since it will change)
>>>>
>>>>
>>>> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
>>>> Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/3f395f63
>>>> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/3f395f63
>>>> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/3f395f63
>>>>
>>>> Branch: refs/heads/master
>>>> Commit: 3f395f63d03b603ef628e214d3f01dd226793baf
>>>> Parents: 07cd44a
>>>> Author: rpopma <rp...@apache.org>
>>>> Authored: Sat Apr 16 21:41:40 2016 +0900
>>>> Committer: rpopma <rp...@apache.org>
>>>> Committed: Sat Apr 16 21:41:40 2016 +0900
>>>>
>>>> ----------------------------------------------------------------------
>>>> .../apache/logging/log4j/test/appender/ListAppender.java | 9 ++++++++-
>>>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>>> ----------------------------------------------------------------------
>>>>
>>>>
>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/3f395f63/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/ListAppender.java
>>>> ----------------------------------------------------------------------
>>>> diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/ListAppender.java b/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/ListAppender.java
>>>> index 97ca15d..19aeaee 100644
>>>> --- a/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/ListAppender.java
>>>> +++ b/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/ListAppender.java
>>>> @@ -31,6 +31,8 @@ import org.apache.logging.log4j.core.config.plugins.PluginAttribute;
>>>> import org.apache.logging.log4j.core.config.plugins.PluginElement;
>>>> import org.apache.logging.log4j.core.config.plugins.PluginFactory;
>>>> import org.apache.logging.log4j.core.config.plugins.validation.constraints.Required;
>>>> +import org.apache.logging.log4j.core.impl.Log4jLogEvent;
>>>> +import org.apache.logging.log4j.core.impl.MutableLogEvent;
>>>> import org.apache.logging.log4j.core.layout.SerializedLayout;
>>>>
>>>> /**
>>>> @@ -81,7 +83,12 @@ public class ListAppender extends AbstractAppender {
>>>> public synchronized void append(final LogEvent event) {
>>>> final Layout<? extends Serializable> layout = getLayout();
>>>> if (layout == null) {
>>>> - events.add(event);
>>>> + if (event instanceof MutableLogEvent) {
>>>> + // must take snapshot or subsequent calls to logger.log() will modify this event
>>>> + events.add(Log4jLogEvent.deserialize(Log4jLogEvent.serialize(event, event.isIncludeLocation())));
>>>> + } else {
>>>> + events.add(event);
>>>> + }
>>>> } else if (layout instanceof SerializedLayout) {
>>>> final byte[] header = layout.getHeader();
>>>> final byte[] content = layout.toByteArray(event);
>
>
>
> --
>
>
> Mikael Ståldal
> Senior software developer
>
> Magine TV
> mikael.staldal@magine.com
> Grev Turegatan 3 | 114 46 Stockholm, Sweden | www.magine.com
>
> Privileged and/or Confidential Information may be contained in this message. If you are not the addressee indicated in this message
> (or responsible for delivery of the message to such a person), you may not copy or deliver this message to anyone. In such case,
> you should destroy this message and kindly notify the sender by reply email.
Re: [3/5] logging-log4j2 git commit: LOG4J2-1334 ListAppender must
add snapshot of MutableLogEvent to the list, not the MutableLogEvent itself
(since it will change)
Posted by Mikael Ståldal <mi...@magine.com>.
Isn't this what clone() should be used for?
On Sat, Apr 16, 2016 at 10:51 PM, Remko Popma <re...@gmail.com> wrote:
> Makes sense.
>
> Sent from my iPhone
>
> On 2016/04/16, at 23:20, Gary Gregory <ga...@gmail.com> wrote:
>
> Hi,
>
> The ser+deser sequence feels like something that should be refactored into
> a serializedCopy() or deepCopy() method depending on whether or not you
> want to publicize the copying technique.
>
> Gary
> ---------- Forwarded message ----------
> From: <rp...@apache.org>
> Date: Apr 16, 2016 5:55 AM
> Subject: [3/5] logging-log4j2 git commit: LOG4J2-1334 ListAppender must
> add snapshot of MutableLogEvent to the list, not the MutableLogEvent itself
> (since it will change)
> To: <co...@logging.apache.org>
> Cc:
>
> LOG4J2-1334 ListAppender must add snapshot of MutableLogEvent to the list,
>> not the MutableLogEvent itself (since it will change)
>>
>>
>> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
>> Commit:
>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/3f395f63
>> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/3f395f63
>> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/3f395f63
>>
>> Branch: refs/heads/master
>> Commit: 3f395f63d03b603ef628e214d3f01dd226793baf
>> Parents: 07cd44a
>> Author: rpopma <rp...@apache.org>
>> Authored: Sat Apr 16 21:41:40 2016 +0900
>> Committer: rpopma <rp...@apache.org>
>> Committed: Sat Apr 16 21:41:40 2016 +0900
>>
>> ----------------------------------------------------------------------
>> .../apache/logging/log4j/test/appender/ListAppender.java | 9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>> ----------------------------------------------------------------------
>>
>>
>>
>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/3f395f63/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/ListAppender.java
>> ----------------------------------------------------------------------
>> diff --git
>> a/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/ListAppender.java
>> b/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/ListAppender.java
>> index 97ca15d..19aeaee 100644
>> ---
>> a/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/ListAppender.java
>> +++
>> b/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/ListAppender.java
>> @@ -31,6 +31,8 @@ import
>> org.apache.logging.log4j.core.config.plugins.PluginAttribute;
>> import org.apache.logging.log4j.core.config.plugins.PluginElement;
>> import org.apache.logging.log4j.core.config.plugins.PluginFactory;
>> import
>> org.apache.logging.log4j.core.config.plugins.validation.constraints.Required;
>> +import org.apache.logging.log4j.core.impl.Log4jLogEvent;
>> +import org.apache.logging.log4j.core.impl.MutableLogEvent;
>> import org.apache.logging.log4j.core.layout.SerializedLayout;
>>
>> /**
>> @@ -81,7 +83,12 @@ public class ListAppender extends AbstractAppender {
>> public synchronized void append(final LogEvent event) {
>> final Layout<? extends Serializable> layout = getLayout();
>> if (layout == null) {
>> - events.add(event);
>> + if (event instanceof MutableLogEvent) {
>> + // must take snapshot or subsequent calls to
>> logger.log() will modify this event
>> +
>> events.add(Log4jLogEvent.deserialize(Log4jLogEvent.serialize(event,
>> event.isIncludeLocation())));
>> + } else {
>> + events.add(event);
>> + }
>> } else if (layout instanceof SerializedLayout) {
>> final byte[] header = layout.getHeader();
>> final byte[] content = layout.toByteArray(event);
>>
>>
--
[image: MagineTV]
*Mikael Ståldal*
Senior software developer
*Magine TV*
mikael.staldal@magine.com
Grev Turegatan 3 | 114 46 Stockholm, Sweden | www.magine.com
Privileged and/or Confidential Information may be contained in this
message. If you are not the addressee indicated in this message
(or responsible for delivery of the message to such a person), you may not
copy or deliver this message to anyone. In such case,
you should destroy this message and kindly notify the sender by reply
email.
Re: [3/5] logging-log4j2 git commit: LOG4J2-1334 ListAppender must add snapshot of MutableLogEvent to the list, not the MutableLogEvent itself (since it will change)
Posted by Remko Popma <re...@gmail.com>.
Makes sense.
Sent from my iPhone
> On 2016/04/16, at 23:20, Gary Gregory <ga...@gmail.com> wrote:
>
> Hi,
>
> The ser+deser sequence feels like something that should be refactored into a serializedCopy() or deepCopy() method depending on whether or not you want to publicize the copying technique.
>
> Gary
>
> ---------- Forwarded message ----------
> From: <rp...@apache.org>
> Date: Apr 16, 2016 5:55 AM
> Subject: [3/5] logging-log4j2 git commit: LOG4J2-1334 ListAppender must add snapshot of MutableLogEvent to the list, not the MutableLogEvent itself (since it will change)
> To: <co...@logging.apache.org>
> Cc:
>
>> LOG4J2-1334 ListAppender must add snapshot of MutableLogEvent to the list, not the MutableLogEvent itself (since it will change)
>>
>>
>> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/3f395f63
>> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/3f395f63
>> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/3f395f63
>>
>> Branch: refs/heads/master
>> Commit: 3f395f63d03b603ef628e214d3f01dd226793baf
>> Parents: 07cd44a
>> Author: rpopma <rp...@apache.org>
>> Authored: Sat Apr 16 21:41:40 2016 +0900
>> Committer: rpopma <rp...@apache.org>
>> Committed: Sat Apr 16 21:41:40 2016 +0900
>>
>> ----------------------------------------------------------------------
>> .../apache/logging/log4j/test/appender/ListAppender.java | 9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>> ----------------------------------------------------------------------
>>
>>
>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/3f395f63/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/ListAppender.java
>> ----------------------------------------------------------------------
>> diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/ListAppender.java b/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/ListAppender.java
>> index 97ca15d..19aeaee 100644
>> --- a/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/ListAppender.java
>> +++ b/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/ListAppender.java
>> @@ -31,6 +31,8 @@ import org.apache.logging.log4j.core.config.plugins.PluginAttribute;
>> import org.apache.logging.log4j.core.config.plugins.PluginElement;
>> import org.apache.logging.log4j.core.config.plugins.PluginFactory;
>> import org.apache.logging.log4j.core.config.plugins.validation.constraints.Required;
>> +import org.apache.logging.log4j.core.impl.Log4jLogEvent;
>> +import org.apache.logging.log4j.core.impl.MutableLogEvent;
>> import org.apache.logging.log4j.core.layout.SerializedLayout;
>>
>> /**
>> @@ -81,7 +83,12 @@ public class ListAppender extends AbstractAppender {
>> public synchronized void append(final LogEvent event) {
>> final Layout<? extends Serializable> layout = getLayout();
>> if (layout == null) {
>> - events.add(event);
>> + if (event instanceof MutableLogEvent) {
>> + // must take snapshot or subsequent calls to logger.log() will modify this event
>> + events.add(Log4jLogEvent.deserialize(Log4jLogEvent.serialize(event, event.isIncludeLocation())));
>> + } else {
>> + events.add(event);
>> + }
>> } else if (layout instanceof SerializedLayout) {
>> final byte[] header = layout.getHeader();
>> final byte[] content = layout.toByteArray(event);
Fwd: [3/5] logging-log4j2 git commit: LOG4J2-1334 ListAppender must
add snapshot of MutableLogEvent to the list, not the MutableLogEvent itself
(since it will change)
Posted by Gary Gregory <ga...@gmail.com>.
Hi,
The ser+deser sequence feels like something that should be refactored into
a serializedCopy() or deepCopy() method depending on whether or not you
want to publicize the copying technique.
Gary
---------- Forwarded message ----------
From: <rp...@apache.org>
Date: Apr 16, 2016 5:55 AM
Subject: [3/5] logging-log4j2 git commit: LOG4J2-1334 ListAppender must add
snapshot of MutableLogEvent to the list, not the MutableLogEvent itself
(since it will change)
To: <co...@logging.apache.org>
Cc:
LOG4J2-1334 ListAppender must add snapshot of MutableLogEvent to the list,
> not the MutableLogEvent itself (since it will change)
>
>
> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
> Commit:
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/3f395f63
> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/3f395f63
> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/3f395f63
>
> Branch: refs/heads/master
> Commit: 3f395f63d03b603ef628e214d3f01dd226793baf
> Parents: 07cd44a
> Author: rpopma <rp...@apache.org>
> Authored: Sat Apr 16 21:41:40 2016 +0900
> Committer: rpopma <rp...@apache.org>
> Committed: Sat Apr 16 21:41:40 2016 +0900
>
> ----------------------------------------------------------------------
> .../apache/logging/log4j/test/appender/ListAppender.java | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/3f395f63/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/ListAppender.java
> ----------------------------------------------------------------------
> diff --git
> a/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/ListAppender.java
> b/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/ListAppender.java
> index 97ca15d..19aeaee 100644
> ---
> a/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/ListAppender.java
> +++
> b/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/ListAppender.java
> @@ -31,6 +31,8 @@ import
> org.apache.logging.log4j.core.config.plugins.PluginAttribute;
> import org.apache.logging.log4j.core.config.plugins.PluginElement;
> import org.apache.logging.log4j.core.config.plugins.PluginFactory;
> import
> org.apache.logging.log4j.core.config.plugins.validation.constraints.Required;
> +import org.apache.logging.log4j.core.impl.Log4jLogEvent;
> +import org.apache.logging.log4j.core.impl.MutableLogEvent;
> import org.apache.logging.log4j.core.layout.SerializedLayout;
>
> /**
> @@ -81,7 +83,12 @@ public class ListAppender extends AbstractAppender {
> public synchronized void append(final LogEvent event) {
> final Layout<? extends Serializable> layout = getLayout();
> if (layout == null) {
> - events.add(event);
> + if (event instanceof MutableLogEvent) {
> + // must take snapshot or subsequent calls to logger.log()
> will modify this event
> +
> events.add(Log4jLogEvent.deserialize(Log4jLogEvent.serialize(event,
> event.isIncludeLocation())));
> + } else {
> + events.add(event);
> + }
> } else if (layout instanceof SerializedLayout) {
> final byte[] header = layout.getHeader();
> final byte[] content = layout.toByteArray(event);
>
>