Bug 23021 - Queue reading thread in AsyncAppender blocks if a called appender throws a RuntimeException
Queue reading thread in AsyncAppender blocks if a called appender throws a Ru...
Status: RESOLVED FIXED
Product: Log4j
Classification: Unclassified
Component: Appender
1.2
Other other
: P3 normal
: ---
Assigned To: log4j-dev
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2003-09-09 09:17 UTC by Bojan
Modified: 2014-02-17 13:58 UTC (History)
1 user (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Bojan 2003-09-09 09:17:26 UTC
The inner class Dispatcher in org.apache.log4j.AsyncAppender runs as a thread 
and reads the logging event buffer which is filled by users of the 
AsyncAppender. The Dispatcher thread reads from this buffer and calls the 
append method of all previously added Appenders (in the run() method).

If one of this appenders throws a RuntimeException, it is not catched anywhere, 
and the endless loop in the run() method is exited and the thread dies. The 
logging event buffer is filled up until the buffer writer (in the main thread) 
blocks, which blocks the whole process. 
As Log4J usually is used as a trace module, such an error shall not block the 
whole productive process. It would be better if RuntimeExcpetions whould be 
catched, proberly traced and then continued.
Comment 1 j.c.yip 2004-01-20 22:17:27 UTC
From what I can tell, this should occur at
AppenderAttachableImpl.appendLoopOnAppenders.  Surround appender.doAppend(event)
with a try catch Runtime, log, and continue.  This will prevent a single faulty
appender from preventing logging to other appenders and killing the parent loop.
Comment 2 j.c.yip 2004-01-27 07:09:12 UTC
As in this:

try {
    appender.doAppend(event);
} catch (RuntimeException e) {
    LogLog.error("Error appending to " + appender, e);
}
Comment 3 Mark Womack 2005-07-01 23:09:55 UTC
1.2.12 candidate
Comment 4 Curt Arnold 2005-07-22 01:59:44 UTC
I'm a little troubled by the proposed resolution.  An Appender should not throw a run-time exception 
and the exception would not be caught if the appender was directly attached to the logger hierarchy.  
I'd really prefer not to suggest that an appender that throws run-time exceptions is tolerable as long as 
you wrap it in an async appender.  I'm not sure of the cost of the try/catch block in the async appender 
for the run-time exception that should never come, but it could be expensive enough to be noticable.

My current preference is to check dispatcher.isAlive() at the start of AsyncAppender.doAppend and if 
the dispatch thread has died to invoke the nested appenders synchronously.  This should result in 
similar behavior for both the sync and async other than your first exception is silent if wrapped in an 
async appender.
Comment 5 Curt Arnold 2005-07-22 21:57:19 UTC
Fixed on both 1.2 and CVS HEAD.  If thread is dead on entry to AsyncAppender.append (possibly from a 
earlier run-time exception), then logging event will be processed synchronously.