directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Emmanuel Lecharny (JIRA)" <directory-...@incubator.apache.org>
Subject [jira] Commented: (DIRLDAP-71) CachingNormalizer exhibits concurrency flaw under load.
Date Mon, 21 Nov 2005 19:23:41 GMT
    [ http://issues.apache.org/jira/browse/DIRLDAP-71?page=comments#action_12358185 ] 

Emmanuel Lecharny commented on DIRLDAP-71:
------------------------------------------

Well, the LRUMap has been taken from commons.collections. This is a non-synchronized class,
and I don't know what were the reasons for not using the native  version, so I felt quite
not guilty modifying it.

We have two places in the code where we use this class : 
- org.apache.ldap.server.partition.impl.btree.jdbm.JdbmIndex
and
- org.apache.ldap.common.schema.CachingNormalizer

I bet that those two class will experience race condition when the server will be overloaded,
so this is urgent to fix this issue.

I think that we have three possibilities :
1) transform this LRUMap to a synchronized version of the class (subclassing the LRUMap class);
2) synchronize the access to this class in the two previously names classes (JdbmIndex and
CachingNormalizer)
3) rename the class to SynchronizedLRUMap, and correctly implement synchronization.

I personnaly think that the first solution is the best, for many reasons, but mainly because
I don't see why we should have our own version of LRUMap when you already have 2 in commons-collection
(yes, two : org.apache.commons.collections.LRUMap and org.apache.commons.collections.map.LRUMap
:| ), so surcharging one of the two existing seems simple to me. Second, a cache is something
that will often used in a multi-threaded environment, so having to check that it is correctly
synchronized once is better than to do the work twice.

Of course, this is just MHO.

I have modified the LRUMap in order to fix the put() method (I forgot to synchronized one
part of the code), and also changed the protected method to private and add a 'final' keyword
to the class.

Those modifications are just done in order to have a working server, but I think we must agree
on the best solution before freezing the code. Any idea is welcome !

> CachingNormalizer exhibits concurrency flaw under load.
> -------------------------------------------------------
>
>          Key: DIRLDAP-71
>          URL: http://issues.apache.org/jira/browse/DIRLDAP-71
>      Project: Directory LDAP
>         Type: Bug
>   Components: Common
>     Versions: 0.9.3
>     Reporter: Jacob S. Barrett
>     Assignee: Emmanuel Lecharny
>  Attachments: CachingNormalizer-Concurrency.patch
>
> CachingNormalizer doesn't have it's cache protected from concurrent modifications.  Under
load a state is reached where cache.containsKey(key) is true but the result of cache.get(key)
results in an unexpected null value and thus a NullPointerException.  This is really only
reached when you have more than threads than the max cache size and therefore items in the
cache are being purged.  The attached path synchronizes the cache.  It would be better to
use a R/W lock from JSR 166 backport or something similar.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


Mime
View raw message