MayakaApps / Kache

Kache is a lightweight Kotlin Multiplatform caching library that supports both in-memory and persistent caches and supports different eviction strategies (LRU, FIFO, MRU, and FILO).
https://mayakaapps.github.io/Kache/latest
Apache License 2.0
136 stars 6 forks source link

InMemoryKacke - ArrayIndexOutOfBoundsException on put #249

Open caseymorris61 opened 3 months ago

caseymorris61 commented 3 months ago

Describe the bug My app crashed due to an array indexing exception when I was trying to store a value in an in-memory cache

java.lang.ArrayIndexOutOfBoundsException: length=7; index=-1
at com.mayakapps.kache.collection.MutableChain.resizeStorage$kache(Chain.kt:100)
at com.mayakapps.kache.collection.MutableChainedScatterMap.resizeStorage(MutableChainedScatterMap.kt:89)
at com.mayakapps.kache.collection.MutableScatterMap.adjustStorage(ScatterMap.kt:1314)
at com.mayakapps.kache.collection.MutableScatterMap.findInsertIndex$kache(ScatterMap.kt:1254)
at com.mayakapps.kache.collection.MutableScatterMap.put(ScatterMap.kt:991)
at com.mayakapps.kache.InMemoryKache.put(InMemoryKache.kt:233)

To Reproduce Steps to reproduce the behavior: Occurred when doing a put on an existing key. The onEntryRemoved listener correctly fired but the put failed

Expected behavior The app should not crash and the put should succeed.

System (please complete the following information):

Additional context My cache creation when it crashes looks like:

val pageCache = InMemoryKache<String, CacheableResponse>(maxSize = cacheSize) {
        strategy = KacheStrategy.LRU
        expireAfterWriteDuration = CACHE_EXPIRATION_TIME_MS.milliseconds
        onEntryRemoved = { evicted, key, oldValue, newValue ->
            println("CACHE: Page Entry Removed - Evicted: $evicted")
        }
    }

Note: If I change the Strategy to FIFO, I do not get a crash

caseymorris61 commented 3 months ago

Looking through the code is how I discovered it would work with a FIFO instead of an LRU. The issue seems to be related to resizing the MutableChainedScatterMap.insertionChain. When doing the resizing, the insertionChain.tail is -1 for some reason, and MutableChain.resizeStorage() only checks oldHead index before accessing.

I think the resizeStorage() should include more checks, but am curious if there is an issue elsewhere causing the tail of the insertion chain to remain at -1

caseymorris61 commented 2 months ago

So I did more digging and there are 2 issues around the resizing:

  1. the oldTail is not saved before the resizing in MutableChain.resizeStorage(), so it is reset to -1, and then when we try to access it, we get an index out of bounds error.
  2. When updating the extras (i.e. the time marks of the mutable timed chain), it is done out of sequece, after the index has been advanced, which was causing other issues.

I will submit an MR to fix