You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by bu...@apache.org on 2005/12/23 10:23:15 UTC
DO NOT REPLY [Bug 38026] New: - [logging] LogFactory is not thread-safe
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=38026>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=38026
Summary: [logging] LogFactory is not thread-safe
Product: Commons
Version: unspecified
Platform: Other
OS/Version: other
Status: NEW
Severity: normal
Priority: P2
Component: Logging
AssignedTo: commons-dev@jakarta.apache.org
ReportedBy: maarten.coene@hotpop.com
Hi,
the LogFactory class is not thread-safe. All access to the factories variable
should be in synchronized code. This is sometimes the case (like in the
LogFactory.releasXXX() methods), but in the LogFactory.getFactory() method,
this variable is accessed in a non-synchronized way.
regards,
Maarten
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
DO NOT REPLY [Bug 38026] - [logging] LogFactory is not thread-safe
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=38026>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=38026
rdonkin@apache.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution| |FIXED
------- Additional Comments From rdonkin@apache.org 2006-01-18 22:59 -------
I agree that the performance advantages outweight the downside but it should be
documented. I've committed a note to the getFactory method javadocs which should
be suitable.
Please check and if not satisfactory the please reopen and attach a patch for
the documentation.
Robert
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
DO NOT REPLY [Bug 38026] - [logging] LogFactory is not thread-safe
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=38026>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=38026
------- Additional Comments From maarten.coene@hotpop.com 2006-01-18 20:49 -------
The releaseXXX() methods are safe, but the getFactory() is not.
It is possible that 2 threads calling the getFactory() method will receive
different LogFactory instances for the same classloader. I'm not saying that
you should make getFactory() synchronized, but you should say more explicitly
in the javadoc that the getFactory() method not necessarely returns the same
factory instance in a multithreaded environment.
Consider the following simple scenario:
No Factory has been created yet. 2 Different threads have just been started.
Thread1 calls LogFactory.getFactory(). This method will look in the cache, but
since it is the first time the factory is retrieved, the cache will return
null and the thread will continue creating the factory instance. In the middle
of the getFactory() method, thread1 is suspended and thread2 becomes active.
Thread2 calls getFactory(). The cache still returns null! (Thread1 didn't put
the factory instance in the cache yet). So thread2 will continue creating a
new factory instance. Both threads will end up having their own instance of a
LogFactory. One of them is cached, the other not.
In most cases, this is no big deal: it's just an instance of a factory that
has been created too much. This won't cause problems in > 99% of the
applications.
But it could get worse: if you now call LogFactory.releaseAll(), one instance
of the factory won't be released at all (because it's not present in the
cache)! If your factory implementation consumes a lot of resources, this could
cause a serious negative impact on your system.
regards,
Maarten
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
DO NOT REPLY [Bug 38026] - [logging] LogFactory is not thread-safe
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=38026>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=38026
------- Additional Comments From nbeyer@kc.rr.com 2006-01-18 05:58 -------
The Hashtable is synchronized using synchronized keywords on the methods, which
means that that lock object is the instance itself. As such, everything is
waiting on the same lock, so releaseAll is safe because it synchronizes on the
instance before manipulating anything and the other mutation points, like the
cacheFactory method would be blocked during a Hashtable.put if releaseAll were
in process.
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
DO NOT REPLY [Bug 38026] - [logging] LogFactory is not thread-safe
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=38026>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=38026
------- Additional Comments From skitching@apache.org 2006-01-18 05:18 -------
I don't believe this issue is valid. The Hashtable javadoc specifies that this
class is internally synchronized (ie already thread-safe) which will prevent
race conditions between concurrent calls to get/put.
There is potentially an issue with the removeAll method, as a
ConcurrentModificationException could occur if one thread is in removeAll while
another is adding an entry to the table. However removeAll should only ever be
called by the container during container shutdown so this is not expected to
ever be possible in practice.
Adding synchronisation everywhere just to avoid a theoretical conflict with
removeAll isn't a good idea; that's a significant performance hit.
If I've got this wrong, please let me know. Otherwise I will close this issue in
a couple of days.
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org