We as many others have been bitten by the synchronization nuances of log4j.
Attached is a proposed patch to reduce the scope of the problem. No,
this is not yet regression tested due to the issues I e-mailed about
previously with the regression test suite, but I'd like to spur some
discussion as to what's wrong with such a change, etc.
I was considering using Java 5 features here and could, but the key
issue here does not appear to require such fanciness to address.
--
Jess Holle
# This patch file was generated by NetBeans IDE
# Following Index: paths are relative to: D:\User
Profiles\jessh.PTCNET\Desktop\log4jsvn
# This patch can be applied using context Tools: Patch action on respective
folder.
# It uses platform neutral UTF-8 encoding and \n newlines.
# Above lines and this line are ignored by the patching process.
Index: src/main/java/org/apache/log4j/Category.java
--- src/main/java/org/apache/log4j/Category.java Base (BASE)
+++ src/main/java/org/apache/log4j/Category.java Locally Modified (Based On
LOCAL)
@@ -200,16 +200,15 @@
int writes = 0;
for(Category c = this; c != null; c=c.parent) {
- // Protected against simultaneous call to addAppender, removeAppender,...
- synchronized(c) {
- if(c.aai != null) {
- writes += c.aai.appendLoopOnAppenders(event);
+ // copy into field, so null check and usage are against same value
+ AppenderAttachableImpl cAai = c.aai;
+ if (cAai != null) {
+ writes += cAai.appendLoopOnAppenders(event);
}
- if(!c.additive) {
+ if (!c.additive) {
break;
}
}
- }
if(writes == 0) {
repository.emitNoAppenderWarning(this);
Index: src/main/java/org/apache/log4j/helpers/AppenderAttachableImpl.java
--- src/main/java/org/apache/log4j/helpers/AppenderAttachableImpl.java Base
(BASE)
+++ src/main/java/org/apache/log4j/helpers/AppenderAttachableImpl.java Locally
Modified (Based On LOCAL)
@@ -53,21 +53,26 @@
}
/**
- Call the <code>doAppend</code> method on all attached appenders. */
+ Call the <code>doAppend</code> method on all attached appenders.
+ Implementations should be thread safe against other methods on this class.
+ */
public
int appendLoopOnAppenders(LoggingEvent event) {
- int size = 0;
- Appender appender;
-
- if(appenderList != null) {
- size = appenderList.size();
+ // copy into field, so null check and usage are against same value
+ Vector appenderList = this.appenderList;
+ if ( appenderList != null ) {
+ Appender appenders[];
+ synchronized ( appenderList ) {
+ appenders = (Appender[]) appenderList.toArray( new
Appender[appenderList.size()] );
+ }
+ int size = appenders.length;
for(int i = 0; i < size; i++) {
- appender = (Appender) appenderList.elementAt(i);
- appender.doAppend(event);
+ appenders[i].doAppend(event);
}
- }
return size;
}
+ return 0;
+ }
/**
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
