google / dagger

A fast dependency injector for Android and Java.
https://dagger.dev
Apache License 2.0
17.44k stars 2.02k forks source link

Stackoverflow when exposing parents of Subcomponents #4330

Open ursusursus opened 5 months ago

ursusursus commented 5 months ago

I want to expose parent reference of a subcomponent, since the generated implementation has the reference. (Trying to build a component cache that is a tree)

Simply adding a val parent: AppComponent on UserComponent worked. But unfortunately it doesn't work for subcomponent trees deeper than 2, then compilation stackoverflows.

@AppScope
@Component
interface AppComponent {
    val userComponentFactory: UserComponent.Factory

    @Component.Factory
    interface Factory {
        fun create(@BindsInstance context: Context): AppComponent
    }
}

@UserScope
@Subcomponent
interface UserComponent {
    val parent: AppComponent
    val subscriberComponentFactory: SubscriberComponent.Factory

    @Subcomponent.Factory
    interface Factory {
        fun create(@BindsInstance userId: UserId): UserComponent
    }
}

@SubscriberScope
@Subcomponent
interface SubscriberComponent {
    val what: UserComponent .... makes compile Stackoverflow - removing this line makes it compile

    @Subcomponent.Factory
    interface Factory {
        fun create(@BindsInstance subscriberId: SubscriberId): SubscriberComponent
    }
}
Caused by: java.lang.reflect.InvocationTargetException
    at jdk.internal.reflect.GeneratedMethodAccessor1131.invoke(Unknown Source)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
    at org.jetbrains.kotlin.kapt3.base.AnnotationProcessingKt.doAnnotationProcessing(annotationProcessing.kt:92)
    at org.jetbrains.kotlin.kapt3.base.AnnotationProcessingKt.doAnnotationProcessing$default(annotationProcessing.kt:33)
    at org.jetbrains.kotlin.kapt3.base.Kapt.kapt(Kapt.kt:47)
    ... 33 more
Caused by: com.sun.tools.javac.processing.AnnotationProcessingError: java.lang.StackOverflowError
    at jdk.compiler/com.sun.tools.javac.processing.JavacProcessingEnvironment.callProcessor(Unknown Source)
    at jdk.compiler/com.sun.tools.javac.processing.JavacProcessingEnvironment.discoverAndRunProcs(Unknown Source)
    at jdk.compiler/com.sun.tools.javac.processing.JavacProcessingEnvironment$Round.run(Unknown Source)
    at jdk.compiler/com.sun.tools.javac.processing.JavacProcessingEnvironment.doProcessing(Unknown Source)
    at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.processAnnotations(Unknown Source)
    ... 38 more
Caused by: java.lang.StackOverflowError
    at com.squareup.javapoet.ClassName.get(ClassName.java:176)
    at com.squareup.javapoet.TypeName.get(TypeName.java:339)
    at com.squareup.javapoet.TypeName.get(TypeName.java:323)
    at com.squareup.javapoet.WildcardTypeName.subtypeOf(WildcardTypeName.java:84)
    at com.squareup.javapoet.WildcardTypeName.get(WildcardTypeName.java:111)
    at com.squareup.javapoet.TypeName$1.visitWildcard(TypeName.java:307)
    at com.squareup.javapoet.TypeName$1.visitWildcard(TypeName.java:248)
    at jdk.compiler/com.sun.tools.javac.code.Type$WildcardType.accept(Unknown Source)
    at com.squareup.javapoet.TypeName.get(TypeName.java:248)
    at com.squareup.javapoet.TypeName$1.visitDeclared(TypeName.java:286)
    at com.squareup.javapoet.TypeName$1.visitDeclared(TypeName.java:248)
    at jdk.compiler/com.sun.tools.javac.code.Type$ClassType.accept(Unknown Source)
    at com.squareup.javapoet.TypeName.get(TypeName.java:248)
    at com.squareup.javapoet.TypeName.get(TypeName.java:243)
    at dagger.spi.internal.shaded.androidx.room.compiler.processing.JavaPoetExtKt.safeTypeName(JavaPoetExt.kt:94)
    at dagger.spi.internal.shaded.androidx.room.compiler.processing.javac.JavacType$xTypeName$2.invoke(JavacType.kt:86)
    at dagger.spi.internal.shaded.androidx.room.compiler.processing.javac.JavacType$xTypeName$2.invoke(JavacType.kt:84)
    at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:74)
    at dagger.spi.internal.shaded.androidx.room.compiler.processing.javac.JavacType.getXTypeName(JavacType.kt:84)
    at dagger.spi.internal.shaded.androidx.room.compiler.processing.javac.JavacType.access$getXTypeName(JavacType.kt:39)
    at dagger.spi.internal.shaded.androidx.room.compiler.processing.javac.JavacType$typeName$2.invoke(JavacType.kt:81)
    at dagger.spi.internal.shaded.androidx.room.compiler.processing.javac.JavacType$typeName$2.invoke(JavacType.kt:80)
    at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:74)
    at dagger.spi.internal.shaded.androidx.room.compiler.processing.javac.JavacType.getTypeName(JavacType.kt:80)
    at dagger.spi.internal.shaded.androidx.room.compiler.processing.InternalXAnnotationValue$Kind$Companion.of(InternalXAnnotationValue.kt:36)
    at dagger.spi.internal.shaded.androidx.room.compiler.processing.InternalXAnnotationValue$kind$2.invoke(InternalXAnnotationValue.kt:57)
    at dagger.spi.internal.shaded.androidx.room.compiler.processing.InternalXAnnotationValue$kind$2.invoke(InternalXAnnotationValue.kt:56)
    at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:74)
    at dagger.spi.internal.shaded.androidx.room.compiler.processing.InternalXAnnotationValue.getKind(InternalXAnnotationValue.kt:56)
    at dagger.spi.internal.shaded.androidx.room.compiler.processing.InternalXAnnotationValue.hasAnnotationValue(InternalXAnnotationValue.kt:69)
    at dagger.internal.codegen.base.DaggerSuperficialValidation.validateAnnotationValue(DaggerSuperficialValidation.java:458)
    at dagger.internal.codegen.base.DaggerSuperficialValidation.validateAnnotationValues(DaggerSuperficialValidation.java:443)
    at dagger.internal.codegen.base.DaggerSuperficialValidation.validateAnnotationValue(DaggerSuperficialValidation.java:457)
    at dagger.internal.codegen.base.DaggerSuperficialValidation.validateAnnotationValues(DaggerSuperficialValidation.java:443)
    at dagger.internal.codegen.base.DaggerSuperficialValidation.validateAnnotation(DaggerSuperficialValidation.java:412)
    at dagger.internal.codegen.base.DaggerSuperficialValidation.validateAnnotationOf(DaggerSuperficialValidation.java:217)
    at dagger.internal.codegen.base.ComponentAnnotation.lambda$anyComponentAnnotation$0(ComponentAnnotation.java:180)
    at dagger.internal.codegen.base.ComponentAnnotation.anyComponentAnnotation(ComponentAnnotation.java:178)
    at dagger.internal.codegen.base.ComponentAnnotation.anyComponentAnnotation(ComponentAnnotation.java:170)
    at dagger.internal.codegen.validation.ComponentValidator$ElementValidator.componentAnnotation(ComponentValidator.java:159)
    at dagger.internal.codegen.validation.ComponentValidator$ElementValidator.validateCreators(ComponentValidator.java:226)
    at dagger.internal.codegen.validation.ComponentValidator$ElementValidator.validateElement(ComponentValidator.java:174)
    at dagger.internal.codegen.validation.ComponentValidator.validateUncached(ComponentValidator.java:136)
    at dagger.internal.codegen.base.Util.reentrantComputeIfAbsent(Util.java:33)
    at dagger.internal.codegen.validation.ComponentValidator.validate(ComponentValidator.java:132)
    at dagger.internal.codegen.validation.ComponentValidator$ElementValidator.lambda$validateSubcomponents$11(ComponentValidator.java:533)
    at com.google.common.collect.Maps$KeySet.lambda$forEach$0(Maps.java:4043)
    at com.google.common.collect.Maps$KeySet.forEach(Maps.java:4043)
    at dagger.internal.codegen.validation.ComponentValidator$ElementValidator.validateSubcomponents(ComponentValidator.java:533)
    at dagger.internal.codegen.validation.ComponentValidator$ElementValidator.validateElement(ComponentValidator.java:181)
    at dagger.internal.codegen.validation.ComponentValidator.validateUncached(ComponentValidator.java:136)
    at dagger.internal.codegen.base.Util.reentrantComputeIfAbsent(Util.java:33)
    at dagger.internal.codegen.validation.ComponentValidator.validate(ComponentValidator.java:132)
    at dagger.internal.codegen.validation.ComponentValidator$ElementValidator.lambda$validateSubcomponents$11(ComponentValidator.java:533)
    at com.google.common.collect.Maps$KeySet.lambda$forEach$0(Maps.java:4043)
    at com.google.common.collect.Maps$KeySet.forEach(Maps.java:4043)
    .... repeats

dagger 2.51.1

Chang-Eric commented 5 months ago

We should probably do better than stackoverflow, however, what's happening here is that there are (unfortunately) multiple ways to declare a subcomponent as a child of another component/subcomponent, and one of those ways is to add a entry point method on the parent component/subcomponent interface that returns the child subcomponent.

So when you add the val what: UserComponent, you're actually making the UserComponent a child of SubscriberComponent, which is what causes the infinite loop.

You can probably work around this for now by using an @Binds to bind the UserComponent binding to another interface that doesn't have the @Subcomponent on it (e.g. @Subcomponent interface UserComponent : RealUserComponent), and then make your val use the interface without the @Subcomponent.

It looks like there's a bug in our validation logic that causes this loop.

ursusursus commented 5 months ago

I think I follow, but if you delete SubscriberComponent altogether, then it compiles & looking at generated sources of UserComponent, the val parent: AppComponent does just expose the appComponentImpl referencd which is passed in normally via constructor..so everything as expected.

I checked in the debugger, the returned instance is the same as the one used to create UserComponent instance from..so no surprises.

Is it because AppComponent is a @Component (and not a @Subcomponent?)

Chang-Eric commented 5 months ago

Is it because AppComponent is a @Component (and not a @Subcomponent?)

Yea, you can't make a @Component a child of another component, so we interpret this just as any other getter.

ursusursus commented 5 months ago

I see. So other than fixing the overflow to get a better error message, and hacking around with @Binds, a proper way doesnt seem feasible, since all viable syntax is already taken?

Chang-Eric commented 5 months ago

Yea, you won't be able to do exactly what you wrote in the original comment, however, we should fix the overflow. Also, I think if you did @Binds to a qualifier like @Parent UserComponent you'd still hit the overflow. I need to look into and discuss with team members on whether we want to allow a qualifier to be used here to make the @Binds case easier.

ursusursus commented 5 months ago

by @Binds you mean @Binds abstract fun bind(userComponent: UserComponent): RealUserComponent?

Chang-Eric commented 5 months ago

Yup, that's right.