True. Until now getFormattedMessage() used lazy initialization in many messages. The LOG4J2-763 changes mean initialization always happens in the constructor so the code can be simplified. I won't have time for this tonight though.
Sent from my iPhone > On 2014/08/10, at 21:12, Gary Gregory <[email protected]> wrote: > > Instead of calling getFormattedMessage(), how about refactoring it and call > an initSomething method. This will make the call sites less smelly of > programming by side effect. > > Gary > > > -------- Original message -------- > From: [email protected] > Date:08/10/2014 02:40 (GMT-05:00) > To: [email protected] > Subject: svn commit: r1617051 - in /logging/log4j/log4j2/trunk: > log4j-api/src/main/java/org/apache/logging/log4j/message/ > log4j-api/src/test/java/org/apache/logging/log4j/message/ src/changes/ > src/site/xdoc/manual/ > > Author: rpopma > Date: Sun Aug 10 06:40:14 2014 > New Revision: 1617051 > > URL: http://svn.apache.org/r1617051 > Log: > LOG4J2-763: Improved FormattedMessage, StringFormattedMessage, > LocalizedMessage, MessageFormatMessage and ObjectMessage for asynchronous > logging to ensure the formatted message does not change even if parameters > are modified by the application. Improved docs for MapMessage and > StructuredDataMessage. Improved site docs. > > Added: > > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/Mutable.java > (with props) > Modified: > > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/FormattedMessage.java > > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/LocalizedMessage.java > > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/MapMessage.java > > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/MessageFormatMessage.java > > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/ObjectMessage.java > > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/StringFormattedMessage.java > > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/StructuredDataMessage.java > > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/FormattedMessageTest.java > > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/LocalizedMessageTest.java > > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/MapMessageTest.java > > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/MessageFormatMessageTest.java > > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/ObjectMessageTest.java > > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java > > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/StringFormattedMessageTest.java > > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/StructuredDataMessageTest.java > > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/ThreadDumpMessageTest.java > logging/log4j/log4j2/trunk/src/changes/changes.xml > logging/log4j/log4j2/trunk/src/site/xdoc/manual/async.xml > > Modified: > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/FormattedMessage.java > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/FormattedMessage.java?rev=1617051&r1=1617050&r2=1617051&view=diff > ============================================================================== > --- > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/FormattedMessage.java > (original) > +++ > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/FormattedMessage.java > Sun Aug 10 06:40:14 2014 > @@ -46,12 +46,11 @@ public class FormattedMessage implements > this.messagePattern = messagePattern; > this.argArray = arguments; > this.throwable = throwable; > + getFormattedMessage(); // LOG4J2-763 take snapshot of parameters at > message construction time > } > > public FormattedMessage(final String messagePattern, final Object[] > arguments) { > - this.messagePattern = messagePattern; > - this.argArray = arguments; > - this.throwable = null; > + this(messagePattern, arguments, null); > } > > /** > @@ -60,9 +59,7 @@ public class FormattedMessage implements > * @param arg The parameter. > */ > public FormattedMessage(final String messagePattern, final Object arg) { > - this.messagePattern = messagePattern; > - this.argArray = new Object[] {arg}; > - this.throwable = null; > + this(messagePattern, new Object[] {arg}, null); > } > > /** > > Modified: > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/LocalizedMessage.java > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/LocalizedMessage.java?rev=1617051&r1=1617050&r2=1617051&view=diff > ============================================================================== > --- > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/LocalizedMessage.java > (original) > +++ > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/LocalizedMessage.java > Sun Aug 10 06:40:14 2014 > @@ -78,6 +78,7 @@ public class LocalizedMessage implements > this.baseName = baseName; > this.resourceBundle = null; > this.locale = locale; > + getFormattedMessage(); // LOG4J2-763 take snapshot of parameters at > message construction time > } > > public LocalizedMessage(final ResourceBundle bundle, final Locale > locale, final String key, > @@ -88,6 +89,7 @@ public class LocalizedMessage implements > this.baseName = null; > this.resourceBundle = bundle; > this.locale = locale; > + getFormattedMessage(); // LOG4J2-763 take snapshot of parameters at > message construction time > } > > public LocalizedMessage(final Locale locale, final String key, final > Object[] arguments) { > > Modified: > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/MapMessage.java > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/MapMessage.java?rev=1617051&r1=1617050&r2=1617051&view=diff > ============================================================================== > --- > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/MapMessage.java > (original) > +++ > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/MapMessage.java > Sun Aug 10 06:40:14 2014 > @@ -26,6 +26,11 @@ import org.apache.logging.log4j.util.Str > > /** > * Represents a Message that consists of a Map. > + * <p> > + * Thread-safety note: the contents of this message can be modified after > construction. > + * When using asynchronous loggers and appenders it is not recommended to > modify this message after the message is > + * logged, because it is undefined whether the logged message string will > contain the old values or the modified > + * values. > */ > public class MapMessage implements MultiformatMessage { > /** > > Modified: > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/MessageFormatMessage.java > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/MessageFormatMessage.java?rev=1617051&r1=1617050&r2=1617051&view=diff > ============================================================================== > --- > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/MessageFormatMessage.java > (original) > +++ > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/MessageFormatMessage.java > Sun Aug 10 06:40:14 2014 > @@ -49,6 +49,7 @@ public class MessageFormatMessage implem > if (arguments != null && arguments.length > 0 && > arguments[arguments.length - 1] instanceof Throwable) { > this.throwable = (Throwable) arguments[arguments.length - 1]; > } > + getFormattedMessage(); // LOG4J2-763 take snapshot of parameters at > message construction time > } > > /** > > Modified: > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/ObjectMessage.java > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/ObjectMessage.java?rev=1617051&r1=1617050&r2=1617051&view=diff > ============================================================================== > --- > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/ObjectMessage.java > (original) > +++ > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/ObjectMessage.java > Sun Aug 10 06:40:14 2014 > @@ -29,6 +29,7 @@ public class ObjectMessage implements Me > private static final long serialVersionUID = -5903272448334166185L; > > private transient Object obj; > + private final String objectString; > > /** > * Create the ObjectMessage. > @@ -39,6 +40,9 @@ public class ObjectMessage implements Me > obj = "null"; > } > this.obj = obj; > + > + // LOG4J2-763: take snapshot of parameters at message construction > time > + objectString = String.valueOf(obj); > } > > /** > @@ -47,7 +51,7 @@ public class ObjectMessage implements Me > */ > @Override > public String getFormattedMessage() { > - return obj.toString(); > + return objectString; > } > > /** > @@ -56,7 +60,7 @@ public class ObjectMessage implements Me > */ > @Override > public String getFormat() { > - return obj.toString(); > + return objectString; > } > > /** > @@ -89,7 +93,7 @@ public class ObjectMessage implements Me > > @Override > public String toString() { > - return "ObjectMessage[obj=" + obj.toString() + ']'; > + return "ObjectMessage[obj=" + objectString + ']'; > } > > private void writeObject(final ObjectOutputStream out) throws > IOException { > > Modified: > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/StringFormattedMessage.java > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/StringFormattedMessage.java?rev=1617051&r1=1617050&r2=1617051&view=diff > ============================================================================== > --- > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/StringFormattedMessage.java > (original) > +++ > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/StringFormattedMessage.java > Sun Aug 10 06:40:14 2014 > @@ -48,6 +48,7 @@ public class StringFormattedMessage impl > if (arguments != null && arguments.length > 0 && > arguments[arguments.length - 1] instanceof Throwable) { > this.throwable = (Throwable) arguments[arguments.length - 1]; > } > + getFormattedMessage(); // LOG4J2-763 take snapshot of parameters at > message construction time > } > > /** > > Modified: > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/StructuredDataMessage.java > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/StructuredDataMessage.java?rev=1617051&r1=1617050&r2=1617051&view=diff > ============================================================================== > --- > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/StructuredDataMessage.java > (original) > +++ > logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/message/StructuredDataMessage.java > Sun Aug 10 06:40:14 2014 > @@ -22,6 +22,11 @@ import org.apache.logging.log4j.util.Eng > > /** > * Represents a Message that conforms to an RFC 5424 StructuredData element > along with the syslog message. > + * <p> > + * Thread-safety note: the contents of this message can be modified after > construction. > + * When using asynchronous loggers and appenders it is not recommended to > modify this message after the message is > + * logged, because it is undefined whether the logged message string will > contain the old values or the modified > + * values. > * > * @see <a href="https://tools.ietf.org/html/rfc5424";>RFC 5424</a> > */ > > Modified: > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/FormattedMessageTest.java > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/FormattedMessageTest.java?rev=1617051&r1=1617050&r2=1617051&view=diff > ============================================================================== > --- > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/FormattedMessageTest.java > (original) > +++ > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/FormattedMessageTest.java > Sun Aug 10 06:40:14 2014 > @@ -83,4 +83,16 @@ public class FormattedMessageTest { > result = msg.getFormattedMessage(); > assertEquals(testMsg, result); > } > + > + @Test > + public void testSafeWithMutableParams() { // LOG4J2-763 > + final String testMsg = "Test message %s"; > + final Mutable param = new Mutable().set("abc"); > + FormattedMessage msg = new FormattedMessage(testMsg, param); > + > + // modify parameter before calling msg.getFormattedMessage > + param.set("XYZ"); > + String actual = msg.getFormattedMessage(); > + assertEquals("Should use initial param value", "Test message abc", > actual); > + } > } > > Modified: > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/LocalizedMessageTest.java > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/LocalizedMessageTest.java?rev=1617051&r1=1617050&r2=1617051&view=diff > ============================================================================== > --- > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/LocalizedMessageTest.java > (original) > +++ > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/LocalizedMessageTest.java > Sun Aug 10 06:40:14 2014 > @@ -64,4 +64,16 @@ public class LocalizedMessageTest { > assertEquals("This is test number 1 with string argument Test.", > msg.getFormattedMessage()); > } > > + @Test > + public void testSafeWithMutableParams() { // LOG4J2-763 > + final String testMsg = "Test message %s"; > + final Mutable param = new Mutable().set("abc"); > + LocalizedMessage msg = new LocalizedMessage(testMsg, param); > + > + // modify parameter before calling msg.getFormattedMessage > + param.set("XYZ"); > + String actual = msg.getFormattedMessage(); > + assertEquals("Should use initial param value", "Test message abc", > actual); > + } > + > } > > Modified: > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/MapMessageTest.java > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/MapMessageTest.java?rev=1617051&r1=1617050&r2=1617051&view=diff > ============================================================================== > --- > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/MapMessageTest.java > (original) > +++ > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/MapMessageTest.java > Sun Aug 10 06:40:14 2014 > @@ -70,4 +70,22 @@ public class MapMessageTest { > final String expected = "{message=\"Test message {}\", > project=\"Log4j\"}"; > assertEquals(expected, result); > } > + > + @Test > + public void testMutableByDesign() { // LOG4J2-763 > + MapMessage msg = new MapMessage(); > + > + // modify parameter before calling msg.getFormattedMessage > + msg.put("key1", "value1"); > + msg.put("key2", "value2"); > + final String result = msg.getFormattedMessage(new String[]{"Java"}); > + final String expected = "{key1=\"value1\", key2=\"value2\"}"; > + assertEquals(expected, result); > + > + // modify parameter after calling msg.getFormattedMessage > + msg.put("key3", "value3"); > + final String result2 = msg.getFormattedMessage(new String[]{"Java"}); > + final String expected2 = "{key1=\"value1\", key2=\"value2\", > key3=\"value3\"}"; > + assertEquals(expected2, result2); > + } > } > > Modified: > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/MessageFormatMessageTest.java > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/MessageFormatMessageTest.java?rev=1617051&r1=1617050&r2=1617051&view=diff > ============================================================================== > --- > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/MessageFormatMessageTest.java > (original) > +++ > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/MessageFormatMessageTest.java > Sun Aug 10 06:40:14 2014 > @@ -61,4 +61,16 @@ public class MessageFormatMessageTest { > final Throwable t = msg.getThrowable(); > assertNotNull("No Throwable", t); > } > + > + @Test > + public void testSafeWithMutableParams() { // LOG4J2-763 > + final String testMsg = "Test message {0}"; > + final Mutable param = new Mutable().set("abc"); > + MessageFormatMessage msg = new MessageFormatMessage(testMsg, param); > + > + // modify parameter before calling msg.getFormattedMessage > + param.set("XYZ"); > + String actual = msg.getFormattedMessage(); > + assertEquals("Should use initial param value", "Test message abc", > actual); > + } > } > > Added: > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/Mutable.java > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/Mutable.java?rev=1617051&view=auto > ============================================================================== > --- > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/Mutable.java > (added) > +++ > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/Mutable.java > Sun Aug 10 06:40:14 2014 > @@ -0,0 +1,34 @@ > +/* > + * Licensed to the Apache Software Foundation (ASF) under one or more > + * contributor license agreements. See the NOTICE file distributed with > + * this work for additional information regarding copyright ownership. > + * The ASF licenses this file to You under the Apache license, Version 2.0 > + * (the "License"); you may not use this file except in compliance with > + * the License. You may obtain a copy of the License at > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the license for the specific language governing permissions and > + * limitations under the license. > + */ > +package org.apache.logging.log4j.message; > + > +/** > + * Helper class for JUnit tests. > + */ > +class Mutable { > + private String value; > + > + public Mutable set(String value) { > + this.value = value; > + return this; > + } > + > + @Override > + public String toString() { > + return this.value; > + } > +} > \ No newline at end of file > > Propchange: > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/Mutable.java > ------------------------------------------------------------------------------ > svn:eol-style = native > > Modified: > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/ObjectMessageTest.java > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/ObjectMessageTest.java?rev=1617051&r1=1617050&r2=1617051&view=diff > ============================================================================== > --- > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/ObjectMessageTest.java > (original) > +++ > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/ObjectMessageTest.java > Sun Aug 10 06:40:14 2014 > @@ -39,4 +39,15 @@ public class ObjectMessageTest { > final String result = msg.getFormattedMessage(); > assertEquals(testMsg, result); > } > + > + @Test > + public void testSafeWithMutableParams() { // LOG4J2-763 > + final Mutable param = new Mutable().set("abc"); > + ObjectMessage msg = new ObjectMessage(param); > + > + // modify parameter before calling msg.getFormattedMessage > + param.set("XYZ"); > + String actual = msg.getFormattedMessage(); > + assertEquals("Should use initial param value", "abc", actual); > + } > } > > Modified: > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java?rev=1617051&r1=1617050&r2=1617051&view=diff > ============================================================================== > --- > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java > (original) > +++ > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java > Sun Aug 10 06:40:14 2014 > @@ -36,4 +36,16 @@ public class ParameterizedMessageTest { > result = msg.getFormattedMessage(); > assertEquals(testMsg, result); > } > + > + @Test > + public void testSafeWithMutableParams() { // LOG4J2-763 > + final String testMsg = "Test message {}"; > + final Mutable param = new Mutable().set("abc"); > + ParameterizedMessage msg = new ParameterizedMessage(testMsg, param); > + > + // modify parameter before calling msg.getFormattedMessage > + param.set("XYZ"); > + String actual = msg.getFormattedMessage(); > + assertEquals("Should use initial param value", "Test message abc", > actual); > + } > } > > Modified: > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/StringFormattedMessageTest.java > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/StringFormattedMessageTest.java?rev=1617051&r1=1617050&r2=1617051&view=diff > ============================================================================== > --- > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/StringFormattedMessageTest.java > (original) > +++ > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/StringFormattedMessageTest.java > Sun Aug 10 06:40:14 2014 > @@ -60,4 +60,16 @@ public class StringFormattedMessageTest > final Throwable t = msg.getThrowable(); > assertNotNull("No Throwable", t); > } > + > + @Test > + public void testSafeWithMutableParams() { // LOG4J2-763 > + final String testMsg = "Test message %s"; > + final Mutable param = new Mutable().set("abc"); > + StringFormattedMessage msg = new StringFormattedMessage(testMsg, > param); > + > + // modify parameter before calling msg.getFormattedMessage > + param.set("XYZ"); > + String actual = msg.getFormattedMessage(); > + assertEquals("Should use initial param value", "Test message abc", > actual); > + } > } > > Modified: > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/StructuredDataMessageTest.java > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/StructuredDataMessageTest.java?rev=1617051&r1=1617050&r2=1617051&view=diff > ============================================================================== > --- > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/StructuredDataMessageTest.java > (original) > +++ > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/StructuredDataMessageTest.java > Sun Aug 10 06:40:14 2014 > @@ -43,4 +43,23 @@ public class StructuredDataMessageTest { > final StructuredDataMessage msg = new > StructuredDataMessage("MsgId@12345", testMsg, "Alert"); > msg.put("This is a very long key that will violate the key length > validation", "Testing"); > } > + > + @Test > + public void testMutableByDesign() { // LOG4J2-763 > + final String testMsg = "Test message {}"; > + StructuredDataMessage msg = new StructuredDataMessage("MsgId@1", > testMsg, "Alert"); > + > + // modify parameter before calling msg.getFormattedMessage > + msg.put("message", testMsg); > + msg.put("project", "Log4j"); > + final String result = msg.getFormattedMessage(); > + final String expected = "Alert [MsgId@1 message=\"Test message {}\" > project=\"Log4j\"] Test message {}"; > + assertEquals(expected, result); > + > + // modify parameter after calling msg.getFormattedMessage > + msg.put("memo", "Added later"); > + final String result2 = msg.getFormattedMessage(); > + final String expected2 = "Alert [MsgId@1 memo=\"Added later\" > message=\"Test message {}\" project=\"Log4j\"] Test message {}"; > + assertEquals(expected2, result2); > + } > } > > Modified: > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/ThreadDumpMessageTest.java > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/ThreadDumpMessageTest.java?rev=1617051&r1=1617050&r2=1617051&view=diff > ============================================================================== > --- > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/ThreadDumpMessageTest.java > (original) > +++ > logging/log4j/log4j2/trunk/log4j-api/src/test/java/org/apache/logging/log4j/message/ThreadDumpMessageTest.java > Sun Aug 10 06:40:14 2014 > @@ -73,6 +73,22 @@ public class ThreadDumpMessageTest { > assertEquals(expected, msg.toString()); > } > > + @Test > + public void testUseConstructorThread() throws InterruptedException { // > LOG4J2-763 > + final ThreadDumpMessage msg = new ThreadDumpMessage("Test"); > + > + final String[] actual = new String[1]; > + Thread other = new Thread("OtherThread") { > + public void run() { > + actual[0] = msg.getFormattedMessage(); > + } > + }; > + other.start(); > + other.join(); > + > + assertTrue("No mention of other thread in msg", > !actual[0].contains("OtherThread")); > + } > + > private class Thread1 extends Thread { > private final ReentrantLock lock; > > > Modified: logging/log4j/log4j2/trunk/src/changes/changes.xml > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/src/changes/changes.xml?rev=1617051&r1=1617050&r2=1617051&view=diff > ============================================================================== > --- logging/log4j/log4j2/trunk/src/changes/changes.xml (original) > +++ logging/log4j/log4j2/trunk/src/changes/changes.xml Sun Aug 10 06:40:14 > 2014 > @@ -22,6 +22,12 @@ > </properties> > <body> > <release version="2.0.2" date="2014-0?-??" description="Bug fixes and > enhancements"> > + <action issue="LOG4J2-763" dev="rpopma" type="fix"> > + Improved FormattedMessage, StringFormattedMessage, LocalizedMessage, > MessageFormatMessage and > + ObjectMessage for asynchronous logging to ensure the formatted > message does not change even if > + parameters are modified by the application. (ParameterizedMessage > was already safe.) > + Improved documentation. > + </action> > <action issue="LOG4J2-729" dev="rpopma" type="fix"> > Emit warning message to console if no configuration file found. > </action> > > Modified: logging/log4j/log4j2/trunk/src/site/xdoc/manual/async.xml > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/src/site/xdoc/manual/async.xml?rev=1617051&r1=1617050&r2=1617051&view=diff > ============================================================================== > --- logging/log4j/log4j2/trunk/src/site/xdoc/manual/async.xml (original) > +++ logging/log4j/log4j2/trunk/src/site/xdoc/manual/async.xml Sun Aug 10 > 06:40:14 2014 > @@ -124,6 +124,26 @@ > logging in addition to synchronous > logging for the audit trail.) > </li> > + <li>In some rare cases, care must be taken with mutable messages. > + Most of the time you don't need to worry about this. Log4 will > ensure that log messages like > + <code>logger.debug("My object is {}", myObject)</code> will use the > state of the > + <code>myObject</code> parameter at the time of the call to > <code>logger.debug()</code>. > + The log message will not change even if <code>myObject</code> is > modified later. > + It is safe to asynchronously log mutable objects because most > + <a > href="../log4j-api/apidocs/org/apache/logging/log4j/message/Message.html">Message</a> > + implementations built-in to Log4j take a snapshot of the parameters. > + There are some exceptions however: > + <a > href="../log4j-api/apidocs/org/apache/logging/log4j/message/MapMessage.html">MapMessage</a> > + and > + <a > href="../log4j-api/apidocs/org/apache/logging/log4j/message/StructuredDataMessage.html">StructuredDataMessage</a> > + are mutable by design: fields can be added to these messages after > the message object was created. > + These messages should not be modified after they are logged with > asynchronous loggers or > + asynchronous appenders; you may or may not see the modifications in > the resulting log output. > + Similarly, custom > + <a > href="../log4j-api/apidocs/org/apache/logging/log4j/message/Message.html">Message</a> > + implementations should be designed with asynchronous use in mind, > and either take a snapshot > + of their parameters at construction time, or document their > thread-safety characteristics. > + </li> > </ul> > </subsection> > <a name="AllAsync" /> > > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
