Cache LDAP attributes to go easy on memory footprint. Where I saw this problem, each LDAP BasicAttributes is about 30K big, and a programmatic client logged in 7000 times, resulting in 140MB of memory consumption just by LDAP attributes.

git-svn-id: https://hudson.dev.java.net/svn/hudson/trunk/hudson/main@34211 71c3de6d-444a-0410-be80-ed276b4c234a

Originally-Committed-As: 6d492940d9337ecf36051363c916639e2f5bda0a

core/src/main/java/hudson/security/LDAPSecurityRealm.java

@@ -52,6 +52,7 @@ import org.acegisecurity.userdetails.UserDetailsService;
 import org.acegisecurity.userdetails.UsernameNotFoundException;
 import org.acegisecurity.userdetails.ldap.LdapUserDetails;
 import org.acegisecurity.userdetails.ldap.LdapUserDetailsImpl;
+import org.apache.commons.collections.map.LRUMap;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.QueryParameter;
 import org.springframework.dao.DataAccessException;
@@ -61,6 +62,7 @@ import javax.naming.Context;
 import javax.naming.NamingException;
 import javax.naming.directory.Attribute;
 import javax.naming.directory.Attributes;
+import javax.naming.directory.BasicAttributes;
 import javax.naming.directory.DirContext;
 import javax.naming.directory.InitialDirContext;
 import java.io.IOException;
@@ -339,7 +341,7 @@ public class LDAPSecurityRealm extends AbstractPasswordBasedSecurityRealm {
 
         BeanBuilder builder = new BeanBuilder();
         builder.parse(Hudson.getInstance().servletContext.getResourceAsStream("/WEB-INF/security/LDAPBindSecurityRealm.groovy"),binding);
-        final WebApplicationContext appContext = builder.createApplicationContext();
+        WebApplicationContext appContext = builder.createApplicationContext();
 
         ldapTemplate = new LdapTemplate(findBean(InitialDirContextFactory.class, appContext));
 
@@ -406,20 +408,42 @@ public class LDAPSecurityRealm extends AbstractPasswordBasedSecurityRealm {
     public static class LDAPUserDetailsService implements UserDetailsService {
         public final LdapUserSearch ldapSearch;
         public final LdapAuthoritiesPopulator authoritiesPopulator;
+        /**
+         * {@link BasicAttributes} in LDAP tend to be bulky (about 20K at size), so interning them
+         * to keep the size under control. When a programmatic client is not smart enough to
+         * reuse a session, this helps keeping the memory consumption low.
+         */
+        private final LRUMap attributesCache = new LRUMap(32);
+
         LDAPUserDetailsService(WebApplicationContext appContext) {
             ldapSearch = findBean(LdapUserSearch.class, appContext);
             authoritiesPopulator = findBean(LdapAuthoritiesPopulator.class, appContext);
         }
-        public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException, DataAccessException {
+
+        LDAPUserDetailsService(LdapUserSearch ldapSearch, LdapAuthoritiesPopulator authoritiesPopulator) {
+            this.ldapSearch = ldapSearch;
+            this.authoritiesPopulator = authoritiesPopulator;
+        }
+
+        public LdapUserDetails loadUserByUsername(String username) throws UsernameNotFoundException, DataAccessException {
             try {
                 LdapUserDetails ldapUser = ldapSearch.searchForUser(username);
                 // LdapUserSearch does not populate granted authorities (group search).
                 // Add those, as done in LdapAuthenticationProvider.createUserDetails().
                 if (ldapUser != null) {
                     LdapUserDetailsImpl.Essence user = new LdapUserDetailsImpl.Essence(ldapUser);
+
+                    // intern attributes
+                    Attributes v = ldapUser.getAttributes();
+                    if (v instanceof BasicAttributes) {// BasicAttributes.equals is what makes the interning possible
+                        Attributes vv = (Attributes)attributesCache.get(v);
+                        if (vv==null)   attributesCache.put(v,vv=v);
+                        user.setAttributes(vv);
+                    }
+
                     GrantedAuthority[] extraAuthorities = authoritiesPopulator.getGrantedAuthorities(ldapUser);
-                    for (int i = 0; i < extraAuthorities.length; i++) {
+                    for (GrantedAuthority extraAuthority : extraAuthorities) {
-                        user.addAuthority(extraAuthorities[i]);
+                        user.addAuthority(extraAuthority);
                     }
                     ldapUser = user.createUserDetails();
                 }

test/src/test/groovy/hudson/security/LDAPSecurityRealmTest.groovy

@@ -24,6 +24,12 @@
 package hudson.security
 
 import org.jvnet.hudson.test.HudsonTestCase
+import hudson.security.LDAPSecurityRealm.LDAPUserDetailsService
+import org.acegisecurity.ldap.LdapUserSearch
+import org.acegisecurity.userdetails.ldap.LdapUserDetailsImpl
+import javax.naming.directory.BasicAttributes
+import org.acegisecurity.providers.ldap.LdapAuthoritiesPopulator
+import org.acegisecurity.GrantedAuthority
 
 /**
  *
@@ -39,4 +45,24 @@ public class LDAPSecurityRealmTest extends HudsonTestCase {
         hudson.securityRealm = new LDAPSecurityRealm("ldap.itd.umich.edu",null,null,null,null,null,null);
         println hudson.securityRealm.securityComponents // force the component creation
     }
+
+    void testSessionStressTest() {
+        LDAPUserDetailsService s = new LDAPUserDetailsService(
+                { username ->
+                    def e = new LdapUserDetailsImpl.Essence();
+                    e.username = username;
+                    def ba = new BasicAttributes()
+                    ba.put("test",username);
+                    ba.put("xyz","def");
+                    e.attributes = ba;
+                    return e.createUserDetails();
+                } as LdapUserSearch,
+                { details -> new GrantedAuthority[0] } as LdapAuthoritiesPopulator);
+        def d1 = s.loadUserByUsername("me");
+        def d2 = s.loadUserByUsername("you");
+        def d3 = s.loadUserByUsername("me");
+        // caching should reuse the same attributes
+        assertSame(d1.attributes,d3.attributes);
+        assertNotSame(d1.attributes,d2.attributes);
+    }
 }