google / ksp

Kotlin Symbol Processing API
https://github.com/google/ksp
Apache License 2.0
2.84k stars 266 forks source link

KSP2: KtInvalidLifetimeOwnerAccessException: Access to invalid KtAlwaysAccessibleLifetimeToken: PSI has changed since creation #1854

Open ting-yuan opened 5 months ago

ting-yuan commented 5 months ago

See https://youtrack.jetbrains.com/issue/KT-67158 for test cases.

ting-yuan commented 5 months ago

The keys of androidx.room.compiler.processing.ksp.KspArrayType.Factory.reverseBuiltinArrayLookup are stale. It could be a bug in Room.

FooIbar commented 4 months ago

Is the Room team aware of this? Didn't find it in the public tracker.

vRallev commented 4 months ago

I'm running into the same issue with my processor and another processor. What I'm doing is caching a KSClassDeclaration through multiple rounds and then calling packageName throws this error:

ksp.org.jetbrains.kotlin.analysis.api.lifetime.KtInvalidLifetimeOwnerAccessException: Access to invalid ksp.org.jetbrains.kotlin.analysis.api.lifetime.KtAlwaysAccessibleLifetimeToken@a17dbac: PSI has changed since creation
    at ksp.org.jetbrains.kotlin.analysis.api.fir.utils.ValidityAwareCachedValue.getValue-impl(ValidityAwareCachedValue.kt:33)
    at ksp.org.jetbrains.kotlin.analysis.api.fir.symbols.KtFirNamedClassOrObjectSymbol.getPsi(KtFirNamedClassOrObjectSymbol.kt:36)
    at com.google.devtools.ksp.impl.symbol.kotlin.UtilKt.toContainingFile(util.kt:236)
    at com.google.devtools.ksp.impl.symbol.kotlin.AbstractKSDeclarationImpl$containingFile$2.invoke(AbstractKSDeclarationImpl.kt:77)
    at com.google.devtools.ksp.impl.symbol.kotlin.AbstractKSDeclarationImpl$containingFile$2.invoke(AbstractKSDeclarationImpl.kt:76)
    at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:74)
    at com.google.devtools.ksp.impl.symbol.kotlin.AbstractKSDeclarationImpl.getContainingFile(AbstractKSDeclarationImpl.kt:76)
    at com.google.devtools.ksp.impl.symbol.kotlin.AbstractKSDeclarationImpl$packageName$2.invoke(AbstractKSDeclarationImpl.kt:83)
    at com.google.devtools.ksp.impl.symbol.kotlin.AbstractKSDeclarationImpl$packageName$2.invoke(AbstractKSDeclarationImpl.kt:80)
    at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:74)
    at com.google.devtools.ksp.impl.symbol.kotlin.AbstractKSDeclarationImpl.getPackageName(AbstractKSDeclarationImpl.kt:80)
    at com.amazon.lastmile.app.platform.inject.processor.MergeComponentProcessor.generateComponentInterface(MergeComponentProcessor.kt:91)

To me this seems to be a regression in KSP 2. I tested with 2.0.0-1.0.22 and ksp.useKSP2=true.

I'm running into this issue with another processor that defers processing of symbols to later rounds and caches these symbols. When it's actually processing the symbols the same exception is thrown:

ksp.org.jetbrains.kotlin.analysis.api.lifetime.KtInvalidLifetimeOwnerAccessException: Access to invalid ksp.org.jetbrains.kotlin.analysis.api.lifetime.KtAlwaysAccessibleLifetimeToken@5bf278a3: PSI has changed since creation
    at ksp.org.jetbrains.kotlin.analysis.api.fir.utils.ValidityAwareCachedValue.getValue-impl(ValidityAwareCachedValue.kt:33)
    at ksp.org.jetbrains.kotlin.analysis.api.fir.types.KtFirClassErrorType.getAnnotationsList(KtFirClassErrorType.kt:50)
    at com.google.devtools.ksp.impl.symbol.kotlin.KSTypeImpl$Companion.getCached(KSTypeImpl.kt:40)
    at com.google.devtools.ksp.impl.symbol.kotlin.KSTypeReferenceImpl.resolve(KSTypeReferenceImpl.kt:68)
    at com.google.devtools.ksp.visitor.KSValidateVisitor.visitTypeReference(KSValidateVisitor.kt:57)
    at com.google.devtools.ksp.visitor.KSValidateVisitor.visitTypeReference(KSValidateVisitor.kt:5)
    at com.google.devtools.ksp.impl.symbol.kotlin.KSTypeReferenceImpl.accept(KSTypeReferenceImpl.kt:82)
    at com.google.devtools.ksp.visitor.KSValidateVisitor.visitClassDeclaration(KSValidateVisitor.kt:64)
    at me.tatarka.inject.compiler.ksp.InjectProcessor$validate$1.visitClassDeclaration(InjectProcessor.kt:107)
    at me.tatarka.inject.compiler.ksp.InjectProcessor$validate$1.visitClassDeclaration(InjectProcessor.kt:87)
    at com.google.devtools.ksp.impl.symbol.kotlin.KSClassDeclarationImpl.accept(KSClassDeclarationImpl.kt:175)
    at me.tatarka.inject.compiler.ksp.InjectProcessor.validate(InjectProcessor.kt:86)
    at me.tatarka.inject.compiler.ksp.InjectProcessor.process(InjectProcessor.kt:53)
    at com.google.devtools.ksp.impl.KotlinSymbolProcessing.execute(KotlinSymbolProcessing.kt:533)
    at com.tschuchort.compiletesting.Ksp2PrecursorTool.execute(Ksp2.kt:112)

