commons-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From robert burrell donkin <robertburrelldon...@blueyonder.co.uk>
Subject Re: Log4JLogger Not Thread Safe?
Date Tue, 29 Nov 2005 19:09:21 GMT
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: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org


Mime
View raw message