I'm guess I'm not following why the original code wasn't working. Ehcache is thread safe.
----- Original Message ----- From: "Emmanuel Lécharny" <[email protected]> To: [email protected] Sent: Sunday, March 6, 2016 3:28:07 PM Subject: Re: LDAP Connection Management Le 06/03/16 20:59, Chris Pike a écrit : > Yes, this is a showstopper, our servers are getting hung up every few days, > and this will prevent us from using the service in production. > > Can you point me to where data is getting written in these methods? I'm only > seeing reads. AFAIR, we have discussed this portion of code around 2 years ago. The problem is that we grab the relation graph from a cache, which might need to be loaded if it's not existent. The original piece of code was : private static SimpleDirectedGraph<String, Relationship> getGraph( String contextId ) { SimpleDirectedGraph<String, Relationship> graph = ( SimpleDirectedGraph<String, Relationship> ) roleCache .get( getKey( contextId ) ); if ( graph == null ) { graph = loadGraph( contextId ); } return graph; } CLearly, it wasn't protected against concurrent accesses. And we had some failures... Here is the comment I pushed when I proposed a patch for the issue : "o Fix for FC-38, using read and write locks. When we update the protected data, we release the read lock acquire the write lock, check that we should still update the data (another might have done that just after we released the read lock), then update the data, acquire the read lock again, release the write lock and finally release the read lock. It might look like complicated, because it is complex... Tests are passing." Here is the commit, ftr : http://git-wip-us.apache.org/repos/asf/directory-fortress-core/diff/40f6d2f2 Shawn added some later fix : http://git-wip-us.apache.org/repos/asf/directory-fortress-core/diff/804ca390 (FC-38) > > > > ----- Original Message ----- > From: "Shawn McKinney" <[email protected]> > To: [email protected] > Sent: Sunday, March 6, 2016 2:37:54 PM > Subject: Re: LDAP Connection Management > >> On Mar 6, 2016, at 1:08 PM, Chris Pike <[email protected]> wrote: >> >> I'm not an expert on concurrency, but I think the problem might be the code >> trying to upgrade the read lock to a write lock... >> >> http://stackoverflow.com/questions/464784/java-reentrantreadwritelocks-how-to-safely-acquire-write-lock > Similar, not an expert. This code has been reworked in the past couple of > years and needs to be again. > >> On Mar 6, 2016, at 1:08 PM, Chris Pike <[email protected]> wrote: >> >> Stepping back for a second, what is the purpose of using the lock? > > Thread safety on the graph datastructs (hier role and orgs). There is > potential for multiple threads accessing with reads and writes. This code > has been rewritten some time ago and needs to be again apparently. > > I’ve opened a jira to cover the work. Question is this a showstopper? > > Shawn
