geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kl...@apache.org
Subject [13/21] incubator-geode git commit: GEODE-190: Fix ReflectionBasedAutoSerializer leak
Date Thu, 24 Sep 2015 22:44:50 GMT
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/6da9b849
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/6da9b849
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/6da9b849

Branch: refs/heads/feature/GEODE-189
Commit: 6da9b849656e171af6ea59bfa9963a068e61e877
Parents: e0cd532
Author: Darrel Schneider <dschneider@pivotal.io>
Authored: Mon Aug 3 14:04:23 2015 -0700
Committer: Kirk Lund <klund@pivotal.io>
Committed: Thu Aug 13 11:39:45 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/6da9b849/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/6da9b849/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/6da9b849/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/6da9b849/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/6da9b849/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/6da9b849/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