Improve tests for JDK 17 (#886)

* Improve tests for JDK 17 * Fall back on resolving `SerializedLambda` via capturing class if `SerializedLambda#readResolve` is not accessible * Fix `--ad-opens` * Do not compare static and synthetic fields * Remove broken custom `equals` from test data * Use custom `equals` contract if available * Remove now unncessary `--add-opens` arguments
EsotericSoftware · Nov 15, 2022 · 7d90a4c · 7d90a4c
1 parent 5f561b3
commit 7d90a4c
Show file tree

Hide file tree

Showing 19 changed files with 196 additions and 175 deletions.
diff --git a/README.md b/README.md
@@ -617,12 +617,11 @@ Kryo `isFinal` is used to determine if a class is final. This method can be over
 
 Kryo can serialize Java 8+ closures that implement java.io.Serializable, with some caveats. Closures serialized on one JVM may fail to be deserialized on a different JVM.
 
-Kryo `isClosure` is used to determine if a class is a closure. If so, then ClosureSerializer.Closure is used to find the class registration instead of the closure's class. To serialize closures, the following classes must be registered: ClosureSerializer.Closure, SerializedLambda, Object[], and Class. Additionally, the closure's capturing class must be registered.
+Kryo `isClosure` is used to determine if a class is a closure. If so, then ClosureSerializer.Closure is used to find the class registration instead of the closure's class. To serialize closures, the following classes must be registered: ClosureSerializer.Closure, Object[], and Class. Additionally, the closure's capturing class must be registered.
 
 ```java
 kryo.register(Object[].class);
 kryo.register(Class.class);
-kryo.register(SerializedLambda.class);
 kryo.register(ClosureSerializer.Closure.class, new ClosureSerializer());
 kryo.register(CapturingClass.class);
 

diff --git a/pom.xml b/pom.xml
@@ -436,15 +436,9 @@
 							<configuration>
 									<argLine>
 										--enable-preview
-										--add-opens java.base/java.lang=ALL-UNNAMED
-										--add-opens java.base/java.lang.invoke=ALL-UNNAMED
-										--add-opens=java.base/java.net=ALL-UNNAMED
-										--add-opens=java.base/java.nio=ALL-UNNAMED
-										--add-opens=java.base/java.time=ALL-UNNAMED
-										--add-opens=java.base/java.util=ALL-UNNAMED
+										--add-opens java.base/sun.nio.ch=ALL-UNNAMED
+										--add-opens java.base/java.nio=ALL-UNNAMED
 										--add-opens java.base/java.util.concurrent.atomic=ALL-UNNAMED
-										--add-opens=java.base/sun.nio.ch=ALL-UNNAMED
-										--add-opens=java.base/sun.util.calendar=ALL-UNNAMED
 									</argLine>
 							</configuration>
 						</plugin>

diff --git a/src/com/esotericsoftware/kryo/serializers/ClosureSerializer.java b/src/com/esotericsoftware/kryo/serializers/ClosureSerializer.java
@@ -27,6 +27,7 @@
 import com.esotericsoftware.kryo.Serializer;
 import com.esotericsoftware.kryo.io.Input;
 import com.esotericsoftware.kryo.io.Output;
+import com.esotericsoftware.minlog.Log;
 
 import java.io.Serializable;
 import java.lang.invoke.SerializedLambda;
@@ -36,7 +37,6 @@
  * <p>
  * <code>kryo.register(Object[].class);
  * kryo.register(Class.class);
- * kryo.register(java.lang.invoke.SerializedLambda.class);<br>
  * kryo.register(ClosureSerializer.Closure.class, new ClosureSerializer());</code>
  * <p>
  * Also, the closure's capturing class must be registered.
@@ -56,7 +56,9 @@ public ClosureSerializer () {
 				readResolve = SerializedLambda.class.getDeclaredMethod("readResolve");
 				readResolve.setAccessible(true);
 			} catch (Exception ex) {
-				throw new KryoException("Unable to obtain SerializedLambda#readResolve via reflection.", ex);
+				readResolve = null;
+				Log.warn("Unable to obtain SerializedLambda#readResolve via reflection. " +
+					"Falling back on resolving lambdas via capturing class.", ex);
 			}
 		}
 	}
@@ -87,24 +89,38 @@ public Object read (Kryo kryo, Input input, Class type) {
 		Object[] capturedArgs = new Object[count];
 		for (int i = 0; i < count; i++)
 			capturedArgs[i] = kryo.readClassAndObject(input);
-		SerializedLambda serializedLambda = new SerializedLambda(kryo.readClass(input).getType(), input.readString(),
+		Class<?> capturingClass = kryo.readClass(input).getType();
+		SerializedLambda serializedLambda = new SerializedLambda(capturingClass, input.readString(),
 			input.readString(), input.readString(), input.readVarInt(true), input.readString(), input.readString(),
 			input.readString(), input.readString(), capturedArgs);
 		try {
-			return readResolve.invoke(serializedLambda);
+			return readResolve(capturingClass, serializedLambda);
 		} catch (Exception ex) {
 			throw new KryoException("Error reading closure.", ex);
 		}
 	}
 
 	public Object copy (Kryo kryo, Object original) {
 		try {
-			return readResolve.invoke(toSerializedLambda(original));
+			SerializedLambda lambda = toSerializedLambda(original);
+			Class<?> capturingClass = Class.forName(lambda.getCapturingClass().replace('/', '.'));
+			return readResolve(capturingClass, lambda);
 		} catch (Exception ex) {
 			throw new KryoException("Error copying closure.", ex);
 		}
 	}
 
+	private Object readResolve (Class<?> capturingClass, SerializedLambda lambda) throws Exception {
+		if (readResolve != null) {
+			return readResolve.invoke(lambda);
+		}
+
+		// See SerializedLambda#readResolve
+		Method m = capturingClass.getDeclaredMethod("$deserializeLambda$", SerializedLambda.class);
+		m.setAccessible(true);
+		return m.invoke(null, lambda);
+	}
+
 	private SerializedLambda toSerializedLambda (Object object) {
 		Object replacement;
 		try {
@@ -118,9 +134,7 @@ private SerializedLambda toSerializedLambda (Object object) {
 		try {
 			return (SerializedLambda)replacement;
 		} catch (Exception ex) {
-			throw new KryoException(
-				"writeReplace must return a SerializedLambda: " + (replacement == null ? null : className(replacement.getClass())),
-				ex);
+			throw new KryoException("writeReplace must return a SerializedLambda: " + className(replacement.getClass()), ex);
 		}
 	}
 }
diff --git a/src/com/esotericsoftware/kryo/util/Util.java b/src/com/esotericsoftware/kryo/util/Util.java
@@ -68,6 +68,10 @@ public class Util {
 		primitiveWrappers.put(short.class, Short.class);
 	}
 
+	public static boolean isUnsafeAvailable () {
+		return unsafe;
+	}
+
 	public static boolean isClassAvailable (String className) {
 		try {
 			Class.forName(className);

diff --git a/test-jdk11/com/esotericsoftware/kryo/serializers/ImmutableCollectionsSerializersTest.java b/test-jdk11/com/esotericsoftware/kryo/serializers/ImmutableCollectionsSerializersTest.java
@@ -82,7 +82,7 @@ void testImmutableSet () {
 		roundTrip(6, Set.of(1, 2, 3));
 	}
 
-	static class TestClass {
+	public static class TestClass {
 		List<Integer> list;
 		Map<Integer, Integer> map;
 		Set<Integer> set;

diff --git a/test-jdk14/com/esotericsoftware/kryo/TestDataJava14.java b/test-jdk14/com/esotericsoftware/kryo/TestDataJava14.java
@@ -19,14 +19,24 @@
 
 package com.esotericsoftware.kryo;
 
-import com.esotericsoftware.kryo.SerializationCompatTestData;
+import org.apache.commons.lang.builder.EqualsBuilder;
+import org.apache.commons.lang.builder.HashCodeBuilder;
 
 /** Test data for {@link com.esotericsoftware.kryo.serializers.RecordSerializerTest}.
  * @author Julia Boes <julia.boes@oracle.com>
  * @author Chris Hegarty <chris.hegarty@oracle.com>
  */
 public class TestDataJava14 extends SerializationCompatTestData.TestData {
-    public record Rec (byte b, short s, int i, long l, float f, double d, boolean bool, char c, String str, Integer[] n) { };
+    public record Rec (byte b, short s, int i, long l, float f, double d, boolean bool, char c, String str, Integer[] n) {
+        // Overriden because of https://stackoverflow.com/questions/61261226/java-14-records-and-arrays
+        public boolean equals(Object o) {
+            return EqualsBuilder.reflectionEquals(this, o);
+        }
+
+        public int hashCode() {
+            return HashCodeBuilder.reflectionHashCode(this);
+        }
+    };
 
     private Rec rec;
 

diff --git a/test/com/esotericsoftware/kryo/KryoTestCase.java b/test/com/esotericsoftware/kryo/KryoTestCase.java
@@ -30,6 +30,7 @@
 import com.esotericsoftware.kryo.unsafe.UnsafeByteBufferOutput;
 import com.esotericsoftware.kryo.unsafe.UnsafeInput;
 import com.esotericsoftware.kryo.unsafe.UnsafeOutput;
+import com.esotericsoftware.kryo.util.Util;
 
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
@@ -139,50 +140,54 @@ public Input createInput (byte[] buffer) {
 			}
 		});
 
-		roundTripWithBufferFactory(length, object1, new BufferFactory() {
-			public Output createOutput (OutputStream os) {
-				return new UnsafeOutput(os);
-			}
-
-			public Output createOutput (OutputStream os, int size) {
-				return new UnsafeOutput(os, size);
-			}
+		if (Util.isUnsafeAvailable()) {
+			roundTripWithBufferFactory(length, object1, new BufferFactory() {
+				public Output createOutput(OutputStream os) {
+					return new UnsafeOutput(os);
+				}
 
-			public Output createOutput (int size, int limit) {
-				return new UnsafeOutput(size, limit);
-			}
+				public Output createOutput(OutputStream os, int size) {
+					return new UnsafeOutput(os, size);
+				}
 
-			public Input createInput (InputStream os, int size) {
-				return new UnsafeInput(os, size);
-			}
+				public Output createOutput(int size, int limit) {
+					return new UnsafeOutput(size, limit);
+				}
 
-			public Input createInput (byte[] buffer) {
-				return new UnsafeInput(buffer);
-			}
-		});
+				public Input createInput(InputStream os, int size) {
+					return new UnsafeInput(os, size);
+				}
 
-		roundTripWithBufferFactory(length, object1, new BufferFactory() {
-			public Output createOutput (OutputStream os) {
-				return new UnsafeByteBufferOutput(os);
-			}
-
-			public Output createOutput (OutputStream os, int size) {
-				return new UnsafeByteBufferOutput(os, size);
-			}
-
-			public Output createOutput (int size, int limit) {
-				return new UnsafeByteBufferOutput(size, limit);
-			}
-
-			public Input createInput (InputStream os, int size) {
-				return new UnsafeByteBufferInput(os, size);
-			}
+				public Input createInput(byte[] buffer) {
+					return new UnsafeInput(buffer);
+				}
+			});
+		}
 
-			public Input createInput (byte[] buffer) {
-				ByteBuffer byteBuffer = allocateByteBuffer(buffer);
-				return new UnsafeByteBufferInput(byteBuffer.asReadOnlyBuffer());
-			}
-		});
+		if (Util.isUnsafeAvailable()) {
+			roundTripWithBufferFactory(length, object1, new BufferFactory() {
+				public Output createOutput(OutputStream os) {
+					return new UnsafeByteBufferOutput(os);
+				}
+
+				public Output createOutput(OutputStream os, int size) {
+					return new UnsafeByteBufferOutput(os, size);
+				}
+
+				public Output createOutput(int size, int limit) {
+					return new UnsafeByteBufferOutput(size, limit);
+				}
+
+				public Input createInput(InputStream os, int size) {
+					return new UnsafeByteBufferInput(os, size);
+				}
+
+				public Input createInput(byte[] buffer) {
+					ByteBuffer byteBuffer = allocateByteBuffer(buffer);
+					return new UnsafeByteBufferInput(byteBuffer.asReadOnlyBuffer());
+				}
+			});
+		}
 
 		return object2;
 	}

diff --git a/test/com/esotericsoftware/kryo/ReferenceTest.java b/test/com/esotericsoftware/kryo/ReferenceTest.java
@@ -25,9 +25,9 @@
 import com.esotericsoftware.kryo.io.Output;
 import com.esotericsoftware.kryo.serializers.MapSerializer;
 
-import java.lang.reflect.Field;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Objects;
 import java.util.TreeMap;
 
 import org.junit.jupiter.api.Test;
@@ -89,19 +89,13 @@ void testReadingNestedObjectsFirst () {
 		list.add("2");
 		list.add("1");
 		list.add("1");
-		List subList = list.subList(0, 5);
+		ArrayListHolder subList = new ArrayListHolder(list);
 
 		kryo.setRegistrationRequired(false);
 		kryo.register(ArrayList.class);
-		Class<List> subListClass = (Class<List>)subList.getClass();
-		if (subListClass.getName().equals("java.util.ArrayList$SubList")) {
-			// This is JDK > = 1.7
-			kryo.register(subList.getClass(), new ArraySubListSerializer());
-		} else {
-			kryo.register(subList.getClass(), new SubListSerializer());
+		kryo.register(ArrayListHolder.class);
 
-		}
-		roundTrip(23, subList);
+		roundTrip(15, subList);
 	}
 
 	@Test
@@ -118,86 +112,25 @@ void testReadInvalidReference() {
 		fail("Exception was expected");
 	}
 
-	public static class SubListSerializer extends Serializer<List> {
-		private Field listField, offsetField, sizeField;
-
-		public SubListSerializer () {
-			try {
-				Class sublistClass = Class.forName("java.util.SubList");
-				listField = sublistClass.getDeclaredField("l");
-				offsetField = sublistClass.getDeclaredField("offset");
-				sizeField = sublistClass.getDeclaredField("size");
-				listField.setAccessible(true);
-				offsetField.setAccessible(true);
-				sizeField.setAccessible(true);
-			} catch (Exception ex) {
-				throw new RuntimeException(ex);
-			}
-		}
-
-		public void write (Kryo kryo, Output output, List list) {
-			try {
-				kryo.writeClassAndObject(output, listField.get(list));
-				int fromIndex = offsetField.getInt(list);
-				int count = sizeField.getInt(list);
-				output.writeInt(fromIndex);
-				output.writeInt(count);
-			} catch (Exception ex) {
-				throw new RuntimeException(ex);
-			}
-		}
-
-		public List read (Kryo kryo, Input input, Class<? extends List> type) {
-			List list = (List)kryo.readClassAndObject(input);
-			int fromIndex = input.readInt();
-			int count = input.readInt();
-			return list.subList(fromIndex, fromIndex + count);
-		}
-	}
+	public static class ArrayListHolder {
+		private List<Object> parent;
 
-	public static class ArraySubListSerializer extends Serializer<List> {
-		private Field parentField, offsetField, sizeField;
-
-		public ArraySubListSerializer () {
-			try {
-				Class sublistClass = Class.forName("java.util.ArrayList$SubList");
-				parentField = getParentField(sublistClass);
-				offsetField = sublistClass.getDeclaredField("offset");
-				sizeField = sublistClass.getDeclaredField("size");
-				parentField.setAccessible(true);
-				offsetField.setAccessible(true);
-				sizeField.setAccessible(true);
-			} catch (Exception ex) {
-				throw new RuntimeException(ex);
-			}
+		public ArrayListHolder () {
 		}
 
-		private Field getParentField(Class sublistClass) throws NoSuchFieldException {
-			try {
-				// java 9+
-				return sublistClass.getDeclaredField("root");
-			} catch(NoSuchFieldException e) {
-				return sublistClass.getDeclaredField("parent");
-			}
+		public ArrayListHolder (List<Object> parent) {
+			this.parent = parent;
 		}
 
-		public void write (Kryo kryo, Output output, List list) {
-			try {
-				kryo.writeClassAndObject(output, parentField.get(list));
-				int offset = offsetField.getInt(list);
-				int size = sizeField.getInt(list);
-				output.writeInt(offset);
-				output.writeInt(size);
-			} catch (Exception ex) {
-				throw new RuntimeException(ex);
-			}
+		public boolean equals (Object o) {
+			if (this == o) return true;
+			if (o == null || getClass() != o.getClass()) return false;
+			ArrayListHolder that = (ArrayListHolder)o;
+			return Objects.equals(parent, that.parent);
 		}
 
-		public List read (Kryo kryo, Input input, Class<? extends List> type) {
-			List list = (List)kryo.readClassAndObject(input);
-			int offset = input.readInt();
-			int size = input.readInt();
-			return list.subList(offset, offset + size);
+		public int hashCode () {
+			return Objects.hash(parent);
 		}
 	}
 }