I think this breaks serialization - the fix I did for LOG4J2-542. The Throwable must be volatile as it may not exist on the remote system, so you cannot rely on it for deserialization.
Sent from my iPad > On May 24, 2014, at 6:54 AM, [email protected] wrote: > > Author: rpopma > Date: Sat May 24 13:54:16 2014 > New Revision: 1597291 > > URL: http://svn.apache.org/r1597291 > Log: > LOG4J2-250: Refactor Log4jLogEvent to lazily create ThrowableProxy > > Modified: > > logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rewrite/MapRewritePolicy.java > > logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jLogEvent.java > > logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/Log4jLogEventTest.java > > logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/JsonLayoutTest.java > > logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/LogEventFixtures.java > > logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/XmlLayoutTest.java > logging/log4j/log4j2/trunk/src/changes/changes.xml > > Modified: > logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rewrite/MapRewritePolicy.java > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rewrite/MapRewritePolicy.java?rev=1597291&r1=1597290&r2=1597291&view=diff > ============================================================================== > --- > logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rewrite/MapRewritePolicy.java > (original) > +++ > logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rewrite/MapRewritePolicy.java > Sat May 24 13:54:16 2014 > @@ -81,8 +81,8 @@ public final class MapRewritePolicy impl > if (source instanceof Log4jLogEvent) { > Log4jLogEvent event = (Log4jLogEvent) source; > return Log4jLogEvent.createEvent(event.getLoggerName(), > event.getMarker(), event.getLoggerFqcn(), > - event.getLevel(), message, event.getThrownProxy(), > event.getContextMap(), event.getContextStack(), > - event.getThreadName(), event.getSource(), > event.getTimeMillis()); > + event.getLevel(), message, event.getThrown(), > event.getThrownProxy(), event.getContextMap(), > + event.getContextStack(), event.getThreadName(), > event.getSource(), event.getTimeMillis()); > } > return new Log4jLogEvent(source.getLoggerName(), source.getMarker(), > source.getLoggerFqcn(), source.getLevel(), > message, source.getThrown(), source.getContextMap(), > source.getContextStack(), source.getThreadName(), > > Modified: > logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jLogEvent.java > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jLogEvent.java?rev=1597291&r1=1597290&r2=1597291&view=diff > ============================================================================== > --- > logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jLogEvent.java > (original) > +++ > logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jLogEvent.java > Sat May 24 13:54:16 2014 > @@ -49,7 +49,8 @@ public class Log4jLogEvent implements Lo > private final String loggerName; > private final Message message; > private final long timeMillis; > - private final ThrowableProxy thrownProxy; > + private final Throwable thrown; > + private ThrowableProxy thrownProxy; > private final Map<String, String> contextMap; > private final ThreadContext.ContextStack contextStack; > private String threadName = null; > @@ -65,7 +66,7 @@ public class Log4jLogEvent implements Lo > * > */ > public Log4jLogEvent(final long timestamp) { > - this(Strings.EMPTY, null, Strings.EMPTY, null, null, > (ThrowableProxy) null, null, null, null, null, timestamp); > + this(Strings.EMPTY, null, Strings.EMPTY, null, null, (Throwable) > null, null, null, null, null, timestamp); > } > > /** > @@ -115,11 +116,11 @@ public class Log4jLogEvent implements Lo > * @param timestamp The timestamp of the event. > */ > public Log4jLogEvent(final String loggerName, final Marker marker, final > String loggerFQCN, final Level level, > - final Message message, final Throwable t, > - final Map<String, String> mdc, final > ThreadContext.ContextStack ndc, final String threadName, > + final Message message, final Throwable t, final > Map<String, String> mdc, > + final ThreadContext.ContextStack ndc, final String > threadName, > final StackTraceElement location, final long > timestamp) { > - this(loggerName, marker, loggerFQCN, level, message, t == null ? > null : new ThrowableProxy(t), mdc, ndc, threadName, > - location, timestamp); > + this(loggerName, marker, loggerFQCN, level, message, t, null, mdc, > ndc, threadName, > + location, timestamp); > } > > /** > @@ -129,7 +130,8 @@ public class Log4jLogEvent implements Lo > * @param loggerFQCN The fully qualified class name of the caller. > * @param level The logging Level. > * @param message The Message. > - * @param t A ThrowableProxy or null. > + * @param thrown A Throwable or null. > + * @param thrownProxy A ThrowableProxy or null. > * @param mdc The mapped diagnostic context. > * @param ndc the nested diagnostic context. > * @param threadName The name of the thread. > @@ -137,11 +139,14 @@ public class Log4jLogEvent implements Lo > * @param timestamp The timestamp of the event. > */ > public static Log4jLogEvent createEvent(final String loggerName, final > Marker marker, final String loggerFQCN, > - final Level level, final Message > message, final ThrowableProxy t, > + final Level level, final Message > message, final Throwable thrown, > + final ThrowableProxy thrownProxy, > final Map<String, String> mdc, > final ThreadContext.ContextStack ndc, > final String threadName, final > StackTraceElement location, > final long timestamp) { > - return new Log4jLogEvent(loggerName, marker, loggerFQCN, level, > message, t, mdc, ndc, threadName, location, timestamp); > + final Log4jLogEvent result = new Log4jLogEvent(loggerName, marker, > loggerFQCN, level, message, thrown, > + thrownProxy, mdc, ndc, threadName, location, timestamp); > + return result; > } > > /** > @@ -151,6 +156,7 @@ public class Log4jLogEvent implements Lo > * @param loggerFQCN The fully qualified class name of the caller. > * @param level The logging Level. > * @param message The Message. > + * @param thrown A Throwable or null. > * @param thrownProxy A ThrowableProxy or null. > * @param contextMap The mapped diagnostic context. > * @param contextStack the nested diagnostic context. > @@ -159,14 +165,15 @@ public class Log4jLogEvent implements Lo > * @param timestamp The timestamp of the event. > */ > private Log4jLogEvent(final String loggerName, final Marker marker, final > String loggerFQCN, final Level level, > - final Message message, final ThrowableProxy > thrownProxy, > - final Map<String, String> contextMap, final > ThreadContext.ContextStack contextStack, final String threadName, > - final StackTraceElement source, final long > timestamp) { > + final Message message, final Throwable thrown, ThrowableProxy > thrownProxy, > + final Map<String, String> contextMap, final > ThreadContext.ContextStack contextStack, > + final String threadName, final StackTraceElement source, final > long timestamp) { > this.loggerName = loggerName; > this.marker = marker; > this.loggerFqcn = loggerFQCN; > this.level = (level == null) ? Level.OFF : level; // LOG4J2-462, > LOG4J2-465 > this.message = message; > + this.thrown = thrown; > this.thrownProxy = thrownProxy; > this.contextMap = contextMap == null ? ThreadContext.EMPTY_MAP : > contextMap; > this.contextStack = contextStack == null ? ThreadContext.EMPTY_STACK > : contextStack; > @@ -250,7 +257,7 @@ public class Log4jLogEvent implements Lo > */ > @Override > public Throwable getThrown() { > - return thrownProxy == null ? null : thrownProxy.getThrowable(); > + return thrown; > } > > /** > @@ -259,6 +266,9 @@ public class Log4jLogEvent implements Lo > */ > @Override > public ThrowableProxy getThrownProxy() { > + if (thrownProxy == null && thrown != null) { > + thrownProxy = new ThrowableProxy(thrown); > + } > return thrownProxy; > } > > @@ -377,7 +387,7 @@ public class Log4jLogEvent implements Lo > final LogEventProxy proxy = (LogEventProxy) event; > final Log4jLogEvent result = new Log4jLogEvent(proxy.loggerName, > proxy.marker, > proxy.loggerFQCN, proxy.level, proxy.message, > - proxy.thrownProxy, proxy.contextMap, proxy.contextStack, > proxy.threadName, > + proxy.thrown, proxy.thrownProxy, proxy.contextMap, > proxy.contextStack, proxy.threadName, > proxy.source, proxy.timeMillis); > result.setEndOfBatch(proxy.isEndOfBatch); > result.setIncludeLocation(proxy.isLocationRequired); > @@ -447,6 +457,9 @@ public class Log4jLogEvent implements Lo > if (threadName != null ? !threadName.equals(that.threadName) : > that.threadName != null) { > return false; > } > + if (thrown != null ? !thrown.equals(that.thrown) : that.thrown != > null) { > + return false; > + } > if (thrownProxy != null ? !thrownProxy.equals(that.thrownProxy) : > that.thrownProxy != null) { > return false; > } > @@ -462,6 +475,7 @@ public class Log4jLogEvent implements Lo > result = 31 * result + loggerName.hashCode(); > result = 31 * result + message.hashCode(); > result = 31 * result + (int) (timeMillis ^ (timeMillis >>> 32)); > + result = 31 * result + (thrown != null ? thrown.hashCode() : 0); > result = 31 * result + (thrownProxy != null ? thrownProxy.hashCode() > : 0); > result = 31 * result + (contextMap != null ? contextMap.hashCode() : > 0); > result = 31 * result + (contextStack != null ? > contextStack.hashCode() : 0); > @@ -484,6 +498,7 @@ public class Log4jLogEvent implements Lo > private final String loggerName; > private final Message message; > private final long timeMillis; > + private final Throwable thrown; > private final ThrowableProxy thrownProxy; > private final Map<String, String> contextMap; > private final ThreadContext.ContextStack contextStack; > @@ -499,6 +514,7 @@ public class Log4jLogEvent implements Lo > this.loggerName = event.loggerName; > this.message = event.message; > this.timeMillis = event.timeMillis; > + this.thrown = event.thrown; > this.thrownProxy = event.thrownProxy; > this.contextMap = event.contextMap; > this.contextStack = event.contextStack; > @@ -513,9 +529,8 @@ public class Log4jLogEvent implements Lo > * @return Log4jLogEvent. > */ > protected Object readResolve() { > - final Log4jLogEvent result = new Log4jLogEvent(loggerName, > marker, loggerFQCN, > - level, message, thrownProxy, contextMap, contextStack, > threadName, source, > - timeMillis); > + final Log4jLogEvent result = new Log4jLogEvent(loggerName, > marker, loggerFQCN, level, message, thrown, > + thrownProxy, contextMap, contextStack, threadName, > source, timeMillis); > result.setEndOfBatch(isEndOfBatch); > result.setIncludeLocation(isLocationRequired); > return result; > > Modified: > logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/Log4jLogEventTest.java > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/Log4jLogEventTest.java?rev=1597291&r1=1597290&r2=1597291&view=diff > ============================================================================== > --- > logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/Log4jLogEventTest.java > (original) > +++ > logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/Log4jLogEventTest.java > Sat May 24 13:54:16 2014 > @@ -83,6 +83,37 @@ public class Log4jLogEventTest { > assertEquals(evt.isEndOfBatch(), evt2.isEndOfBatch()); > assertEquals(evt.isIncludeLocation(), evt2.isIncludeLocation()); > } > + > + @Test > + public void testJavaIoSerializableWithThrown() throws Exception { > + final Error thrown = new InternalError("test error"); > + final Log4jLogEvent evt = new Log4jLogEvent("some.test", null, > Strings.EMPTY, > + Level.INFO, new SimpleMessage("abc"), thrown); > + > + final ByteArrayOutputStream arr = new ByteArrayOutputStream(); > + final ObjectOutputStream out = new ObjectOutputStream(arr); > + out.writeObject(evt); > + > + final ByteArrayInputStream inArr = new > ByteArrayInputStream(arr.toByteArray()); > + final ObjectInputStream in = new ObjectInputStream(inArr); > + final Log4jLogEvent evt2 = (Log4jLogEvent) in.readObject(); > + > + assertEquals(evt.getTimeMillis(), evt2.getTimeMillis()); > + assertEquals(evt.getLoggerFqcn(), evt2.getLoggerFqcn()); > + assertEquals(evt.getLevel(), evt2.getLevel()); > + assertEquals(evt.getLoggerName(), evt2.getLoggerName()); > + assertEquals(evt.getMarker(), evt2.getMarker()); > + assertEquals(evt.getContextMap(), evt2.getContextMap()); > + assertEquals(evt.getContextStack(), evt2.getContextStack()); > + assertEquals(evt.getMessage(), evt2.getMessage()); > + assertEquals(evt.getSource(), evt2.getSource()); > + assertEquals(evt.getThreadName(), evt2.getThreadName()); > + assertEquals(evt.getThrown().getClass(), > evt2.getThrown().getClass()); > + assertEquals(evt.getThrown().getMessage(), > evt2.getThrown().getMessage()); > + assertEquals(evt.getThrownProxy(), evt2.getThrownProxy()); > + assertEquals(evt.isEndOfBatch(), evt2.isEndOfBatch()); > + assertEquals(evt.isIncludeLocation(), evt2.isIncludeLocation()); > + } > > @Test > public void testNullLevelReplacedWithOFF() throws Exception { > > Modified: > logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/JsonLayoutTest.java > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/JsonLayoutTest.java?rev=1597291&r1=1597290&r2=1597291&view=diff > ============================================================================== > --- > logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/JsonLayoutTest.java > (original) > +++ > logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/JsonLayoutTest.java > Sat May 24 13:54:16 2014 > @@ -100,10 +100,11 @@ public class JsonLayoutTest { > assertTrue(str, str.contains(DQUOTE + name + DQUOTE + propSep)); > } > > - private void testAllFeatures(final boolean includeSource, final boolean > compact, final boolean includeContext) throws Exception { > + private void testAllFeatures(final boolean includeSource, final boolean > compact, final boolean includeContext) > + throws Exception { > final Log4jLogEvent expected = LogEventFixtures.createLogEvent(); > - final AbstractJacksonLayout layout = > JsonLayout.createLayout(Boolean.toString(includeSource), > Boolean.toString(includeContext), > - "false", Boolean.toString(compact), "UTF-8"); > + final AbstractJacksonLayout layout = > JsonLayout.createLayout(Boolean.toString(includeSource), > + Boolean.toString(includeContext), "false", > Boolean.toString(compact), "UTF-8"); > final String str = layout.toSerializable(expected); > // System.out.println(str); > final String propSep = this.toPropertySeparator(compact); > @@ -251,8 +252,8 @@ public class JsonLayoutTest { > @Test > public void testLayoutLoggerName() throws Exception { > final AbstractJacksonLayout layout = JsonLayout.createLayout("false", > null, "false", "true", "UTF-8"); > - final Log4jLogEvent expected = Log4jLogEvent.createEvent("a.B", > null, "f.q.c.n", Level.DEBUG, new SimpleMessage("M"), null, null, > - null, "threadName", null, 1); > + final Log4jLogEvent expected = Log4jLogEvent.createEvent("a.B", > null, "f.q.c.n", Level.DEBUG, > + new SimpleMessage("M"), null, null, null, null, > "threadName", null, 1); > final String str = layout.toSerializable(expected); > assertTrue(str, str.contains("\"loggerName\":\"a.B\"")); > final Log4jLogEvent actual = new > Log4jJsonObjectMapper().readValue(str, Log4jLogEvent.class); > > Modified: > logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/LogEventFixtures.java > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/LogEventFixtures.java?rev=1597291&r1=1597290&r2=1597291&view=diff > ============================================================================== > --- > logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/LogEventFixtures.java > (original) > +++ > logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/LogEventFixtures.java > Sat May 24 13:54:16 2014 > @@ -56,7 +56,7 @@ class LogEventFixtures { > final IOException ioException = new IOException("testIOEx", cause); > Throwables.addSuppressed(ioException, new > IndexOutOfBoundsException("I am suppressed exception 1")); > Throwables.addSuppressed(ioException, new > IndexOutOfBoundsException("I am suppressed exception 2")); > - final ThrowableProxy throwable = new ThrowableProxy(ioException); > + final ThrowableProxy throwableProxy = new > ThrowableProxy(ioException); > final Map<String, String> contextMap = new HashMap<String, String>(); > contextMap.put("MDC.A", "A_Value"); > contextMap.put("MDC.B", "B_Value"); > @@ -64,13 +64,15 @@ class LogEventFixtures { > contextStack.clear(); > contextStack.push("stack_msg1"); > contextStack.add("stack_msg2"); > - final Log4jLogEvent expected = Log4jLogEvent.createEvent("a.B", > cMarker, "f.q.c.n", Level.DEBUG, new SimpleMessage("Msg"), > - throwable, contextMap, contextStack, "MyThreadName", source, > 1); > + final Log4jLogEvent expected = Log4jLogEvent.createEvent("a.B", > cMarker, "f.q.c.n", Level.DEBUG, > + new SimpleMessage("Msg"), ioException, throwableProxy, > contextMap, contextStack, "MyThreadName", source, > + 1); > // validate event? > return expected; > } > > - static void assertEqualLogEvents(final LogEvent expected, final LogEvent > actual, final boolean includeSource, final boolean includeContext) { > + static void assertEqualLogEvents(final LogEvent expected, final LogEvent > actual, final boolean includeSource, > + final boolean includeContext) { > assertEquals(expected.getClass(), actual.getClass()); > assertEquals(includeContext ? expected.getContextMap() : > Collections.EMPTY_MAP, actual.getContextMap()); > assertEquals(expected.getContextStack(), actual.getContextStack()); > @@ -82,13 +84,16 @@ class LogEventFixtures { > assertEquals(expected.getTimeMillis(), actual.getTimeMillis()); > assertEquals(includeSource ? expected.getSource() : null, > actual.getSource()); > assertEquals(expected.getThreadName(), actual.getThreadName()); > + assertNotNull("original should have an exception", > expected.getThrown()); > + assertNull("exception should not be serialized", actual.getThrown()); > assertEquals(expected.getThrownProxy(), actual.getThrownProxy()); > assertEquals(expected.isEndOfBatch(), actual.isEndOfBatch()); > assertEquals(expected.isIncludeLocation(), > actual.isIncludeLocation()); > - if (includeSource) { > - assertEquals(expected.hashCode(), actual.hashCode()); > - assertEquals(expected, actual); > - } > + > + // original: non-null thrown & null thrownProxy > + // deserialized: null thrown & non-null thrownProxy > + assertNotEquals(expected.hashCode(), actual.hashCode()); > + assertNotEquals(expected, actual); > } > > } > > Modified: > logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/XmlLayoutTest.java > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/XmlLayoutTest.java?rev=1597291&r1=1597290&r2=1597291&view=diff > ============================================================================== > --- > logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/XmlLayoutTest.java > (original) > +++ > logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/XmlLayoutTest.java > Sat May 24 13:54:16 2014 > @@ -256,8 +256,8 @@ public class XmlLayoutTest { > @Test > public void testLayoutLoggerName() { > final XmlLayout layout = XmlLayout.createLayout("false", "true", > "true", null, null); > - final Log4jLogEvent event = Log4jLogEvent.createEvent("a.B", null, > "f.q.c.n", Level.DEBUG, new SimpleMessage("M"), null, null, > - null, "threadName", null, 1); > + final Log4jLogEvent event = Log4jLogEvent.createEvent("a.B", null, > "f.q.c.n", Level.DEBUG, > + new SimpleMessage("M"), null, null, null, null, > "threadName", null, 1); > final String str = layout.toSerializable(event); > assertTrue(str, str.contains("loggerName=\"a.B\"")); > } > > Modified: logging/log4j/log4j2/trunk/src/changes/changes.xml > URL: > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/src/changes/changes.xml?rev=1597291&r1=1597290&r2=1597291&view=diff > ============================================================================== > --- logging/log4j/log4j2/trunk/src/changes/changes.xml (original) > +++ logging/log4j/log4j2/trunk/src/changes/changes.xml Sat May 24 13:54:16 > 2014 > @@ -22,6 +22,9 @@ > </properties> > <body> > <release version="2.0-rc2" date="2014-MM-DD" description="Bug fixes and > enhancements"> > + <action issue="LOG4J2-250" dev="rpopma" type="update"> > + Refactor Log4jLogEvent to lazily create ThrowableProxy. > + </action> > <action issue="LOG4J2-647" dev="ggregory" type="update"> > Upgrade to Flume 1.5.0. > </action> > > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
