apollographql / apollo-kotlin

:rocket:  A strongly-typed, caching GraphQL client for the JVM, Android, and Kotlin multiplatform.
https://www.apollographql.com/docs/kotlin
MIT License
3.76k stars 653 forks source link

Cascade cache remove loops forever on cyclic references #6133

Closed cvb941 closed 2 months ago

cvb941 commented 2 months ago

Version

4.0.0

Summary

Our schema:

image Screenshot 2024-08-26 at 14 50 36

When deleting the Record object using its CacheKey with cascade = true. It seems to loop forever.

Here's the remove method: https://github.com/apollographql/apollo-kotlin/blob/e969ade1ae58513a0f11b74d250cef094d8bb1e0/libraries/apollo-normalized-cache-api/src/commonMain/kotlin/com/apollographql/apollo/cache/normalized/api/internal/OptimisticCache.kt#L46

Just looking at the code, there is a recursive call which probably calls remove between those two objects indefinitely. The cascade remove should probably gather all unique references first and then delete them.

martinbonnin commented 2 months ago

Thanks for sending this 🙏 Do you have a reproducer by any chance? The remove() call there should be mostly idempotent so calling it multiple times with the same cache key should be harmless I think (recordJournal should be null the 2nd time). But there could very well be something else. I'll keep digging but a reproducer would help a ton.

cvb941 commented 2 months ago

Yes I missed that, you're right the entries should be removed from the journal and lruCache in the memory cache. For some reason the process gets stuck though. I will investigate more and maybe setup a reproduction.

cvb941 commented 2 months ago

Okay I think I got it. The problem is in the SQL cache (I have the memory cache chained to SQL)

Here is the offending method: https://github.com/apollographql/apollo-kotlin/blob/e969ade1ae58513a0f11b74d250cef094d8bb1e0/libraries/apollo-normalized-cache-sqlite/src/commonMain/kotlin/com/apollographql/apollo/cache/normalized/sql/SqlNormalizedCache.kt#L124

It keeps trying to delete children before itself and thus create a loop.

image
martinbonnin commented 2 months ago

Excelent catch! Do you want to submit a fix?

cvb941 commented 2 months ago

Sure, I'll see when I'll get to it though :)

cvb941 commented 2 months ago

Okay I took a look at this and I stopped at having trouble with just opening the Apollo project in the IDE. I tried both Android Studio and IDEA but they fail at processing the source files.

I guess it would be faster for you to fix this properly, it should be very simple for you.

martinbonnin commented 2 months ago

I tried both Android Studio and IDEA but they fail at processing the source files.

Interesting. This was an issue a couple of years ago but it's been a while I haven't seen any issue. Do you have any Gradle sync issue?

cvb941 commented 2 months ago

Gradle sync passes, I found out the issue is caused by the SQLDelight plugin. Do you use it without any issues? Disabling it fixed both IDEs.

martinbonnin commented 2 months ago

Ah good catch. Indeed, I do not have the SQLDelight plugin installed. Might be worth filing a bug if it's reproducible, I'll look into this. Do you still want me to do the patch?

cvb941 commented 2 months ago

I reported the exceptions from IDEA. I'm working on the patch now 😎

github-actions[bot] commented 2 months ago

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Kotlin usage and allow us to serve you better.