reduceBounded StackOverflowError with self-recursing capture

[stevenschlansker]

Hi @kaqqao happy new year,

We received a bug report in Jdbi: https://github.com/jdbi/jdbi/issues/2582 It looks to actually be a bug in geantyref.

Here's the code to reproduce:

public interface UnitX<Q extends QuantityX<Q>> {
}
public interface QuantityX<Q extends QuantityX<Q>> {
}
class TestClass {
    public UnitX<?> returnType() {
        return null;
    }
}

GenericTypeReflector.reduceBounded(GenericTypeReflector.annotate(
        GenericTypeReflector.getExactReturnType(TestClass.class.getMethod("returnType"), TestClass.class)));

This results in a StackOverflowError. Thank you for any insight you can provide!

[hgschmie]

package io.leangen.geantyref;

import junit.framework.TestCase;

/**
 * https://github.com/leangen/geantyref/issues/20
 */
public class Issue20Test extends TestCase {

  public interface UnitX<Q extends QuantityX<Q>> {

    Q getUnitQ();
  }

  public interface QuantityX<Q extends QuantityX<Q>> {

    Q getQuantityQ();
  }

  static class TestClass {

    public UnitX<?> returnType() {
      return null;
    }
  }

  public void testStackOverflow() throws Exception {
    GenericTypeReflector.reduceBounded(GenericTypeReflector.annotate(
        GenericTypeReflector.getExactReturnType(TestClass.class.getMethod("returnType"),
            TestClass.class)));
  }
}

the recursion happens here:

So the CaptureType for capture of ? has an upper bound of "QuantityX<capture of ?>" for which the actual type argument is capture of ?.

As a result, when calling "reduceBounded()", the visitor oscillates between visitParameterizedType where it finds the type parameter and then visits it ("capture of ?"), which calls visitCaptureType which looks at lower and upper bound, only finds an upper bound which is Quantity<capture of ?>, then visits it, which is a parameterized type. Rinse, repeat, stack overflow.

[kaqqao]

Hi @stevenschlansker and @hgschmie, happy new year :)

Thanks for reporting this. Seems like this code had the proper checks in place once upon a time (some related comments still claim so), but they got lost as the changes accreted over time... I'll fix it and make a release in the next couple of days hopefully.

[kaqqao]

I think I've spoken too soon... What would you even expect the result to be here? I can only come up with 3 equally unsatisfying options:

1. An exception. Not StackOverflow, but still an exception, because this type can not be reduced to anything finite.
2. UnitX<QuantityX<QuantityX<... repeating infinitely>>>. While I can produce such a type instance, I can't imagine it leading you anywhere good.
3. UnitX<QuantityX> or even UnitX, all of which are equally arbitrary and not the actual answer...

Did I tie my brain in a knot and missed something obvious? What would be the pragmatic answer for your use-case in this situation?

[hgschmie]

