Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash in generics code when object graph gets wild enough #1140

Open
mikehearn opened this issue Oct 25, 2024 · 1 comment
Open

Crash in generics code when object graph gets wild enough #1140

mikehearn opened this issue Oct 25, 2024 · 1 comment

Comments

@mikehearn
Copy link

Unfortunately I'm not entirely sure how to make a minimal reproducer for this, so figured I'd drop a bug in case anyone who knows the code spots the issue.

When serializing a sufficiently crazy object graph (Micronaut compile time reflection metadata), some kind of limit gets hit in the optimized generics code and there's an ArrayIndexOutOfBoundsException. I tried to understand the code that's failing but it's a bit arcane:

Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 16 out of bounds for length 16
	at com.esotericsoftware.kryo.util.DefaultGenerics.pushTypeVariables(DefaultGenerics.java:127)
	at com.esotericsoftware.kryo.serializers.FieldSerializer.pushTypeVariables(FieldSerializer.java:148)
	at com.esotericsoftware.kryo.serializers.VersionFieldSerializer.write(VersionFieldSerializer.java:97)
	at com.esotericsoftware.kryo.Kryo.writeObject(Kryo.java:642)
	at io.micronaut.tasks.impl.kryo.GraphingKryo.writeObject(GraphingKryo.java:314)
	at com.esotericsoftware.kryo.serializers.ReflectField.write(ReflectField.java:71)
	at com.esotericsoftware.kryo.serializers.VersionFieldSerializer.write(VersionFieldSerializer.java:105)
	at com.esotericsoftware.kryo.Kryo.writeObject(Kryo.java:642)
	at io.micronaut.tasks.impl.kryo.GraphingKryo.writeObject(GraphingKryo.java:314)
	at com.esotericsoftware.kryo.serializers.ReflectField.write(ReflectField.java:71)
	at com.esotericsoftware.kryo.serializers.VersionFieldSerializer.write(VersionFieldSerializer.java:105)
	at com.esotericsoftware.kryo.Kryo.writeObject(Kryo.java:642)
	at io.micronaut.tasks.impl.kryo.GraphingKryo.writeObject(GraphingKryo.java:314)
	at com.esotericsoftware.kryo.serializers.ReflectField.write(ReflectField.java:71)
	at com.esotericsoftware.kryo.serializers.VersionFieldSerializer.write(VersionFieldSerializer.java:105)
	at com.esotericsoftware.kryo.Kryo.writeObject(Kryo.java:627)

Turning off optimized generics is a functioning workaround. The failure happens here:

	@Override
	public int pushTypeVariables (GenericsHierarchy hierarchy, GenericType[] args) {
		// Do not store type variables if hierarchy is empty, or we do not have arguments for all root parameters, or we have more
		// arguments than the hierarchy has parameters.
		if (hierarchy.total == 0 || hierarchy.rootTotal > args.length || args.length > hierarchy.counts.length) return 0;

		int startSize = this.argumentsSize;

		// Ensure arguments capacity.
		int sizeNeeded = startSize + hierarchy.total;
		if (sizeNeeded > arguments.length) {
			Type[] newArray = new Type[Math.max(sizeNeeded, arguments.length << 1)];
			System.arraycopy(arguments, 0, newArray, 0, startSize);
			arguments = newArray;
		}

		// Resolve and store the type arguments.
		int[] counts = hierarchy.counts;
		TypeVariable[] params = hierarchy.parameters;
		for (int i = 0, p = 0, n = args.length; i < n; i++) {
			GenericType arg = args[i];
			Class resolved = arg.resolve(this);
			if (resolved == null) continue;
			int count = counts[i];
			if (arg == null)
				p += count;
			else {
				for (int nn = p + count; p < nn; p++) {
					arguments[argumentsSize] = params[p];     /// <--- crash here
					arguments[argumentsSize + 1] = resolved;
					argumentsSize += 2;
				}
			}
		}

		return argumentsSize - startSize;
	}

From a bit of debugging, the issue is that sizeNeeded seems to be calculated wrong. There are two iterations of the outer for loop, but after the first the array is full and argumentsSize points off the end. My guess is that the test suite doesn't exercise the case where the arguments array gets so full it needs to be expanded, because normal object graphs never encounter it. But I'm not sure. The for loops are structured in a rather confusing way.

@theigl
Copy link
Collaborator

theigl commented Oct 28, 2024

The whole generics optimization code is very complex and used to be quite buggy. We managed to fix almost all edge cases but apparently there are more. I'm afraid I won't be able to fix this without a minimal reproducer.

I'd recommend that you simply keep generics optimization turned off. Serialization should be faster but your serialized size will increase a bit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants