Le 08/03/16 17:22, Shawn McKinney a écrit : >> On Mar 8, 2016, at 8:40 AM, Christopher Harm <[email protected]> wrote: >> >> The Javadocs state that upgrading from a readLock to a writeLock is not >> possible, and I've been reading other material that states that this is >> because that operation is deadlock prone. The function is pretty simple, in >> that it doesn't do anything inside the read lock after it's acquired the >> write lock. I suggest that we de-couple the read and write lock scopes as >> such.... >> >> private static SimpleDirectedGraph<String, Relationship> getGraph(String >> contextId) { >> ReadWriteLock hierLock = HierUtil.getLock(contextId, HierUtil.Type.ROLE); >> String key = getKey(contextId); >> >> SimpleDirectedGraph<String, Relationship> graph = null; >> hierLock.readLock().lock(); >> try { >> graph = (SimpleDirectedGraph<String, Relationship>) roleCache.get(key); >> if (graph != null) { >> return graph; >> } >> } finally { >> hierLock.readLock().unlock(); >> } >> >> hierLock.writeLock().lock(); >> try { >> graph = (SimpleDirectedGraph<String, Relationship>) roleCache.get(key); >> if (graph != null) { >> return graph; >> } >> >> graph = loadGraph(contextId); >> >> return graph; >> } finally { >> hierLock.writeLock().unlock(); >> } >> } > +1 looks good. (Much simpler)
I think Shawn is right when he said that we first should check if the protection against concurrent access is really needed. That's a bit foggy for me (it was 2 years ago), and I'm afraid that I suggested to protect this code just for the sake of it. I'd liek to get a second review of this specific piece of code which *may* not need synchronization at all...