Good question. The challenge is that this is real world code (https://unitsofmeasurement.github.io/unit-api/apidocs/javax/measure/Unit.html).

(btw, the Quantity is not even needed, public interface UnitX<Q extends UnitX<Q>> crashes in the same way)

Given that the return value is an Unit<?>, I would expect that to come back as either Unit<Quantity<?>> or Unit<Quantity<Object>>. I understand that the goal here is to enforce that the type parameter conforms to Quantity<Q>, however the type definition Foo<Q extends Foo<Q>> seems to be common enough (we actually use it in Jdbi for our SqlStatement class as well) that it should return some meaningful result.

Highly scientific research (google, stack overflow) yields this:

https://stackoverflow.com/questions/7385949/what-does-recursive-type-bound-in-generics-mean http://www.angelikalanger.com/GenericsFAQ/FAQSections/TypeParameters.html#FAQ106

[kaqqao]

Yup, I have plenty of these cases in my own code as well, just never needed to call reduceBounded on them 😅

That method is supposed to simplify types by "collapsing" them into their first bound (hence it never leaves wildcards and variables). But in the case of recursive types... I'm stumped. Terminating with Object when recursion is detected might be the most pragmatic, if incorrect, answer...

[hgschmie]

Well, we use this in a piece of code (https://github.com/jdbi/jdbi/blob/master/core/src/main/java/org/jdbi/v3/core/mapper/reflect/internal/BeanPropertiesFactory.java#L158-L173) that looks e.g. at

    public static class GenericBean<T extends String> {
        public List<T> getProperty() {
            return Collections.emptyList();
        }
    }

to determine the return type of getProperty() (which is List<String>). Extrapolating from this, when the code looks at

    public static class WeirdBean<T extends WeirdBean<T>> {
        public List<T> getProperty() {
            return Collections.emptyList();
        }
    }

which currently crashes, maybe returning a List<WeirdBean>? So basically drop the type parameter in this case? List<Object> feels wrong.

So to answer your question from earlier: I think that the example code above should resolve to UnitX<QuantityX> (resolve the ? to <Q extends QuantityX<Q>>, then detect a recursive definition and therefore use QuantityX without further recursion, thus yielding UnitX<QuantityX>.

[hgschmie]

Alternatively, refusing to resolve (throw an exception) but also offer a GenericTypeReflector.isRecursiveType(Type type) method would allow users to write code that can deal with this corner case at a higher level.

[stevenschlansker]

I agree, the answer may not be well formed in this case.

In that case, I think the theory-correct answer is to refuse to produce an answer, maybe via a specific exception (catch StackOverflowError feels really bad).

The more pragmatic answer might be to fudge it a bit and return Object or an unbounded wildcard, but it offends me a bit that the answer isn't "correct".

If geantyref raises such an error, Jdbi could then react by erasing the type. Although, this is a big hammer - there might be cases with multiple type parameters <X extends String, Q extends This<Q>> where one parameter can be reduced and the other cannot, which such an exception cannot cleanly handle...

[hgschmie]

here is a patch that works for me:

diff --git a/src/main/java/io/leangen/geantyref/GenericTypeReflector.java b/src/main/java/io/leangen/geantyref/GenericTypeReflector.java
index 4f9338d..a198412 100644
--- a/src/main/java/io/leangen/geantyref/GenericTypeReflector.java
+++ b/src/main/java/io/leangen/geantyref/GenericTypeReflector.java
@@ -1166,9 +1166,20 @@ public class GenericTypeReflector {

             @Override
             protected AnnotatedType visitCaptureType(AnnotatedCaptureType type) {
-                return type.getAnnotatedLowerBounds().length > 0
-                        ? updateAnnotations(transform(type.getAnnotatedLowerBounds()[0], this), type.getAnnotations())
-                        : updateAnnotations(transform(type.getAnnotatedUpperBounds()[0], this), type.getAnnotations());
+                AnnotatedType capturedType = type.getAnnotatedLowerBounds().length > 0
+                    ? type.getAnnotatedLowerBounds()[0]
+                    : type.getAnnotatedUpperBounds()[0];
+
+                if (capturedType instanceof AnnotatedParameterizedType) {
+                    AnnotatedType[] capturedTypes = ((AnnotatedParameterizedType) capturedType).getAnnotatedActualTypeArguments();
+                    if (capturedTypes.length == 1 && type.equals(capturedTypes[0])) {
+                        // recursive definition
+                        ParameterizedType parameterizedType = (ParameterizedType) capturedType.getType();
+                        return annotate(parameterizedType.getRawType(), type.getAnnotations());
+                    }
+                }
+
+                return updateAnnotations(transform(capturedType, this), type.getAnnotations());

I will polish this a bit and send a PR.

[stevenschlansker]

Thank you so much for fixing this bug! We'll eagerly upgrade to a new release as soon as we can :)

[johnarrr]

I reported the bug that led to this one. This was a fascinating discussion. Thank you all for looking into it and putting in a fix!

[kaqqao]

I've just released v1.3.15. Should be in Central soon. Thanks everyone for this awesome co-op! 😊