To preface, let me state I am certainly not a JVM, compiler, or threading expert! 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.
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? Chris -----Original Message----- From: robert burrell donkin [mailto:[EMAIL PROTECTED] Sent: Tuesday, November 29, 2005 1:09 PM To: Jakarta Commons Users List Subject: Re: Log4JLogger Not Thread Safe? On Mon, 2005-11-28 at 10:15 -0600, Wilson, Chris (SAHQ I/S) wrote: > Hello, > > I was looking at the source code of Log4JLogger and it seems to me that > it may not be thread safe after deserialization. > > The implementation holds a transient variable called logger which is > initialized at construction time. There is no synchronized access to > the logger variable anywhere in the class. The classes encapsulates all > access to the logger variable via the getLogger() method which > optionally initializes it if it's null. > > Since the logger variable is transient, when a Log4JLogger instance is > deserialized, the logger variable will be null and thus reinitialized > the next time getLogger() is called. If a Log4JLogger instance is held > as a static variable in a containing class (a standard commons logging > pattern), it would be possible for two threads to call getLogger() at > the same time which could lead to a thread unsafe scenario since > getLogger() is not synchronized. > > Even if the underlying log implementation is thread safe (which Log4J > is), this class accesses and stores a reference to its returned logger > in a thread unsafe way. > > Unless I'm missing something, it seems this class should protect access > to the logger variable if it is to be dynamically created via > getLogger() or override the readObject() method so that it's created > during deserialization (which I'm assuming would be thread safe as the > JVM would release the instance for use until it had been completely > deserialized). i've take a look and here's how i see things... since the class is unsynchronized, it is likely that concurrent use cases could be developed without the need for serialization. (there are a lot of oddities in java. for example see http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html.) JCL is (for better or worse) one of the most widely used java libraries and is now approaching maturity after many years of use. though it has it's deficiencies, i would have expected any synchronization issues to have emerged by now. for me, the key question is: what are the possible scenarios when two threads concurrently enter getLogger() and are any of these unsafe? luckily, the method is very simple. the worst case scenario (i can see) results in two threads both setting the logger to a non-null value. this would be safe but a little inefficient. but if you think i'm wrong or my arguments seem unconvincing, please take this as a challenge to create a unit test that proves log4j logger is thread unsafe :) - robert --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
