Le 09/03/16 16:33, Shawn McKinney a écrit : >> 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?
The real question is : do we have to protect #7 at all ? graph is not a global variable, so there is no concurrent access issue here. Worse case scenario : two threads call loadGraph() with the same ContextID, but here, that mean we should check the conncury in loadgraph, not in getGraph(). That of course assume roleCache itself to be thread-safe.
