"Simon van der Sluis":https://jira.spring.io/secure/ViewProfile.jspa?name=svanders said:
BasicLookupStrategy opens an additional connection to the database for each ACL level of ACL parent that needs to be retrieved from the database.
The error is in the private inner class ProcessResultSet (implements ResultSetExtractor) in the method extractData(ResultSet), at the end of the method
// Lookup parents, adding Acls (with StubAclParents) to "acl" map if (parentIdsToLookup.size() > 0) { lookupPrimaryKeys(acls, parentIdsToLookup, sids); }
lookupPrimaryKeys(acls, parentIdsToLookup, sids); creates another ProcessResultSet which as per Spring JDBC template classes grabs another connection to the database. In a round about way this recursively uses one database connection per ACL -> parent ACL relationship to be looked up.
In our case we have some test data where we have an ACL with a hierarchy of 16 parents, so for each level of the hierarchy a new connection to the DB is used Our test set up had only 10 connections in the pool, which is how we found it.
I'm currently trying to fix this, as it's causing us problems. As part of the fix I'm writing some unit tests for BasicLookupStrategy (as there currently are none), not sure if I'll be able to get DB connection checking usage into the unit tests, but will investigate.
I'll add my fixes to this Issue when they are ready.
This issue should probably be added to http://opensource.atlassian.com/projects/spring/browse/SEC-532
Comment From: spring-projects-issues
["Simon van der Sluis":https://jira.spring.io/secure/ViewProfile.jspa?name=svanders] said:
I've made some changes to the BasicLookupStrategy. The recursive lookup for primary keys of parent ACLs has been moved out of ProcessResultSetMap#extractData(...) method, into lookupPrimaryKeys(...) itself, and lookupObjectIdentities(...) has been modified to call lookupPrimaryKeys(...) if needed. To allow this, ProcessResultSetMap#extractData(...) now returns a Set containing the parentIdsToLookup.
These changes mean that JdbcTemplate can close the result set and connection (thus returning it to the pool) before the next recursion. Previously it would start the recursion before JdbcTemplate was finished, and require as many concurrent connections as there were ACL parent relationships.
Changed files will be attached.
NOTE: To make the unit test be able to clear the ACL cache I had to split the AclCache configuration into 2 beans, 1 being the AclCache, the other being the underlying ehCache that needs to be cleared (emptied) during the test, so I've also attached the modified application context xml, which comes from core/src/test/resources//org/acegisecurity/acls/jdbc/applicationContext-test.xml
Also I didn't have time to think how / if the unit tests could be modified to monitor how many concurrent database connections are made at any one time.
Comment From: spring-projects-issues
["Simon van der Sluis":https://jira.spring.io/secure/ViewProfile.jspa?name=svanders] said:
There are also some additional depency injectable properties in BasicLookupStrategy, these are from an unfinished attempt to make the id method / data type of domain objects protected by ACLs configurable. This is unfinished, but the default values are getId with data type Long, as per the 1.0.3 release version, so the changes will (should) have no effect.
Comment From: spring-projects-issues
["Simon van der Sluis":https://jira.spring.io/secure/ViewProfile.jspa?name=svanders] said:
BasicLookupStrategy contained recursive parent acl lookup bug fix for concurrent DB connections.
Comment From: spring-projects-issues
["Simon van der Sluis":https://jira.spring.io/secure/ViewProfile.jspa?name=svanders] said:
BasicLookupStrategy unit tests for recursive parent acl lookup bug fix for concurrent DB connections.
Simply checks that parent are still looked up correctly
Comment From: spring-projects-issues
["Simon van der Sluis":https://jira.spring.io/secure/ViewProfile.jspa?name=svanders] said:
modified application context for LookupStrategyTest. has AclCache bean config split into two, to provide the ability to clear (empty) the underlying ehCache.
Comment From: spring-projects-issues
["Ben Alex":https://jira.spring.io/secure/ViewProfile.jspa?name=balex] said:
Modified BasicLookupStrategy to operate as per the patch, namely the ProcessResultSet.extractData(ResultSet) methods now returns a Set of AclIds that still remain to be looked up. The caller is then responsible for looking up those AclIds.
Tests did not require modification, and still pass.
SVN commit revision 2873.
Comment From: spring-projects-issues
Luke Taylor said:
Reopening to remove watchers because of spammer...
Comment From: sumanmitra
I know this issue is closed but I am having connection leak when using Hikari cp. I posted the details in stackoverflow. Can you guys please have a look? https://stackoverflow.com/questions/64695847/spring-security-acl-connection-leak