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:20:59 UTC
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)
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);
>
>
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);