The kotlin-inject processor caches the deferred classes here.

~This happens while running unit tests using https://github.com/ZacSweers/kotlin-compile-testing/releases/tag/0.5.0. This could be related.~ This happens during normal compilation as well and is not related to the compile testing library.

ting-yuan commented 4 months ago

Using elements across rounds is currently unsupported, because generated code may affect the elements. For example, the return type of a function may be an error in round 1 and defined by a class generated in round 2. In KSP1, caching elements across rounds happens to work for some cases but not all. In KSP2, the lifecycle check is more strict and the error is always thrown. This is not a regression.

KSP's deferral mechanism (returning symbols to be deferred from process()) is designed to handle this use case. Please try to use it and let us know if it doesn't serve your use case well.

vRallev commented 4 months ago

KSP's deferral mechanism (returning symbols to be deferred from process()) is designed to handle this use case. Please try to use it and let us know if it doesn't serve your use case well.

I'm not sure I understand this sentence. The kotlin-inject processor supports multiple rounds and defers generating files when needed. This creates challenges though. To avoid generating duplicate files, you either must track which were already generated when getting symbols with an annotation, or you get symbols of with the annotation only of the new files of this round. kotlin-inject does the latter and therefore must track the deferred symbols. Even the previous options requires tracking symbols from the previous round.

An alternative is to try generating duplicate files and catch the exception, but that doesn't sound right. I feel like I'm missing something or there's a gap.

My processor is in a similar situation. My workaround for now looks something like this, which only works because I can "reload" or "refresh" the class symbol:

deferredClassesFromPreviousRound.forEach {
    val clazz = resolver.getClassDeclarationByName(it.qualifiedName!!)!!
    generate...(resolver, clazz)
}

But all of this feels wrong.

evant commented 4 months ago

Fyi I had asked about this originally when implementing and I told that what I was doing was ok https://kotlinlang.slack.com/archives/C013BA8EQSE/p1635106134012300

evant commented 4 months ago

My last comment in that thread still stands

so the part that caught me up wasn't that you needed to return them, but that you need keep track of them yourself for reprocessing. I erroneously thought they'd be given back as new on the next round.

You have to track what you deferred yourself between rounds as you won't get those symbols given back to you which makes it very easy to trip on this issue.

ting-yuan commented 4 months ago

The deferral mechanism is designed around Resolver.getSymbolsWithAnntation. In addition to newly generated files, the coming round also considers deferred symbols in getSymbolsWithAnntation, the implementation of which is literally:

    override fun getSymbolsWithAnnotation(annotationName: String, inDepth: Boolean): Sequence<KSAnnotated> {
        // ...
        return (newSymbols + deferredSymbolsRestored).asSequence().filter(::checkAnnotated)
    }

There are no other ways to access deferred symbols though, so it makes more sense to defer the root elements than others, or at least those with annotation. We should probably have created another API that allows access to all deferred symbols. Before doing that, I'd like to know what you think. Is the current API fit your use case?

vRallev commented 4 months ago

From my own testing, the API isn't reliable. In our internal project I see non-deferred symbols from previous rounds popping up again. So I thought this was by design until I checked the documentation yesterday. Any chance this changed recently? I'm not sure if I can isolate the problem, it definitely happens in unit tests.

ting-yuan commented 4 months ago

Would you mind checking if the popped up symbols were (indirectly) from getAllFiles (instead of getNewFiles) or getClassDeclarationByName? which returns / searches all files, processed or not.

evant commented 4 months ago

The deferral mechanism is designed around Resolver.getSymbolsWithAnntation

Oh that's good to know, I was using getNewFiles() directly since I know where my annotation may be present I can resolve less of the ast. But that does not include the deferred symbols.

