RE: Log4JLogger Not Thread Safe?

[robert burrell donkin <ro...@blueyonder.co.uk>]

On Tue, 2005-11-29 at 13:31 -0600, Wilson, Chris (SAHQ I/S) wrote:
> To preface, let me state I am certainly not a JVM, compiler, or
> threading expert!  

that makes two of us!

however, the first step to knowledge is the acknowledgement of
ignorance...

> The link you provided
> (http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html)
> and this article by Doug Lea (http://gee.cs.oswego.edu/dl/cpj/jmm.html)
> talk about the compiler or memory system reordering statements as long
> as "as-if-serial" semantics are preserved.

that's correct: as far as i can tell (at least for older JVMs) there are
no guarantees that construction will be completed before the object is
exposed to other threads.

http://www-128.ibm.com/developerworks/java/library/j-jtp02244.html and
http://www-128.ibm.com/developerworks/java/library/j-jtp03304/ are good
references.

(i'm sure i used to know this: but it's always a pleasure to dip back
into Concurrent Programming in Java by Doug Lea)

> Since getLogger() is not synchronized, I suppose the only danger could
> be the following scenario:
> 
> Thread A enters getLogger() and determines that the logger instance is
> null (because the class has been recently deserialized).  Thread A
> proceeds to get an instance of logger from the underlying logging
> system.  Because getLogger() is not synchronized, a reference to the new
> logger instance may be set in the local logger variable before the
> object is fully constructed.  At this point Thread A gets swapped out
> for another thread to run.
> 
> Thread B enters getLogger() and sees that the logger instance is not
> null and happily returns it for use.  The code using the logger instance
> calls some methods on it before it has been completely setup (still
> waiting to happen in Thread A which is waiting to run again).
> 
> At this point, what happens?  The behavior of the logger instance is
> undefined as I understand it.  It may not fail, but could possibly act
> in a strange way?

what happens will depends on the internal behaviour of log4j logger but
it's probably wiser to assume the worst and address this issue. 

i agree that should be fixed and think that overriding the readObject
method is probably the right way to do it. if you'd like to submit a
patch (through bugzilla) i'll take a look at committing it. otherwise,
i'll take a look at the weekend (when i hope to do the preparation work
for a 1.1 JCL release).

note that the string constructor (which calls getLogger) potentially
suffers from the same problem. i really don't want to synchronize
getLogger() - it is called too much. would need to review the usage to
determine whether this is safe as used in JCL. should probably put
something in the documentation for the constructor warning that it's not
thread safe. again, if you want to lend a hand please do :) 

- robert


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org