Yes, this is what we are currently testing on our servers. https://github.com/PennState/directory-fortress-core-1/blob/release/src/main/java/org/apache/directory/fortress/core/impl/RoleUtil.java
I still don't understand why #7 needs guarded, but it is guarded by #4. Since the cache is a blocking cache, it returns null on first hit, then blocks until a value is put in the cache. By default, the blocking cache will block forever, so I set the timeout to 60 seconds as a failsafe. It is possible (however unlikely) that multiple gets could timeout after 60 seconds and result in simultaneous loadGraph calls, so I also made the loadGraph method synchronized. ----- Original Message ----- From: "Shawn McKinney" <[email protected]> To: [email protected] Sent: Wednesday, March 9, 2016 10:33:45 AM Subject: Re: LDAP Connection Management > On Mar 8, 2016, at 2:08 PM, Chris Pike <[email protected]> wrote: > > I tried implementing the suggested code, and while it works in eclipse, when > running the service, the first run passes, but all subsequent requests hang > while getting object from the cache. > > After looking more closely at the caching code, it is already using a > blocking cache, which by default will return null on the first get and lock > indefinitely on subsequent gets until the cache is populated. Perhaps it's > the combination of our locks and ehcache's locks are causing issues??? I do > think the blocking cache makes our lock code unnecessary as the cache is > already handling multiple threads. Thoughts? > > I would like to add ability to configure the maxtimeout on the blocking cache > so indefinite blocks can't happen. If the timeout triggers on multiple gets > to the blocking cache, it is possible that load graph could be called > simultaneously. Perhaps this method needs to be synchronized for this > hopefully unlikely scenario. Let’s dissect the function in question: private static SimpleDirectedGraph<String, Relationship> getGraph( String contextId ) { 1 ReadWriteLock hierLock = HierUtil.getLock( contextId, HierUtil.Type.ROLE ); 2 String key = getKey( contextId ); try { 3 hierLock.readLock().lock(); 4 SimpleDirectedGraph<String, Relationship> graph = ( SimpleDirectedGraph<String, Relationship> ) roleCache .get( key ); if ( graph == null ) { try { 5 hierLock.readLock().unlock(); 6 hierLock.writeLock().lock(); 7 graph = loadGraph( contextId ); 8 hierLock.readLock().lock(); } finally { 9 hierLock.writeLock().unlock(); } } return graph; } finally { 10 hierLock.readLock().unlock(); } } I placed numbers in front of statements that are significant to this conversation. (Hopefully that formatting is preserved.) You are proposing removing statements 1,3,5,6,8,9 and 10 - right? So the question becomes what is left in place to guard statement #7? Entry to it must be synchronized to one thread at a time. Are you suggesting that ehcache already maintains synchronicity via either #2, #4, or both? Shawn
