search

evant / kotlin-inject

Dependency injection lib for kotlin
Apache License 2.0
1.14k stars 51 forks source link

Fix `KtInvalidLifetimeOwnerAccessException` with KSP2 #392

Closed vRallev closed 3 weeks ago

vRallev commented 3 weeks ago

InjectProcessor remembers deferred symbols across rounds. KSP (KSP 2 in particular) forbids to cache symbols across rounds and verifies the lifecycle. The fix is trying to get new symbols of the deferred classes each round.

For more details: https://github.com/google/ksp/issues/1854

evant commented 3 weeks ago

I think we need to do the same thing with deferredFunctions too right?

evant commented 3 weeks ago

Might be a little cleaner to only hold on to the qualified names between rounds, that way it has to be looked up again instead of having to remember to.

evant commented 3 weeks ago

We really should have some more rounds tests to verify things like this, not going to hold up this pr on that though

vRallev commented 3 weeks ago

I think we need to do the same thing with deferredFunctions too right?

Yes, but I'm not sure what these function cover or how to "refresh" the symbols. Are these only top-level functions? I also couldn't test this whereas I had reproducible steps for classes and wanted to start with that.

Might be a little cleaner to only hold on to the qualified names between rounds, that way it has to be looked up again instead of having to remember to.

I thought about that, but decided against it in the end. Reason is that APIs like qualifiedName are nullable and I'm falling back to the original symbol in case we need to cover anonymous classes or other rare cases. Not all APIs on a symbol throw this exception.

I really believe theres a lack of API support in KSP.

evant commented 3 weeks ago

Are these only top-level functions?

Yes, they were added to support KmpComponentCreate

evant commented 3 weeks ago

in case we need to cover anonymous classes or other rare cases

I only expect we'll ever be scanning classes & top-level functions which are always supported right? Falling back to the original symbol that is 'unsupported' sounds like a recipe for some rare hard-to-debug issues to me.

evant commented 3 weeks ago

https://github.com/evant/kotlin-inject/pull/393 is what I mean. You don't need to look up the symbols again except in the finish case which should hopefully be less common as it implies something was never resolved.

vRallev commented 3 weeks ago

I left comments. Let's continue the discussion there and then merge one or the other fix.

evant commented 3 weeks ago

Superseded by https://github.com/evant/kotlin-inject/pull/393