Sorry, trying to understand... in your example, if the second thread gets null and calls loadGraph a second time, what problem does that cause (other than loading data twice)? Asked another way, what about loadGraph is not thread safe?
----- Original Message ----- From: "Emmanuel Lécharny" <[email protected]> To: [email protected] Sent: Monday, March 7, 2016 1:40:50 AM Subject: Re: LDAP Connection Management Le 07/03/16 03:36, Chris Pike a écrit : > So I changed it to this locally... > > String key = getKey( contextId ); > > LOG.info("Getting graph for key " + contextId); > > SimpleDirectedGraph<String, Relationship> graph = ( > SimpleDirectedGraph<String, Relationship> ) roleCache > .get( key ); > > if(graph == null){ > LOG.info("Graph was null, creating... " + contextId); > return loadGraph( contextId ); > } > else{ > LOG.info("Graph found in cache, returning..."); > return graph; > } > > That seems to have fixed the problem. It will be a few days before I can > change the other methods that use the pattern and test it out on our severs. > I'm not following why it's a problem if multiple threads load the graph data > structure. That's pretty simple. Have a look at the loadGraph( String contextId ) method : private static SimpleDirectedGraph<String, Relationship> loadGraph( String contextId ) { ... graph = HierUtil.buildGraph( hier ); // Step 1 roleCache.put( getKey( contextId ), graph ); // Step 2 and the getGrapgh method : private static SimpleDirectedGraph<String, Relationship> getGraph( String contextId ) { ReadWriteLock hierLock = HierUtil.getLock( contextId, HierUtil.Type.ROLE ); String key = getKey( contextId ); try { hierLock.readLock().lock(); SimpleDirectedGraph<String, Relationship> graph = ( SimpleDirectedGraph<String, Relationship> ) roleCache // Step 3 .get( key ); if ( graph == null ) { try { hierLock.readLock().unlock(); hierLock.writeLock().lock(); graph = loadGraph( contextId ); One thread can update the graph in Step 1 while a second graph try to get it from teh cache in Step 3. Obviously, there is a possibility that the second thread get null becauase the first thread hasn't yet updated the cache in Step 3, leading to the loadGraph method to be called by the second thread. the fact that ehCache is thread safe is not protecting in this case. There is definitively a need for some lokcs in the getGraph method, now it's critical to do it correctly. Can you provide a thread dump done when you see a lock in your application ? That would help understanding why it's blocking with th original code.
