You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ds...@apache.org on 2015/08/06 21:08:14 UTC
incubator-geode git commit: GEODE-190: Fix
ReflectionBasedAutoSerializer leak
Repository: incubator-geode
Updated Branches:
refs/heads/feature/GEODE-190 [created] 6cb4a5682
GEODE-190: Fix ReflectionBasedAutoSerializer leak
A weak map value references its key keeping the key from ever being garbage.
The only reason the weak map was added was to allow internal code to get
the AutoSerializableManager given a ReflectionBasedAutoSerializer.
But the map was not really needed since the ReflectionBasedAutoSerializer
has a final field named "manager".
The only problem is that ReflectionBasedAutoSerializer is a public
api and AutoSerializableManager is internal.
This fix adds a getManager method on ReflectionBasedAutoSerializer that returns object
and is javadoc'd for "internal use only". The internal code casts the result to
AutoSerializableManager. This allows the weak map that was causing the memory leak
to be removed.
Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/6cb4a568
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/6cb4a568
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/6cb4a568
Branch: refs/heads/feature/GEODE-190
Commit: 6cb4a5682667eacc0017898b81367c3e5029c888
Parents: 7d4ae09
Author: Darrel Schneider <ds...@pivotal.io>
Authored: Mon Aug 3 14:04:23 2015 -0700
Committer: Darrel Schneider <ds...@pivotal.io>
Committed: Thu Aug 6 10:38:29 2015 -0700
----------------------------------------------------------------------
.../com/gemstone/gemfire/internal/cache/CacheConfig.java | 4 ++--
.../gemstone/gemfire/internal/cache/GemFireCacheImpl.java | 2 +-
.../gemstone/gemfire/pdx/ReflectionBasedAutoSerializer.java | 9 +++++++++
.../gemfire/pdx/internal/AutoSerializableManager.java | 8 --------
.../com/gemstone/gemfire/pdx/internal/TypeRegistry.java | 4 ++--
.../com/gemstone/gemfire/pdx/AutoSerializableJUnitTest.java | 2 +-
6 files changed, 15 insertions(+), 14 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/6cb4a568/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/CacheConfig.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/CacheConfig.java b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/CacheConfig.java
index 60b765c..7aaa241 100644
--- a/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/CacheConfig.java
+++ b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/CacheConfig.java
@@ -175,8 +175,8 @@ public class CacheConfig {
Object o2 = s2;
if (s1 instanceof ReflectionBasedAutoSerializer && s2 instanceof ReflectionBasedAutoSerializer) {
// Fix for bug 44907.
- o1 = AutoSerializableManager.getInstance((ReflectionBasedAutoSerializer) s1);
- o2 = AutoSerializableManager.getInstance((ReflectionBasedAutoSerializer) s2);
+ o1 = ((ReflectionBasedAutoSerializer) s1).getManager();
+ o2 = ((ReflectionBasedAutoSerializer) s2).getManager();
}
return equals(o1, o2);
}
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/6cb4a568/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java
index f5be144..79bcbc2 100644
--- a/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java
+++ b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java
@@ -5258,7 +5258,7 @@ public class GemFireCacheImpl implements InternalCache, ClientCache, HasCachePer
private void basicSetPdxSerializer(PdxSerializer v) {
TypeRegistry.setPdxSerializer(v);
if (v instanceof ReflectionBasedAutoSerializer) {
- AutoSerializableManager asm = AutoSerializableManager.getInstance((ReflectionBasedAutoSerializer) v);
+ AutoSerializableManager asm = (AutoSerializableManager) ((ReflectionBasedAutoSerializer) v).getManager();
if (asm != null) {
asm.setRegionService(this);
}
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/6cb4a568/gemfire-core/src/main/java/com/gemstone/gemfire/pdx/ReflectionBasedAutoSerializer.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/pdx/ReflectionBasedAutoSerializer.java b/gemfire-core/src/main/java/com/gemstone/gemfire/pdx/ReflectionBasedAutoSerializer.java
index 7065cea..ef89bcb 100644
--- a/gemfire-core/src/main/java/com/gemstone/gemfire/pdx/ReflectionBasedAutoSerializer.java
+++ b/gemfire-core/src/main/java/com/gemstone/gemfire/pdx/ReflectionBasedAutoSerializer.java
@@ -540,4 +540,13 @@ public class ReflectionBasedAutoSerializer implements PdxSerializer, Declarable
public final RegionService getRegionService() {
return this.manager.getRegionService();
}
+ /**
+ * For internal use only.
+ * @since 8.2
+ */
+ public final Object getManager() {
+ // The result is not AutoSerializableManager because
+ // that class is not part of our public APIs.
+ return this.manager;
+ }
}
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/6cb4a568/gemfire-core/src/main/java/com/gemstone/gemfire/pdx/internal/AutoSerializableManager.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/pdx/internal/AutoSerializableManager.java b/gemfire-core/src/main/java/com/gemstone/gemfire/pdx/internal/AutoSerializableManager.java
index e7a7ab7..5c560df 100644
--- a/gemfire-core/src/main/java/com/gemstone/gemfire/pdx/internal/AutoSerializableManager.java
+++ b/gemfire-core/src/main/java/com/gemstone/gemfire/pdx/internal/AutoSerializableManager.java
@@ -133,8 +133,6 @@ public class AutoSerializableManager {
new CopyOnWriteHashSet<String>();
- private static final Map<ReflectionBasedAutoSerializer, AutoSerializableManager> instances = new CopyOnWriteWeakHashMap<ReflectionBasedAutoSerializer, AutoSerializableManager>();
-
private final ReflectionBasedAutoSerializer owner;
public ReflectionBasedAutoSerializer getOwner() {
@@ -144,18 +142,12 @@ public class AutoSerializableManager {
public static AutoSerializableManager create(ReflectionBasedAutoSerializer owner, boolean checkPortability, String... patterns) {
AutoSerializableManager result = new AutoSerializableManager(owner);
result.reconfigure(checkPortability, patterns);
- instances.put(owner, result);
return result;
}
private AutoSerializableManager(ReflectionBasedAutoSerializer owner) {
this.owner = owner;
}
- public static AutoSerializableManager getInstance(ReflectionBasedAutoSerializer owner) {
- return instances.get(owner);
- }
-
-
public Map<Class<?>, AutoClassInfo> getClassMap() {
return classMap;
}
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/6cb4a568/gemfire-core/src/main/java/com/gemstone/gemfire/pdx/internal/TypeRegistry.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/pdx/internal/TypeRegistry.java b/gemfire-core/src/main/java/com/gemstone/gemfire/pdx/internal/TypeRegistry.java
index 4368e84..4ca1a90 100644
--- a/gemfire-core/src/main/java/com/gemstone/gemfire/pdx/internal/TypeRegistry.java
+++ b/gemfire-core/src/main/java/com/gemstone/gemfire/pdx/internal/TypeRegistry.java
@@ -321,13 +321,13 @@ public class TypeRegistry {
if (v == null) {
PdxSerializer oldValue = pdxSerializer.getAndSet(null);
if (oldValue instanceof ReflectionBasedAutoSerializer) {
- asm.compareAndSet(AutoSerializableManager.getInstance((ReflectionBasedAutoSerializer) oldValue), null);
+ asm.compareAndSet((AutoSerializableManager) ((ReflectionBasedAutoSerializer) oldValue).getManager(), null);
}
} else {
pdxSerializerWasSet = true;
pdxSerializer.set(v);
if (v instanceof ReflectionBasedAutoSerializer) {
- asm.set(AutoSerializableManager.getInstance((ReflectionBasedAutoSerializer) v));
+ asm.set((AutoSerializableManager) ((ReflectionBasedAutoSerializer) v).getManager());
}
}
}
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/6cb4a568/gemfire-core/src/test/java/com/gemstone/gemfire/pdx/AutoSerializableJUnitTest.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/pdx/AutoSerializableJUnitTest.java b/gemfire-core/src/test/java/com/gemstone/gemfire/pdx/AutoSerializableJUnitTest.java
index e3ab9ca..cb9d0f3 100644
--- a/gemfire-core/src/test/java/com/gemstone/gemfire/pdx/AutoSerializableJUnitTest.java
+++ b/gemfire-core/src/test/java/com/gemstone/gemfire/pdx/AutoSerializableJUnitTest.java
@@ -103,7 +103,7 @@ public class AutoSerializableJUnitTest {
private void setupSerializer(ReflectionBasedAutoSerializer s, boolean readSerialized) {
this.serializer = s;
- this.manager = AutoSerializableManager.getInstance(s);
+ this.manager = (AutoSerializableManager) s.getManager();
this.c = (GemFireCacheImpl) new CacheFactory().
set("mcast-port", "0").
setPdxReadSerialized(readSerialized).