[ 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
