EsotericSoftware / kryo

Java binary serialization and cloning: fast, efficient, automatic
BSD 3-Clause "New" or "Revised" License
6.21k stars 828 forks source link

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

Open mikehearn opened 1 month ago

mikehearn commented 1 month ago

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 commented 1 month ago

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.