Repository: avro Updated Branches: refs/heads/master 1de82871c -> 58daaf08a
AVRO-1821: Fix possible memory leak of Schemas in ReflectData. Contributed by Byran Harclerode. Project: http://git-wip-us.apache.org/repos/asf/avro/repo Commit: http://git-wip-us.apache.org/repos/asf/avro/commit/58daaf08 Tree: http://git-wip-us.apache.org/repos/asf/avro/tree/58daaf08 Diff: http://git-wip-us.apache.org/repos/asf/avro/diff/58daaf08 Branch: refs/heads/master Commit: 58daaf08a2637e0976cc124571200ce198b3143d Parents: 1de8287 Author: Ryan Blue <[email protected]> Authored: Sat Apr 16 14:10:45 2016 -0700 Committer: Ryan Blue <[email protected]> Committed: Sat Apr 16 14:13:29 2016 -0700 ---------------------------------------------------------------------- CHANGES.txt | 3 ++ lang/java/avro/pom.xml | 5 +++ .../org/apache/avro/reflect/ReflectData.java | 10 +++--- .../apache/avro/reflect/TestReflectData.java | 35 ++++++++++++++++++++ 4 files changed, 49 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/avro/blob/58daaf08/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index dd41810..f656de3 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -31,6 +31,9 @@ Trunk (not yet released) AVRO-1829. C++ documentation improvements (William S Fulton via thiru) + AVRO-1821: Java: Fix possible memory leak in ReflectData accessor cache. + (Bryan Harclerode via blue) + Avro 1.8.0 (22 January 2016) INCOMPATIBLE CHANGES http://git-wip-us.apache.org/repos/asf/avro/blob/58daaf08/lang/java/avro/pom.xml ---------------------------------------------------------------------- diff --git a/lang/java/avro/pom.xml b/lang/java/avro/pom.xml index f5da34f..ab798ee 100644 --- a/lang/java/avro/pom.xml +++ b/lang/java/avro/pom.xml @@ -198,6 +198,11 @@ <artifactId>joda-time</artifactId> <optional>true</optional> </dependency> + <dependency> + <groupId>org.hamcrest</groupId> + <artifactId>hamcrest-all</artifactId> + <scope>test</scope> + </dependency> </dependencies> </project> http://git-wip-us.apache.org/repos/asf/avro/blob/58daaf08/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java ---------------------------------------------------------------------- diff --git a/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java b/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java index 827e1fa..9e13e7c 100644 --- a/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java +++ b/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java @@ -38,6 +38,8 @@ import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import com.google.common.cache.CacheBuilder; +import com.google.common.collect.MapMaker; import org.apache.avro.AvroRemoteException; import org.apache.avro.AvroRuntimeException; import org.apache.avro.AvroTypeException; @@ -52,6 +54,7 @@ import org.apache.avro.generic.GenericFixed; import org.apache.avro.generic.IndexedRecord; import org.apache.avro.io.BinaryData; import org.apache.avro.util.ClassUtils; +import org.apache.avro.util.WeakIdentityHashMap; import org.apache.avro.io.DatumReader; import org.apache.avro.io.DatumWriter; import org.apache.avro.specific.FixedSize; @@ -230,13 +233,12 @@ public class ReflectData extends SpecificData { static final ConcurrentHashMap<Class<?>, ClassAccessorData> ACCESSOR_CACHE = new ConcurrentHashMap<Class<?>, ClassAccessorData>(); - private static class ClassAccessorData { + static class ClassAccessorData { private final Class<?> clazz; private final Map<String, FieldAccessor> byName = new HashMap<String, FieldAccessor>(); - private final IdentityHashMap<Schema, FieldAccessor[]> bySchema = - new IdentityHashMap<Schema, FieldAccessor[]>(); - + final Map<Schema, FieldAccessor[]> bySchema = new MapMaker().weakKeys().makeMap(); + private ClassAccessorData(Class<?> c) { clazz = c; for(Field f : getFields(c, false)) { http://git-wip-us.apache.org/repos/asf/avro/blob/58daaf08/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflectData.java ---------------------------------------------------------------------- diff --git a/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflectData.java b/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflectData.java new file mode 100644 index 0000000..46645c2 --- /dev/null +++ b/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflectData.java @@ -0,0 +1,35 @@ +package org.apache.avro.reflect; + +import org.apache.avro.Schema; +import org.junit.Test; +import java.io.IOException; +import java.lang.reflect.Field; +import java.util.Collections; +import java.util.Map; + +import static org.hamcrest.Matchers.lessThan; +import static org.junit.Assert.assertThat; + +public class TestReflectData { + @Test + @SuppressWarnings("unchecked") + public void testWeakSchemaCaching() throws Exception { + int numSchemas = 1000000; + for (int i = 0; i < numSchemas; i++) { + // Create schema + Schema schema = Schema.createRecord("schema", null, null, false); + schema.setFields(Collections.<Schema.Field>emptyList()); + + ReflectData.get().getRecordState(new Object(), schema); + } + + // Reflect the number of schemas currently in the cache + ReflectData.ClassAccessorData classData = ReflectData.ACCESSOR_CACHE + .get(Object.class); + + System.gc(); // Not guaranteed, but seems to be reliable enough + + assertThat("ReflectData cache should release references", + classData.bySchema.size(), lessThan(numSchemas)); + } +}
