Revert to using stately-collections to ensure JVM thread-safety. (#27)

cache4k/build.gradle.kts

@@ -59,6 +59,7 @@ kotlin {
         val commonMain by getting {
             dependencies {
                 implementation(libs.coroutines.core)
+                implementation(libs.stately.isoCollections)
                 implementation(libs.atomicfu)
             }
         }

cache4k/src/commonMain/kotlin/io/github/reactivecircus/cache4k/KeyedSynchronizer.kt

@@ -1,5 +1,6 @@
 package io.github.reactivecircus.cache4k
 
+import co.touchlab.stately.collections.IsoMutableMap
 import kotlinx.atomicfu.locks.reentrantLock
 import kotlinx.atomicfu.locks.withLock
 import kotlinx.coroutines.sync.Mutex
@@ -10,7 +11,7 @@ import kotlinx.coroutines.sync.withLock
  */
 internal class KeyedSynchronizer<Key : Any> {
 
-    private val keyBasedMutexes: MutableMap<Key, MutexEntry> = mutableMapOf()
+    private val keyBasedMutexes = IsoMutableMap<Key, MutexEntry>()
 
     private val mapLock = reentrantLock()
 

cache4k/src/commonMain/kotlin/io/github/reactivecircus/cache4k/RealCache.kt

@@ -1,5 +1,7 @@
 package io.github.reactivecircus.cache4k
 
+import co.touchlab.stately.collections.IsoMutableMap
+import co.touchlab.stately.collections.IsoMutableSet
 import kotlinx.atomicfu.AtomicRef
 import kotlinx.atomicfu.atomic
 import kotlinx.atomicfu.update
@@ -35,7 +37,7 @@ internal class RealCache<Key : Any, Value : Any>(
     val timeSource: TimeSource,
 ) : Cache<Key, Value> {
 
-    private val cacheEntries: MutableMap<Key, CacheEntry<Key, Value>> = mutableMapOf()
+    private val cacheEntries = IsoMutableMap<Key, CacheEntry<Key, Value>>()
 
     /**
      * Whether to perform size based evictions.
@@ -61,9 +63,9 @@ internal class RealCache<Key : Any, Value : Any>(
      * A queue of unique cache entries ordered by write time.
      * Used for performing write-time based cache expiration.
      */
-    private val writeQueue: MutableSet<CacheEntry<Key, Value>>? =
+    private val writeQueue: IsoMutableSet<CacheEntry<Key, Value>>? =
         takeIf { expiresAfterWrite }?.let {
-            mutableSetOf()
+            ReorderingIsoMutableSet()
         }
 
     /**
@@ -73,9 +75,9 @@ internal class RealCache<Key : Any, Value : Any>(
      *
      * Note that a write is also considered an access.
      */
-    private val accessQueue: MutableSet<CacheEntry<Key, Value>>? =
+    private val accessQueue: IsoMutableSet<CacheEntry<Key, Value>>? =
         takeIf { expiresAfterAccess || evictsBySize }?.let {
-            mutableSetOf()
+            ReorderingIsoMutableSet()
         }
 
     override fun get(key: Key): Value? {
@@ -170,15 +172,17 @@ internal class RealCache<Key : Any, Value : Any>(
         )
 
         queuesToProcess.forEach { queue ->
-            val iterator = queue.iterator()
-            for (entry in iterator) {
-                if (entry.isExpired()) {
-                    cacheEntries.remove(entry.key)
-                    // remove the entry from the current queue
-                    iterator.remove()
-                } else {
-                    // found unexpired entry, no need to look any further
-                    break
+            queue.access {
+                val iterator = queue.iterator()
+                for (entry in iterator) {
+                    if (entry.isExpired()) {
+                        cacheEntries.remove(entry.key)
+                        // remove the entry from the current queue
+                        iterator.remove()
+                    } else {
+                        // found unexpired entry, no need to look any further
+                        break
+                    }
                 }
             }
         }
@@ -203,10 +207,12 @@ internal class RealCache<Key : Any, Value : Any>(
         checkNotNull(accessQueue)
 
         while (cacheEntries.size > maxSize) {
-            accessQueue.firstOrNull()?.run {
-                cacheEntries.remove(key)
-                writeQueue?.remove(this)
-                accessQueue.remove(this)
+            accessQueue.access {
+                it.firstOrNull()?.run {
+                    cacheEntries.remove(key)
+                    writeQueue?.remove(this)
+                    accessQueue.remove(this)
+                }
             }
         }
     }
@@ -219,7 +225,7 @@ internal class RealCache<Key : Any, Value : Any>(
             val accessTimeMark = cacheEntry.accessTimeMark.value
             cacheEntry.accessTimeMark.update { accessTimeMark + accessTimeMark.elapsedNow() }
         }
-        accessQueue?.addLastOrReorder(cacheEntry)
+        accessQueue?.add(cacheEntry)
     }
 
     /**
@@ -235,8 +241,8 @@ internal class RealCache<Key : Any, Value : Any>(
             val writeTimeMark = cacheEntry.writeTimeMark.value
             cacheEntry.writeTimeMark.update { (writeTimeMark + writeTimeMark.elapsedNow()) }
         }
-        accessQueue?.addLastOrReorder(cacheEntry)
-        writeQueue?.addLastOrReorder(cacheEntry)
+        accessQueue?.add(cacheEntry)
+        writeQueue?.add(cacheEntry)
     }
 }
 

cache4k/src/commonMain/kotlin/io/github/reactivecircus/cache4k/ReorderingIsoMutableSet.kt

@@ -0,0 +1,17 @@
+package io.github.reactivecircus.cache4k
+
+import co.touchlab.stately.collections.IsoMutableSet
+
+/**
+ * A custom [IsoMutableSet] that updates the insertion order when an element is re-inserted,
+ * i.e. an inserted element will always be placed at the end
+ * regardless of whether the element already exists.
+ */
+internal class ReorderingIsoMutableSet<T> : IsoMutableSet<T>(), MutableSet<T> {
+    override fun add(element: T): Boolean = access {
+        val exists = remove(element)
+        super.add(element)
+        // respect the contract "true if this set did not already contain the specified element"
+        !exists
+    }
+}

cache4k/src/commonTest/kotlin/io/github/reactivecircus/cache4k/MutableSetExtTest.kt → cache4k/src/commonTest/kotlin/io/github/reactivecircus/cache4k/ReorderingIsoMutableSetTest.kt

@@ -5,36 +5,36 @@ import kotlin.test.assertEquals
 import kotlin.test.assertFalse
 import kotlin.test.assertTrue
 
-class MutableSetExtTest {
+class ReorderingIsoMutableSetTest {
 
-    private val set: MutableSet<String> = mutableSetOf()
+    private val set = ReorderingIsoMutableSet<String>()
 
     @Test
     fun maintainsOriginalInsertionOrder() {
-        set.addLastOrReorder("a")
-        set.addLastOrReorder("b")
+        set.add("a")
+        set.add("b")
 
         assertEquals(listOf("a", "b"), set.toList())
     }
 
     @Test
     fun reInsertedElementMovedToTheEnd() {
-        set.addLastOrReorder("a")
-        set.addLastOrReorder("b")
-        set.addLastOrReorder("a")
+        set.add("a")
+        set.add("b")
+        set.add("a")
 
         assertEquals(listOf("b", "a"), set.toList())
     }
 
     @Test
     fun add_elementExists() {
-        set.addLastOrReorder("a")
+        set.add("a")
         assertFalse(set.add("a"))
     }
 
     @Test
     fun add_elementNotExists() {
-        assertTrue(set.addLastOrReorder("a"))
-        assertTrue(set.addLastOrReorder("b"))
+        assertTrue(set.add("a"))
+        assertTrue(set.add("b"))
     }
 }

gradle/libs.versions.toml

@@ -6,13 +6,15 @@ kotlin = "1.7.20"
 kotlinBinaryCompabilityValidator = "0.12.1"
 mavenPublish = "0.22.0"
 atomicfu = "0.18.5"
+statelyIso = "1.2.2"
 
 [libraries]
 plugin-detekt = { module = "io.gitlab.arturbosch.detekt:detekt-gradle-plugin", version.ref = "detekt" }
 plugin-dokka = { module = "org.jetbrains.dokka:dokka-gradle-plugin", version.ref = "dokka" }
 plugin-kotlin = { module = "org.jetbrains.kotlin:kotlin-gradle-plugin", version.ref = "kotlin" }
 plugin-kotlinBinaryCompatibilityValidator = { module = "org.jetbrains.kotlinx:binary-compatibility-validator", version.ref = "kotlinBinaryCompabilityValidator" }
 plugin-mavenPublish = { module ="com.vanniktech:gradle-maven-publish-plugin", version.ref = "mavenPublish" }
-coroutines-test = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-test", version.ref = "coroutines" }
 coroutines-core = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-core", version.ref = "coroutines" }
+coroutines-test = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-test", version.ref = "coroutines" }
 atomicfu = { module = "org.jetbrains.kotlinx:atomicfu", version.ref = "atomicfu" }
+stately-isoCollections = { module = "co.touchlab:stately-iso-collections", version.ref = "statelyIso" }