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

StackOverflowError in VarMap.map when calling GenericTypeReflector.getParameterTypes() #27

Closed
AndreasTu opened this issue Mar 6, 2024 · 7 comments · Fixed by #30
Closed

Comments

@AndreasTu
Copy link

mfriedenhagen reported an issue in Spock spockframework/spock#1909 where geantyref throws a StackOverflowError.

I have removed the Spock dependency from the reproducer attached to the Spock issue, and the error is still there:

import java.lang.reflect.Method;
import java.util.Objects;

public class StackOverflowReproducer {

  static abstract class Status<StatusT, StatusBuilderT> {
    static class Builder<StatusT, StatusBuilderT> {
    }
  }

  static abstract class Resource<StatusT, StatusBuilderT> {
  }

  static class ProjectValidator {
    public <
      StatusT extends Status<StatusT, StatusBuilderT>,
      StatusBuilderT extends Status.Builder<StatusT, StatusBuilderT>,
      T extends Resource<StatusT, StatusBuilderT>>
    void validate(long id, Class<T> klass) {
      throw new UnsupportedOperationException("Just for testing");
    }
  }

  public static void main(String[] args) throws NoSuchMethodException {
    Class<?> cls = ProjectValidator.class;
    Method method = Objects.requireNonNull(cls.getMethod("validate", long.class, Class.class));
    io.leangen.geantyref.GenericTypeReflector.getParameterTypes(method, cls);
  }
}

The exception stack looks like:

java.lang.StackOverflowError
	at java.base/sun.reflect.generics.reflectiveObjects.TypeVariableImpl.getGenericDeclaration(TypeVariableImpl.java:144)
	at java.base/sun.reflect.generics.reflectiveObjects.TypeVariableImpl.typeVarIndex(TypeVariableImpl.java:227)
	at java.base/sun.reflect.generics.reflectiveObjects.TypeVariableImpl.getAnnotatedBounds(TypeVariableImpl.java:220)
	at java.base/sun.reflect.annotation.AnnotatedTypeFactory$AnnotatedTypeVariableImpl.getAnnotatedBounds(AnnotatedTypeFactory.java:383)
	at io.leangen.geantyref.VarMap.map(VarMap.java:98)
	at io.leangen.geantyref.VarMap.map(VarMap.java:115)
	at io.leangen.geantyref.VarMap.map(VarMap.java:148)
	at io.leangen.geantyref.VarMap.map(VarMap.java:98)

The used geantyref version is io.leangen.geantyref:geantyref:1.3.15
Tested for Java 8 and 17

@leonard84
Copy link

leonard84 commented Mar 21, 2024

This problem can be simplified to a single self-referential type.

import java.lang.reflect.Method;
import java.util.Objects;

public class StackOverflowReproducer {

  static abstract class Resource<StatusT> {
  }

  static class ProjectValidator {
    public <T extends Resource<T>>
    void validate(long id, Class<T> klass) {
      throw new UnsupportedOperationException("Just for testing");
    }
  }

  public static void main(String[] args) throws NoSuchMethodException {
    Class<?> cls = ProjectValidator.class;
    Method method = Objects.requireNonNull(cls.getMethod("validate", long.class, Class.class));
    io.leangen.geantyref.GenericTypeReflector.getParameterTypes(method, cls);
  }
}

@leonard84
Copy link

This can probably be avoided by tracking already seen AnnotatedTypeVariables in io.leangen.geantyref.VarMap#map(java.lang.reflect.AnnotatedType, io.leangen.geantyref.VarMap.MappingMode). However, I'm unsure what the correct return type would be in this case, maybe just Resource without any generic information? Or, do we need to throw an UnresolvedTypeVariableException.

AndreasTu added a commit to AndreasTu/geantyref that referenced this issue Jul 15, 2024
The VarMap now tracks the currently resolving TypeVars
to detect, if we already resolve the same TypeVar.

Fixes leangen#27
@AndreasTu
Copy link
Author

@leonard84 The VarMap.MappingMode.EXACT mode does already throw an UnresolvedTypeVariableException in that case, so only the VarMap.MappingMode.ALLOW_INCOMPLETE case is left, where I would say we shall just return the imcomplete type.

I have created the PR #30, which fixes the issue but returning the incomplete type.

AndreasTu added a commit to AndreasTu/geantyref that referenced this issue Jul 15, 2024
The VarMap now tracks the currently resolving TypeVars
to detect, if we already resolve the same TypeVar.

Fixes leangen#27
@kaqqao kaqqao closed this as completed in 1465873 Aug 25, 2024
@kaqqao
Copy link
Member

kaqqao commented Aug 25, 2024

Phew, this thing really sent me soul-searching 😅 Nothing seemed correct. Terminating with Object would violate type constraints, returning the given type as-is, as I suggested here would potentially skip merging annotations...
I think I got the right semantics in the end (the type stays recursive, annotations get merged) but it required some rather ugly code 😓
Can you (@AndreasTu, @leonard84) please verify it now works correctly for you in Spock? If so, I can release this immediately.

@AndreasTu
Copy link
Author

@kaqqao I have tested the Spock issue report against the geantyref master, and it fixes the issue. Thanks a lot!

@leonard84
Copy link

leonard84 commented Aug 26, 2024

Thanks, @kaqqao, for the fix. I'm looking forward to the release.

@kaqqao
Copy link
Member

kaqqao commented Aug 27, 2024

Released v1.3.16 just now 🚀

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

Successfully merging a pull request may close this issue.

3 participants