Bug 41186 - AsyncAppender in 1.2.14 DiscardSummary events create NullPointerExceptions in layouts
AsyncAppender in 1.2.14 DiscardSummary events create NullPointerExceptions in...
Status: RESOLVED FIXED
Product: Log4j
Classification: Unclassified
Component: Appender
1.2
All All
: P2 normal
: ---
Assigned To: log4j-dev
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2006-12-15 14:16 UTC by gopaczewski
Modified: 2007-04-20 20:36 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description gopaczewski 2006-12-15 14:16:20 UTC
The 1.2.14 version of the AsyncAppender creates LoggingEvent's from
DiscardSummary objects with null fqnOfCategoryClass.  This causes layouts that
call getLocationInformation() on the LoggingEvent to throw a NullPointerException:

Exception in thread "Dispatcher-Thread-6"
java.lang.NullPointerException
     at java.lang.String.lastIndexOf(Unknown Source)
     at java.lang.String.lastIndexOf(Unknown Source)
     at org.apache.log4j.spi.LocationInfo.<init>(LocationInfo.java:119)
     at
org.apache.log4j.spi.LoggingEvent.getLocationInformation(LoggingEvent.java:191)
     at
org.apache.log4j.helpers.PatternParser$ClassNamePatternConverter.getFullyQualifiedName(PatternParser.java:538)
     at
org.apache.log4j.helpers.PatternParser$NamedPatternConverter.convert(PatternParser.java:511)
     at org.apache.log4j.helpers.PatternConverter.format(PatternConverter.java:64)
     at org.apache.log4j.PatternLayout.format(PatternLayout.java:503)
     at org.apache.log4j.WriterAppender.subAppend(WriterAppender.java:301)
     at org.apache.log4j.WriterAppender.append(WriterAppender.java:159)
     at org.apache.log4j.AppenderSkeleton.doAppend(AppenderSkeleton.java:230)
     at
org.apache.log4j.helpers.AppenderAttachableImpl.appendLoopOnAppenders(AppenderAttachableImpl.java:65)
     at org.apache.log4j.AsyncAppender$Dispatcher.run(AsyncAppender.java:578)
     at java.lang.Thread.run(Unknown Source)

This kills off the Dispatcher thread, reverting to synchronous logging.

Steps to Reproduce:
1. Configure an appender with a PatternLayout that retrieves LocationInfo:

<appender name="TestAppender" class="org.apache.log4j.FileAppender">
  <param name="file" value="/tmp/afile.log" />
  <layout class="org.apache.log4j.PatternLayout">
    <param name="ConversionPattern" value="%C:%L %m%n" />
  </layout>
</appender>

2. Configure a second AsyncAppender that references the first appender as follows:

<appender name="MyAsyncAppender" class="org.apache.log4j.AsyncAppender">
  <param name="locationInfo" value="true" />
  <param name="blocking" value="false" />
  <param name="bufferSize" value="1" />
  <appender-ref ref="TestAppender" />
</appender>

3. Create a scenario where the buffer in the AsyncAppender fills up, generating
discard events.  (Should be pretty easy with a bufferSize=1)
Comment 1 Curt Arnold 2006-12-15 16:11:33 UTC
Committed rev 487725 on trunk, 487224 on 1.2 branch.  Passing null for the fully qualified category (aka 
logger) name in the LoggingEvent constructor would cause a NPE in the 1.2 code base, but not 1.3.  
Addressed problem by passing in a string that should not match any class name in a stack trace which 
should result in the class name and location being reported as "?" in both branches.

Added tests to both test suites.  Surprised that very few of the AsyncAppender tests had been ported back 
to log4j 1.2, but only backported essential support classes.
Comment 2 gopaczewski 2006-12-18 06:27:45 UTC
Thanks for resolving so quickly.  Is there an ETA for 1.2.15 at this point?
Comment 3 Curt Arnold 2007-04-20 20:36:59 UTC
The underlying problem, a NullPointerException on a call to LoggingEvent.getLocationInformation() 
when FQCN is null was also reported in Chainsaw running on log4j 1.2.  The initial suggestion was to 
backport a log4j 1.3 change so that getLocationInfo() would return null.  However, the log4j 1.2 pattern 
layout does not check for a null when calling getLocationInformation() and only making that change 
(and reverting the previous change) will just move the NPE to a slightly different place in the test case.

I added a new unit test that calls getLocationInformation() when FQCN is null and checks that all the 
accessors return "?".  The behavior should be identical to passing a bogus class name.  The change 
committed in rev 530974 is sufficient to pass the unit tests for the original reported bug without the 
earlier change to AsyncAppender.  However, think it is safer to leave the bogus FQCN in 
AsyncAppender.