From 6014070431c31cd92d30a0f5200cfa6971134c43 Mon Sep 17 00:00:00 2001 From: Hynek Mlnarik Date: Wed, 5 Apr 2023 12:47:47 +0200 Subject: [PATCH] Fix memory leak in LDAP The caching in LDAP stores and reuses the session at the time of creating `LDAPIdentityStore`. On top of that, there is not much cached, since apart from the session which must not be part of long-lived cache, only config is cached in the objects which is anyway always recomputed. The cache for the LDAP still retains the LDAPConfig to keep the `logLDAPConfig` call upon config change. Closes: #19396 --- .../ldap/LDAPIdentityStoreRegistry.java | 27 +++++-------------- 1 file changed, 6 insertions(+), 21 deletions(-) diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPIdentityStoreRegistry.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPIdentityStoreRegistry.java index d224940605..bc953c6170 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPIdentityStoreRegistry.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPIdentityStoreRegistry.java @@ -35,11 +35,9 @@ public class LDAPIdentityStoreRegistry { private static final Logger logger = Logger.getLogger(LDAPIdentityStoreRegistry.class); - private Map ldapStores = new ConcurrentHashMap<>(); + private final Map ldapStores = new ConcurrentHashMap<>(); public LDAPIdentityStore getLdapStore(KeycloakSession session, ComponentModel ldapModel, Map configDecorators) { - LDAPIdentityStoreContext context = ldapStores.get(ldapModel.getId()); - // Ldap config might have changed for the realm. In this case, we must re-initialize MultivaluedHashMap configModel = ldapModel.getConfig(); LDAPConfig ldapConfig = new LDAPConfig(configModel); @@ -50,14 +48,13 @@ public class LDAPIdentityStoreRegistry { decorator.updateLDAPConfig(ldapConfig, mapperModel); } - if (context == null || !ldapConfig.equals(context.config)) { + LDAPConfig cachedConfig = ldapStores.get(ldapModel.getId()); + if (cachedConfig == null || !ldapConfig.equals(cachedConfig)) { logLDAPConfig(session, ldapModel, ldapConfig); - - LDAPIdentityStore store = createLdapIdentityStore(session, ldapConfig); - context = new LDAPIdentityStoreContext(ldapConfig, store); - ldapStores.put(ldapModel.getId(), context); + ldapStores.put(ldapModel.getId(), ldapConfig); } - return context.store; + + return createLdapIdentityStore(session, ldapConfig); } // Don't log LDAP password @@ -97,16 +94,4 @@ public class LDAPIdentityStoreRegistry { } System.setProperty(name, value); } - - - private static class LDAPIdentityStoreContext { - - private LDAPIdentityStoreContext(LDAPConfig config, LDAPIdentityStore store) { - this.config = config; - this.store = store; - } - - private LDAPConfig config; - private LDAPIdentityStore store; - } }