From 898cf8056dd4e7d11a7dfb559e06a4215fce8eb2 Mon Sep 17 00:00:00 2001
From: Yannick Lecaillez <yannick.lecaillez@forgerock.com>
Date: Thu, 30 Jun 2016 08:31:23 +0000
Subject: [PATCH] OPENDJ-3192: Memory leak in dn and AttributeDescription cache
---
opendj-core/src/main/java/org/forgerock/opendj/ldap/DN.java | 36 ++++++------------
opendj-core/src/main/java/org/forgerock/opendj/ldap/AttributeDescription.java | 55 +++++++++++----------------
2 files changed, 35 insertions(+), 56 deletions(-)
diff --git a/opendj-core/src/main/java/org/forgerock/opendj/ldap/AttributeDescription.java b/opendj-core/src/main/java/org/forgerock/opendj/ldap/AttributeDescription.java
index f735c95..56e4e55 100644
--- a/opendj-core/src/main/java/org/forgerock/opendj/ldap/AttributeDescription.java
+++ b/opendj-core/src/main/java/org/forgerock/opendj/ldap/AttributeDescription.java
@@ -26,13 +26,13 @@
import java.util.Map;
import java.util.SortedSet;
import java.util.TreeSet;
-import java.util.WeakHashMap;
import org.forgerock.i18n.LocalizableMessage;
import org.forgerock.i18n.LocalizedIllegalArgumentException;
import org.forgerock.opendj.ldap.schema.AttributeType;
import org.forgerock.opendj.ldap.schema.Schema;
import org.forgerock.opendj.ldap.schema.UnknownSchemaElementException;
+import org.forgerock.util.Pair;
import org.forgerock.util.Reject;
import com.forgerock.opendj.util.ASCIICharProp;
@@ -334,12 +334,19 @@
}
- private static final ThreadLocal<WeakHashMap<Schema, Map<String, AttributeDescription>>> CACHE =
- new ThreadLocal<WeakHashMap<Schema, Map<String, AttributeDescription>>>() {
-
+ private static final ThreadLocal<Map<String, Pair<Schema, AttributeDescription>>> CACHE =
+ new ThreadLocal<Map<String, Pair<Schema, AttributeDescription>>>() {
+ @SuppressWarnings("serial")
@Override
- protected WeakHashMap<Schema, Map<String, AttributeDescription>> initialValue() {
- return new WeakHashMap<>();
+ protected Map<String, Pair<Schema, AttributeDescription>> initialValue() {
+ return new LinkedHashMap<String, Pair<Schema, AttributeDescription>>(
+ ATTRIBUTE_DESCRIPTION_CACHE_SIZE, 0.75f, true) {
+ @Override
+ protected boolean removeEldestEntry(
+ final Map.Entry<String, Pair<Schema, AttributeDescription>> eldest) {
+ return size() > ATTRIBUTE_DESCRIPTION_CACHE_SIZE;
+ }
+ };
}
};
@@ -767,38 +774,22 @@
* If {@code attributeDescription} or {@code schema} was
* {@code null}.
*/
- @SuppressWarnings("serial")
public static AttributeDescription valueOf(final String attributeDescription,
final Schema schema) {
Reject.ifNull(attributeDescription, schema);
// First look up the attribute description in the cache.
- final WeakHashMap<Schema, Map<String, AttributeDescription>> threadLocalMap = CACHE.get();
- Map<String, AttributeDescription> schemaLocalMap = threadLocalMap.get(schema);
-
- AttributeDescription ad = null;
- if (schemaLocalMap == null) {
- schemaLocalMap =
- new LinkedHashMap<String, AttributeDescription>(
- ATTRIBUTE_DESCRIPTION_CACHE_SIZE, 0.75f, true) {
- @Override
- protected boolean removeEldestEntry(
- final Map.Entry<String, AttributeDescription> eldest) {
- return size() > ATTRIBUTE_DESCRIPTION_CACHE_SIZE;
- }
- };
- threadLocalMap.put(schema, schemaLocalMap);
- } else {
- ad = schemaLocalMap.get(attributeDescription);
+ final Map<String, Pair<Schema, AttributeDescription>> threadLocalCache = CACHE.get();
+ Pair<Schema, AttributeDescription> ad = threadLocalCache.get(attributeDescription);
+ // WARNING: When we'll support multiple schema, this schema equality check will be a problem
+ // for heavily used core attributes like "cn" which will be inherited in any sub-schema.
+ // See OPENDJ-3191
+ if (ad == null || ad.getFirst() != schema) {
+ // Cache miss: decode and cache.
+ ad = Pair.of(schema, valueOf0(attributeDescription, schema));
+ threadLocalCache.put(attributeDescription, ad);
}
-
- // Cache miss: decode and cache.
- if (ad == null) {
- ad = valueOf0(attributeDescription, schema);
- schemaLocalMap.put(attributeDescription, ad);
- }
-
- return ad;
+ return ad.getSecond();
}
private static int skipTrailingWhiteSpace(final String attributeDescription, int i,
diff --git a/opendj-core/src/main/java/org/forgerock/opendj/ldap/DN.java b/opendj-core/src/main/java/org/forgerock/opendj/ldap/DN.java
index b165e0d..f1cd46d 100644
--- a/opendj-core/src/main/java/org/forgerock/opendj/ldap/DN.java
+++ b/opendj-core/src/main/java/org/forgerock/opendj/ldap/DN.java
@@ -20,9 +20,9 @@
import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.Map;
+import java.util.Map.Entry;
import java.util.NoSuchElementException;
import java.util.UUID;
-import java.util.WeakHashMap;
import org.forgerock.i18n.LocalizedIllegalArgumentException;
import org.forgerock.opendj.ldap.schema.CoreSchema;
@@ -68,13 +68,18 @@
*/
private static final int DN_CACHE_SIZE = 32;
- private static final ThreadLocal<WeakHashMap<Schema, Map<String, DN>>> CACHE =
- new ThreadLocal<WeakHashMap<Schema, Map<String, DN>>>() {
+ private static final ThreadLocal<Map<String, DN>> CACHE = new ThreadLocal<Map<String, DN>>() {
+ @SuppressWarnings("serial")
+ @Override
+ protected Map<String, DN> initialValue() {
+ return new LinkedHashMap<String, DN>(DN_CACHE_SIZE, 0.75f, true) {
@Override
- protected WeakHashMap<Schema, Map<String, DN>> initialValue() {
- return new WeakHashMap<>();
+ protected boolean removeEldestEntry(Entry<String, DN> eldest) {
+ return size() > DN_CACHE_SIZE;
}
};
+ }
+ };
/**
* Returns the LDAP string representation of the provided DN attribute value
@@ -230,9 +235,9 @@
}
// First check if DN is already cached.
- final Map<String, DN> cache = getCache(schema);
+ final Map<String, DN> cache = CACHE.get();
final DN cachedDN = cache.get(dn);
- if (cachedDN != null) {
+ if (cachedDN != null && cachedDN.schema == schema) {
return cachedDN;
}
@@ -307,23 +312,6 @@
return new DN(schema, parent, rdn);
}
- @SuppressWarnings("serial")
- private static Map<String, DN> getCache(final Schema schema) {
- final WeakHashMap<Schema, Map<String, DN>> threadLocalMap = CACHE.get();
- Map<String, DN> schemaLocalMap = threadLocalMap.get(schema);
-
- if (schemaLocalMap == null) {
- schemaLocalMap = new LinkedHashMap<String, DN>(DN_CACHE_SIZE, 0.75f, true) {
- @Override
- protected boolean removeEldestEntry(final Map.Entry<String, DN> e) {
- return size() > DN_CACHE_SIZE;
- }
- };
- threadLocalMap.put(schema, schemaLocalMap);
- }
- return schemaLocalMap;
- }
-
private final RDN rdn;
private DN parent;
private final int size;
--
Gitblit v1.10.0