geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dschnei...@apache.org
Subject incubator-geode git commit: GEODE-190: Fix ReflectionBasedAutoSerializer leak
Date Mon, 10 Aug 2015 21:02:44 GMT
Repository: incubator-geode
Updated Branches:
  refs/heads/develop b1e7466cb -> 1c7cbb2d1


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/1c7cbb2d
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/1c7cbb2d
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/1c7cbb2d

Branch: refs/heads/develop
Commit: 1c7cbb2d1c56f66f76370ebd82a44a90d3c4e7e1
Parents: b1e7466
Author: Darrel Schneider <dschneider@pivotal.io>
Authored: Mon Aug 3 14:04:23 2015 -0700
Committer: Darrel Schneider <dschneider@pivotal.io>
Committed: Mon Aug 10 14:01:13 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/1c7cbb2d/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/1c7cbb2d/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/1c7cbb2d/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/1c7cbb2d/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/1c7cbb2d/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/1c7cbb2d/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).


Mime
View raw message