Bryan Harclerode created AVRO-1821:
--------------------------------------
Summary: Avro (Java) Memory Leak in ReflectData Caching
Key: AVRO-1821
URL: https://issues.apache.org/jira/browse/AVRO-1821
Project: Avro
Issue Type: Bug
Components: java
Environment: OS X 10.11.3
{code}java version "1.8.0_66"
Java(TM) SE Runtime Environment (build 1.8.0_66-b17)
Java HotSpot(TM) 64-Bit Server VM (build 25.66-b17, mixed mode){code}
Reporter: Bryan Harclerode
I think I have encountered one of the memory leaks described by AVRO-1283 in
the way Java Avro implements field accessor caching in {{ReflectData}}. When a
reflected object is serialized, the key of {{ClassAccessorData.bySchema}} (as
retained by {{ReflectData.ACCESSOR_CACHE}}) retains a strong reference to the
schema that was used to serialize the object, but there exists no code path for
clearing these references after a schema will no longer be used.
While in most cases, a class will probably only have one schema associated with
it (created and cached by {{ReflectData.getSchema(Type)}}), I experienced
{{OutOfMemoryError}} when serializing generic classes with
dynamically-generated schemas. The following is a minimal example which will
exhaust a 50MiB heap ({{-Xmx50m}}) after about 190K iterations:
{code:title=AvroMemoryLeakMinimal.java|borderStyle=solid}
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.util.Collections;
import org.apache.avro.Schema;
import org.apache.avro.io.BinaryEncoder;
import org.apache.avro.io.EncoderFactory;
import org.apache.avro.reflect.ReflectDatumWriter;
public class AvroMemoryLeakMinimal {
public static void main(String[] args) throws IOException {
long count = 0;
EncoderFactory encFactory = EncoderFactory.get();
try {
while (true) {
// Create schema
Schema schema = Schema.createRecord("schema", null, null,
false);
schema.setFields(Collections.<Schema.Field>emptyList());
// serialize
ByteArrayOutputStream baos = new ByteArrayOutputStream(1024);
BinaryEncoder encoder = encFactory.binaryEncoder(baos, null);
(new ReflectDatumWriter<Object>(schema)).write(new Object(),
encoder);
byte[] result = baos.toByteArray();
count++;
}
} catch (OutOfMemoryError e) {
System.out.print("Memory exhausted after ");
System.out.print(count);
System.out.println(" schemas");
throw e;
}
}
}
{code}
I was able to fix the bug in the latest 1.9.0-SNAPSHOT from git with the
following patch to {{ClassAccessorData.bySchema}} to use weak keys so that it
properly released the {{Schema}} objects if no other threads are still
referencing them:
{code:title=ReflectData.java.patch|borderStyle=solid}
--- 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
@@ -57,6 +57,7 @@ import org.apache.avro.io.DatumWriter;
import org.apache.avro.specific.FixedSize;
import org.apache.avro.specific.SpecificData;
import org.apache.avro.SchemaNormalization;
+import org.apache.avro.util.WeakIdentityHashMap;
import org.codehaus.jackson.JsonNode;
import org.codehaus.jackson.node.NullNode;
@@ -234,8 +235,8 @@ public class ReflectData extends SpecificData {
private final Class<?> clazz;
private final Map<String, FieldAccessor> byName =
new HashMap<String, FieldAccessor>();
- private final IdentityHashMap<Schema, FieldAccessor[]> bySchema =
- new IdentityHashMap<Schema, FieldAccessor[]>();
+ private final WeakIdentityHashMap<Schema, FieldAccessor[]> bySchema =
+ new WeakIdentityHashMap<Schema, FieldAccessor[]>();
private ClassAccessorData(Class<?> c) {
clazz = c;
{code}
Additionally, I'm not sure why an {{IdentityHashMap}} was used instead of a
standard {{HashMap}}, since two equivalent schemas have the same set of
{{FieldAccessor}}. Everything appears to work and all tests pass if I use a
{{WeakHashMap}} instead of an {{WeakIdentityHashMap}}, but I don't know if
there was some other reason object identity was important for this map. If a
non-identity map can be used, this will help reduce memory/CPU usage further by
not regenerating all the field accessors for equivalent schemas.
The following unit test appears to reliably catch this bug, but is
non-deterministic due to the nature of garbage collection (and I'm not sure
there's a way around that):
{code:title=TestReflectData.java|borderStyle=solid}
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 if ReflectData is leaking {@link Schema} references
*/
@SuppressWarnings("unchecked")
@Test public void testWeakSchemaCaching() throws IOException,
NoSuchFieldException, IllegalAccessException {
for (int i = 0; i < 1000; 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
Field cacheField = ReflectData.class.getDeclaredField("ACCESSOR_CACHE");
cacheField.setAccessible(true);
Map<Class<?>, ?> ACCESSOR_CACHE = (Map) cacheField.get(null);
Object classData = ACCESSOR_CACHE.get(Object.class);
Field bySchemaField = classData.getClass().getDeclaredField("bySchema");
bySchemaField.setAccessible(true);
Map<Schema, FieldAccessor[]> accessors = (Map)
bySchemaField.get(classData);
System.gc(); // Not guaranteed reliable, but seems to be reliable
enough for our purposes
// See if the number of schemas in the cache is less than the number we
generated - if so, then they are being released.
assertThat("ReflectData cache should release references",
accessors.size(), lessThan(1000));
}
}
{code}
(Added {{org.hamcrest:hamcrest-all}} dependency to test scope for the built-in
{{lessThan()}} matcher)
----
The current workaround that I'm using to mitigate the leak is to cache schemas
and re-use older instances when I'm about to serialize an equivalent schema.
Since most of the generated schemas are equivalent, this limits the number of
leaked schemas to a handful. A more permanent workaround would be to switch to
using a {{GenericRecord}} instead of a generic java class for the object that
is being serialized, since this cuts out the use of {{ReflectData}} entirely.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
