Prevent creation of cyclic Generics stack when serializing specific generics object trees

src/com/esotericsoftware/kryo/Generics.java

@@ -19,14 +19,6 @@
 
 package com.esotericsoftware.kryo;
 
-import static com.esotericsoftware.minlog.Log.TRACE;
-import static com.esotericsoftware.minlog.Log.trace;
-
-import java.lang.reflect.Array;
-import java.lang.reflect.GenericArrayType;
-import java.lang.reflect.ParameterizedType;
-import java.lang.reflect.Type;
-import java.lang.reflect.TypeVariable;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -36,51 +28,28 @@
 public class Generics {
 	private Map<String, Class> typeVar2class;
 
-	private Generics parentScope;
-
 	public Generics () {
 		typeVar2class = new HashMap<String, Class>();
-		parentScope = null;
 	}
 
 	public Generics (Map<String, Class> mappings) {
 		typeVar2class = new HashMap<String, Class>(mappings);
-		parentScope = null;
-	}
-
-	public Generics (Generics parentScope) {
-		typeVar2class = new HashMap<String, Class>();
-		this.parentScope = parentScope;
 	}
 
 	public void add (String typeVar, Class clazz) {
 		typeVar2class.put(typeVar, clazz);
 	}
 
 	public Class getConcreteClass (String typeVar) {
-		Class clazz = typeVar2class.get(typeVar);
-		if (clazz == null && parentScope != null) return parentScope.getConcreteClass(typeVar);
-		return clazz;
+		return typeVar2class.get(typeVar);
 	}
 
-	public void setParentScope (Generics scope) {
-		if (parentScope != null) throw new IllegalStateException("Parent scope can be set just once");
-		parentScope = scope;
-	}
-
-	public Generics getParentScope () {
-		return parentScope;
-	}
-
-	public Map<String, Class> getMappings() {
+	public Map<String, Class> getMappings () {
 		return typeVar2class;
 	}
 
 	public String toString () {
 		return typeVar2class.toString();
 	}
 
-	public void resetParentScope () {
-		parentScope = null;
-	}
 }

src/com/esotericsoftware/kryo/GenericsResolver.java

@@ -0,0 +1,52 @@
+/* Copyright (c) 2008, Nathan Sweet
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer.
+ * - Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials provided with the distribution.
+ * - Neither the name of Esoteric Software nor the names of its contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING,
+ * BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT
+ * SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */
+
+package com.esotericsoftware.kryo;
+
+import java.util.LinkedList;
+
+/** Helper class that resolves a type name variable to a concrete class using the current class serialization stack
+ *
+ * @author Jeroen van Erp <jeroen@hierynomus.com> */
+public class GenericsResolver {
+    private LinkedList<Generics> stack = new LinkedList<Generics>();
+
+    GenericsResolver () {
+    }
+
+    public Class getConcreteClass (String typeVar) {
+        for (Generics generics : stack) {
+            Class clazz = generics.getConcreteClass(typeVar);
+            if (clazz != null) return clazz;
+        }
+        return null;
+    }
+
+    public boolean isSet () {
+        return !stack.isEmpty();
+    }
+
+    void pushScope (Generics scope) {
+        stack.addFirst(scope);
+    }
+
+    void popScope () {
+        stack.removeFirst();
+    }
+}

src/com/esotericsoftware/kryo/Kryo.java

@@ -138,7 +138,7 @@ public class Kryo {
 	private boolean copyShallow;
 	private IdentityMap originalToCopy;
 	private Object needsCopyReference;
-	private Generics genericsScope;
+	private GenericsResolver genericsResolver = new GenericsResolver();
 	/** Tells if ASM-based backend should be used by new serializer instances created using this Kryo instance. */
 	private boolean asmEnabled = false;
 
@@ -1161,22 +1161,15 @@ static final class DefaultSerializerEntry {
 
 	public void pushGenericsScope (Class type, Generics generics) {
 		if (TRACE) trace("kryo", "Settting a new generics scope for class " + type.getName() + ": " + generics);
-		Generics currentScope = genericsScope;
-		if (generics.getParentScope() != null) {
-			generics = new Generics(generics.getMappings());
-		}
-		genericsScope = generics;
-		genericsScope.setParentScope(currentScope);
+		genericsResolver.pushScope(generics);
 	}
 
 	public void popGenericsScope () {
-		Generics oldScope = genericsScope;
-		if (genericsScope != null) genericsScope = genericsScope.getParentScope();
-		if (oldScope != null) oldScope.resetParentScope();
+		genericsResolver.popScope();
 	}
 
-	public Generics getGenericsScope () {
-		return genericsScope;
+	public GenericsResolver getGenericsResolver () {
+		return genericsResolver;
 	}
 
 	public StreamFactory getStreamFactory () {

src/com/esotericsoftware/kryo/serializers/FieldSerializer.java

@@ -557,7 +557,7 @@ public T read (Kryo kryo, Input input, Class<T> type) {
 			}
 			return object;
 		} finally {
-			if (genericsScope != null && kryo.getGenericsScope() != null) {
+			if (genericsScope != null && kryo.getGenericsResolver() != null) {
 				// Pop the scope for generics
 				kryo.popGenericsScope();
 			}

src/com/esotericsoftware/kryo/serializers/FieldSerializerGenericsUtil.java

@@ -34,6 +34,7 @@
 import java.util.Map;
 
 import com.esotericsoftware.kryo.Generics;
+import com.esotericsoftware.kryo.GenericsResolver;
 import com.esotericsoftware.kryo.Kryo;
 import com.esotericsoftware.kryo.serializers.FieldSerializer.CachedField;
 
@@ -126,8 +127,8 @@ private Class<?> getTypeVarConcreteClass (Class[] generics, int typeVarNum, Stri
 		} else {
 			// Otherwise try to derive the information from the current GenericScope
 			if (TRACE) trace("kryo", "Trying to use kryo.getGenericScope");
-			Generics scope = kryo.getGenericsScope();
-			if (scope != null) {
+			GenericsResolver scope = kryo.getGenericsResolver();
+			if (scope.isSet()) {
 				return scope.getConcreteClass(typeVarName);
 			}
 		}
@@ -277,8 +278,8 @@ public static Class[] getGenerics (Type genericType, Kryo kryo) {
 			else if (actualType instanceof ParameterizedType)
 				generics[i] = (Class)((ParameterizedType)actualType).getRawType();
 			else if (actualType instanceof TypeVariable) {
-				Generics scope = kryo.getGenericsScope();
-				if (scope != null) {
+				GenericsResolver scope = kryo.getGenericsResolver();
+				if (scope.isSet()) {
 					Class clazz = scope.getConcreteClass(((TypeVariable)actualType).getName());
 					if (clazz != null) {
 						generics[i] = clazz;
@@ -291,8 +292,8 @@ else if (actualType instanceof TypeVariable) {
 				if (componentType instanceof Class)
 					generics[i] = Array.newInstance((Class)componentType, 0).getClass();
 				else if (componentType instanceof TypeVariable) {
-					Generics scope = kryo.getGenericsScope();
-					if (scope != null) {
+					GenericsResolver scope = kryo.getGenericsResolver();
+					if (scope.isSet()) {
 						Class clazz = scope.getConcreteClass(((TypeVariable)componentType).getName());
 						if (clazz != null) {
 							generics[i] = Array.newInstance(clazz, 0).getClass();

test/com/esotericsoftware/kryo/FieldSerializerGenericsTest.java

@@ -0,0 +1,93 @@
+package com.esotericsoftware.kryo;
+
+import com.esotericsoftware.kryo.io.FastOutput;
+import com.esotericsoftware.minlog.Log;
+import org.junit.Test;
+
+import java.io.ByteArrayOutputStream;
+import java.util.*;
+
+public class FieldSerializerGenericsTest {
+
+    @Test
+    public void testNoStackOverflowForSimpleGenericsCase () {
+        FooRef fooRef = new FooRef();
+        GenericFoo<FooRef> genFoo1 = new GenericFoo<FooRef>(fooRef);
+        GenericFoo<FooRef> genFoo2 = new GenericFoo<FooRef>(fooRef);
+        List<GenericFoo<?>> foos = new ArrayList<GenericFoo<?>>();
+        foos.add(genFoo2);
+        foos.add(genFoo1);
+        new FooContainer(foos);
+        Kryo kryo = new Kryo();
+        ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
+
+        kryo.writeObject(new FastOutput(outputStream), genFoo1);
+    }
+
+    @Test
+    public void testNoStackOverflowForComplexGenericsCase () {
+        BarRef barRef = new BarRef();
+        GenericBar<BarRef> genBar1 = new GenericBar<BarRef>(barRef);
+        GenericBar<BarRef> genBar2 = new GenericBar<BarRef>(barRef);
+        List<GenericBar<?>> bars = new ArrayList<GenericBar<?>>();
+        bars.add(genBar2);
+        bars.add(genBar1);
+        new GenericBarContainer<GenericBar>(new BarContainer(bars));
+        Kryo kryo = new Kryo();
+        ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
+
+        kryo.writeObject(new FastOutput(outputStream), genBar1);
+    }
+
+    static class GenericBarContainer<T extends Bar> {
+        BarContainer barContainer;
+
+        public GenericBarContainer(BarContainer barContainer) {
+            this.barContainer = barContainer;
+            for (GenericBar<?> foo : barContainer.foos) {
+                foo.container = this;
+            }
+        }
+    }
+    static class BarContainer {
+        List<GenericBar<?>> foos;
+
+        public BarContainer (List<GenericBar<?>> foos) {
+            this.foos = foos;
+        }
+    }
+    interface Bar {}
+    static class BarRef implements Bar {}
+    static class GenericBar<B extends Bar> implements Bar {
+        private Map<String, Object> map = Collections.singletonMap("myself", (Object) this);
+        B foo;
+        GenericBarContainer<?> container;
+
+        public GenericBar (B foo) {
+            this.foo = foo;
+        }
+    }
+
+    interface Foo {}
+    static class FooRef implements Foo {}
+    static class FooContainer {
+        List<GenericFoo<?>> foos = new ArrayList<GenericFoo<?>>();
+
+        public FooContainer (List<GenericFoo<?>> foos) {
+            this.foos = foos;
+            for (GenericFoo<?> foo : foos) {
+                foo.container = this;
+            }
+        }
+    }
+    static class GenericFoo<B extends Foo> implements Foo {
+        private Map<String, Object> map = Collections.singletonMap("myself", (Object) this);
+        B foo;
+        FooContainer container;
+
+        public GenericFoo (B foo) {
+            this.foo = foo;
+        }
+    }
+}
+