evant commented 4 months ago

Before doing that, I'd like to know what you think. Is the current API fit your use case?

Even if I switched I'd think I'd still want to get the deferred symbols directly so I can report errors on the final round in finish().

evant commented 4 months ago

Just pushed up https://github.com/evant/kotlin-inject/pull/393/files to fix this. Should show off the trade-offs that are needed with the current api using getSymbolsWithAnnotation and the extra lookups required in finish().

vRallev commented 4 months ago

Would you mind checking if the popped up symbols were (indirectly) from getAllFiles (instead of getNewFiles) or getClassDeclarationByName? which returns / searches all files, processed or not.

No, I'm definitely using

fun getSymbolsWithAnnotation(annotationName: String, inDepth: Boolean = false): Sequence<KSAnnotated>

And because of this issue I have to do this:

    fun Resolver.getNewSymbolsWithAnnotation(annotation: KClass<*>): Sequence<KSAnnotated> {
        val newFiles = getNewFiles().toSet()
        return getSymbolsWithAnnotation(annotation)
            .filter { it.containingFile in newFiles }
    }
evant commented 4 months ago

@vRallev won't the code you posted filter out the deferred symbols again? Since they may not be from new files for the round.

vRallev commented 4 months ago

Yes, currently we track deferred symbols in our processor like you do.

joffrey-bion commented 3 days ago

FYI I'm facing this as well when trying to run Koin with KSP2. I'm not 100% sure whether it's the same issue, because the stacktrace is slightly different:

Exception in thread "main" ksp.org.jetbrains.kotlin.analysis.api.lifetime.KaInvalidLifetimeOwnerAccessException: Access to invalid ksp.org.jetbrains.kotlin.analysis.api.platform.lifetime.KotlinAlwaysAccessibleLifetimeToken@8f09a02: PSI has changed since creation
    at ksp.org.jetbrains.kotlin.analysis.api.fir.annotations.KaFirAnnotationListForDeclaration.iterator(KaFirAnnotationListForDeclaration.kt:107)
    at com.google.devtools.ksp.impl.symbol.kotlin.UtilKt$annotations$3$1.invoke(util.kt:1032)
    at com.google.devtools.ksp.impl.symbol.kotlin.UtilKt$annotations$3$1.invoke(util.kt:375)
    at com.google.devtools.ksp.impl.symbol.kotlin.KSAnnotationImpl$annotationApplication$2.invoke(KSAnnotationImpl.kt:58)
    at com.google.devtools.ksp.impl.symbol.kotlin.KSAnnotationImpl$annotationApplication$2.invoke(KSAnnotationImpl.kt:57)
    at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:74)
    at com.google.devtools.ksp.impl.symbol.kotlin.KSAnnotationImpl.getAnnotationApplication(KSAnnotationImpl.kt:57)
    at com.google.devtools.ksp.impl.symbol.kotlin.KSAnnotationImpl.access$getAnnotationApplication(KSAnnotationImpl.kt:41)
    at com.google.devtools.ksp.impl.symbol.kotlin.KSAnnotationImpl$arguments$2.invoke(KSAnnotationImpl.kt:73)
    at com.google.devtools.ksp.impl.symbol.kotlin.KSAnnotationImpl$arguments$2.invoke(KSAnnotationImpl.kt:72)
    at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:74)
    at com.google.devtools.ksp.impl.symbol.kotlin.KSAnnotationImpl.getArguments(KSAnnotationImpl.kt:72)
    at org.koin.compiler.metadata.AnnotationMetadataKt.includedModules(AnnotationMetadata.kt:81)
    at org.koin.compiler.scanner.ModuleScanner.getIncludedModules(ModuleScanner.kt:69)
    at org.koin.compiler.scanner.ModuleScanner.createClassModule(ModuleScanner.kt:32)
    at org.koin.compiler.scanner.KoinMetaDataScanner.scanClassModules(KoinMetaDataScanner.kt:130)
    at org.koin.compiler.scanner.KoinMetaDataScanner.scanKoinModules(KoinMetaDataScanner.kt:76)
    at org.koin.compiler.BuilderProcessor.process(BuilderProcessor.kt:57)
    at com.google.devtools.ksp.impl.KotlinSymbolProcessing.execute(KotlinSymbolProcessing.kt:540)
    at com.google.devtools.ksp.cmdline.KSPJvmMainKt.runWithArgs(KSPJvmMain.kt:45)
    at com.google.devtools.ksp.cmdline.KSPJvmMain$Companion.main(KSPJvmMain.kt:20)
    at com.google.devtools.ksp.cmdline.KSPJvmMain.main(KSPJvmMain.kt)