From e947a10a5d381a3865d73e50aeb488534476ede4 Mon Sep 17 00:00:00 2001
From: Martin Strambach <martin.strambach@gmail.com>
Date: Mon, 1 Jun 2026 14:57:39 +0200
Subject: [PATCH 1/3] Fix RealMutableStore write-queue data race (inverted lock
 polarity)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The per-key write-request queue is a non-thread-safe ArrayDeque, but
withWriteRequestQueueLock guarded it with a Lightswitch — a shared/reader
lock that lets multiple holders run concurrently. So addWriteRequestToQueue
(add) and updateWriteRequestQueue (iterate + rebuild) could run on the same
deque at once. A structural add during iteration corrupts the backing array:
ConcurrentModificationException on the JVM, EXC_BAD_ACCESS on Kotlin/Native.
The only exclusive (mutex) holder was a pure read (getLatestWriteRequest) —
the polarity was inverted.

Guard all write-queue access with the per-key exclusive mutex instead, so
add/iterate/rebuild mutually exclude each other and getLatestWriteRequest.

Adds MutableStoreConcurrencyTest reproducing the race (red pre-fix on both
JVM and native, green after). Lightswitch is now unused in RealMutableStore;
left in place to keep the diff focused (can be removed as a follow-up).

Signed-off-by: Martin Strambach <martin.strambach@gmail.com>
---
 .../store/store5/impl/RealMutableStore.kt     |  13 +-
 .../MutableStoreConcurrencyTest.kt            | 127 ++++++++++++++++++
 2 files changed, 134 insertions(+), 6 deletions(-)
 create mode 100644 store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/MutableStoreConcurrencyTest.kt

diff --git a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealMutableStore.kt b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealMutableStore.kt
index 6e74462c..40982613 100644
--- a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealMutableStore.kt
+++ b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealMutableStore.kt
@@ -222,13 +222,14 @@ internal class RealMutableStore<Key : Any, Network : Any, Output : Any, Local :
         // Acquire the ThreadSafety object for this key without holding storeLock.
         val threadSafety = getThreadSafety(key)
 
-        // Now safely lock the queue's own mutex.
-        threadSafety.writeRequests.lightswitch.lock(threadSafety.writeRequests.mutex)
-        return try {
-            val queue = getQueue((key))
+        // Exclusively lock the queue's own mutex. The block both reads and structurally mutates the
+        // per-key ArrayDeque (add / iterate-and-rebuild), so callers must mutually exclude each other.
+        // A shared lock here (e.g. Lightswitch) would allow a concurrent add() during iteration,
+        // corrupting the deque's backing array — a ConcurrentModificationException on the JVM and an
+        // EXC_BAD_ACCESS on Kotlin/Native.
+        return threadSafety.writeRequests.mutex.withLock {
+            val queue = getQueue(key)
             queue.block()
-        } finally {
-            threadSafety.writeRequests.lightswitch.unlock(threadSafety.writeRequests.mutex)
         }
     }
 
diff --git a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/MutableStoreConcurrencyTest.kt b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/MutableStoreConcurrencyTest.kt
new file mode 100644
index 00000000..97c3912a
--- /dev/null
+++ b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/MutableStoreConcurrencyTest.kt
@@ -0,0 +1,127 @@
+@file:OptIn(ExperimentalCoroutinesApi::class, ExperimentalStoreApi::class)
+
+package org.mobilenativefoundation.store.store5.mutablestore
+
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.async
+import kotlinx.coroutines.awaitAll
+import kotlinx.coroutines.coroutineScope
+import kotlinx.coroutines.test.runTest
+import org.mobilenativefoundation.store.cache5.CacheBuilder
+import org.mobilenativefoundation.store.core5.ExperimentalStoreApi
+import org.mobilenativefoundation.store.store5.StoreWriteRequest
+import org.mobilenativefoundation.store.store5.StoreWriteResponse
+import org.mobilenativefoundation.store.store5.Updater
+import org.mobilenativefoundation.store.store5.UpdaterResult
+import org.mobilenativefoundation.store.store5.impl.RealMutableStore
+import org.mobilenativefoundation.store.store5.impl.RealStore
+import org.mobilenativefoundation.store.store5.mutablestore.util.TestConverter
+import org.mobilenativefoundation.store.store5.mutablestore.util.TestFetcher
+import org.mobilenativefoundation.store.store5.mutablestore.util.TestLogger
+import org.mobilenativefoundation.store.store5.mutablestore.util.TestValidator
+import org.mobilenativefoundation.store.store5.mutablestore.util.testStore
+import kotlin.test.Test
+import kotlin.test.assertTrue
+
+/**
+ * Regression test for a data race in [RealMutableStore]'s per-key write-request queue.
+ *
+ * The queue is a non-thread-safe `ArrayDeque`. Mutating access goes through
+ * `withWriteRequestQueueLock`, which guards it with a `Lightswitch` (a shared/reader lock that lets
+ * multiple holders run concurrently). As a result two operations on the same key can run at once:
+ * `addWriteRequestToQueue` doing `add(...)` while `updateWriteRequestQueue` iterates the same deque
+ * (`for (writeRequest in this)`). A structural `add` during iteration corrupts the backing array.
+ *
+ * On Kotlin/Native this surfaces as `EXC_BAD_ACCESS` (a hard process crash). On the JVM the deque's
+ * fail-fast iterator throws `ConcurrentModificationException`, which `RealMutableStore` catches and
+ * converts into a [StoreWriteResponse.Error.Exception]. Either way, with correct mutual exclusion
+ * every write should succeed.
+ *
+ * The delegate is backed by a real thread-safe cache (cache5) with no source of truth, so the only
+ * unsynchronized shared mutable state exercised here is the write-request queue itself.
+ */
+@OptIn(ExperimentalCoroutinesApi::class, ExperimentalStoreApi::class)
+class MutableStoreConcurrencyTest {
+
+    private fun newMutableStore(): RealMutableStore<String, Int, Int, Int> {
+        val delegate: RealStore<String, Int, Int, Int> =
+            testStore(
+                fetcher = TestFetcher(),
+                sourceOfTruth = null,
+                converter = TestConverter(),
+                validator = TestValidator(),
+                memoryCache = CacheBuilder<String, Int>().build(),
+            )
+        return RealMutableStore(
+            delegate = delegate,
+            updater = Updater.by<String, Int, Int>({ _, value -> UpdaterResult.Success.Typed(value) }),
+            bookkeeper = null,
+            logger = TestLogger(),
+        )
+    }
+
+    @Test
+    fun sequentialWritesToSameKey_allSucceed() =
+        runTest {
+            val mutableStore = newMutableStore()
+            val key = "key"
+            val responses = (1..500).map { i -> mutableStore.write<Int>(StoreWriteRequest.of(key = key, value = i)) }
+            val failures = responses.filterIsInstance<StoreWriteResponse.Error.Exception>()
+            assertTrue(
+                failures.isEmpty(),
+                "Baseline sequential writes should all succeed, but ${failures.size} failed" +
+                    (failures.firstOrNull()?.let { ", first error = ${it.error}" } ?: ""),
+            )
+        }
+
+    @Test
+    fun concurrentWritesToSameKey_doNotCorruptWriteQueue() =
+        runTest {
+            val mutableStore = newMutableStore()
+            val key = "key"
+            val concurrentWriters = 64
+            val rounds = 50
+
+            repeat(rounds) { round ->
+                val responses =
+                    coroutineScope {
+                        (1..concurrentWriters)
+                            .map { i ->
+                                async(Dispatchers.Default) {
+                                    mutableStore.write<Int>(
+                                        StoreWriteRequest.of(key = key, value = round * concurrentWriters + i),
+                                    )
+                                }
+                            }
+                            .awaitAll()
+                    }
+
+                // A corrupted ArrayDeque surfaces as a memory-safety symptom: ConcurrentModificationException,
+                // NullPointerException, or IndexOutOfBoundsException on the JVM (EXC_BAD_ACCESS aborts the
+                // process on Native, so reaching this assertion at all already proves no native crash).
+                // NOTE: concurrent writes to the SAME key can still legitimately fail with
+                // IllegalArgumentException("No writes found ...") — a separate, pre-existing logical race
+                // where one write drains another's queue entry. That is not memory corruption and is out of
+                // scope for this fix, so it is tolerated here.
+                val corruption =
+                    responses
+                        .filterIsInstance<StoreWriteResponse.Error.Exception>()
+                        .filter { response ->
+                            when (response.error) {
+                                is ConcurrentModificationException,
+                                is NullPointerException,
+                                is IndexOutOfBoundsException,
+                                -> true
+                                else -> false
+                            }
+                        }
+                assertTrue(
+                    corruption.isEmpty(),
+                    "Write-queue memory corruption in round $round: ${corruption.size}/${responses.size} " +
+                        "writes hit a corruption-class error" +
+                        (corruption.firstOrNull()?.let { ", first = ${it.error}" } ?: ""),
+                )
+            }
+        }
+}

From d75ed05865e8384e223b0a535f4ee4c270203018 Mon Sep 17 00:00:00 2001
From: Martin Strambach <martin.strambach@gmail.com>
Date: Mon, 1 Jun 2026 15:09:30 +0200
Subject: [PATCH 2/3] Remove now-dead Lightswitch

After the previous commit nothing acquires the per-key Lightswitch:
withWriteRequestQueueLock uses writeRequests.mutex directly, and
readCompletions already used readCompletions.mutex. The lightswitch field
on StoreThreadSafety was therefore read by nothing.

Drop the field and delete the class. All three types (Lightswitch,
StoreThreadSafety, ThreadSafety) are internal and absent from the
binary-compatibility-validator dumps, so this changes no public API/ABI:
jvmApiCheck and klibApiCheck both pass unchanged.

Signed-off-by: Martin Strambach <martin.strambach@gmail.com>
---
 .../store/store5/impl/RealMutableStore.kt     |  6 ++--
 .../store5/internal/concurrent/Lightswitch.kt | 33 -------------------
 .../internal/concurrent/ThreadSafety.kt       |  1 -
 .../MutableStoreConcurrencyTest.kt            |  4 +--
 4 files changed, 5 insertions(+), 39 deletions(-)
 delete mode 100644 store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/internal/concurrent/Lightswitch.kt

diff --git a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealMutableStore.kt b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealMutableStore.kt
index 40982613..83f7af84 100644
--- a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealMutableStore.kt
+++ b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealMutableStore.kt
@@ -224,9 +224,9 @@ internal class RealMutableStore<Key : Any, Network : Any, Output : Any, Local :
 
         // Exclusively lock the queue's own mutex. The block both reads and structurally mutates the
         // per-key ArrayDeque (add / iterate-and-rebuild), so callers must mutually exclude each other.
-        // A shared lock here (e.g. Lightswitch) would allow a concurrent add() during iteration,
-        // corrupting the deque's backing array — a ConcurrentModificationException on the JVM and an
-        // EXC_BAD_ACCESS on Kotlin/Native.
+        // A shared/reader lock here would allow a concurrent add() during iteration, corrupting the
+        // deque's backing array — a ConcurrentModificationException on the JVM and an EXC_BAD_ACCESS
+        // on Kotlin/Native.
         return threadSafety.writeRequests.mutex.withLock {
             val queue = getQueue(key)
             queue.block()
diff --git a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/internal/concurrent/Lightswitch.kt b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/internal/concurrent/Lightswitch.kt
deleted file mode 100644
index 8f772fca..00000000
--- a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/internal/concurrent/Lightswitch.kt
+++ /dev/null
@@ -1,33 +0,0 @@
-package org.mobilenativefoundation.store.store5.internal.concurrent
-
-import kotlinx.coroutines.sync.Mutex
-import kotlinx.coroutines.sync.withLock
-
-/**
- * Locks when first reader starts and unlocks when last reader finishes.
- * Lightswitch analogy: First one into a room turns on the light (locks the mutex), and the last one out turns off the light (unlocks the mutex).
- * @property counter Number of readers
- */
-internal class Lightswitch {
-    private var counter = 0
-    private val mutex = Mutex()
-
-    suspend fun lock(room: Mutex) {
-        mutex.withLock {
-            counter += 1
-            if (counter == 1) {
-                room.lock()
-            }
-        }
-    }
-
-    suspend fun unlock(room: Mutex) {
-        mutex.withLock {
-            counter -= 1
-            check(counter >= 0)
-            if (counter == 0) {
-                room.unlock()
-            }
-        }
-    }
-}
diff --git a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/internal/concurrent/ThreadSafety.kt b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/internal/concurrent/ThreadSafety.kt
index aaae4951..28e04a70 100644
--- a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/internal/concurrent/ThreadSafety.kt
+++ b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/internal/concurrent/ThreadSafety.kt
@@ -9,5 +9,4 @@ internal data class ThreadSafety(
 
 internal data class StoreThreadSafety(
     val mutex: Mutex = Mutex(),
-    val lightswitch: Lightswitch = Lightswitch(),
 )
diff --git a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/MutableStoreConcurrencyTest.kt b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/MutableStoreConcurrencyTest.kt
index 97c3912a..61cdffb8 100644
--- a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/MutableStoreConcurrencyTest.kt
+++ b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/MutableStoreConcurrencyTest.kt
@@ -28,8 +28,8 @@ import kotlin.test.assertTrue
  * Regression test for a data race in [RealMutableStore]'s per-key write-request queue.
  *
  * The queue is a non-thread-safe `ArrayDeque`. Mutating access goes through
- * `withWriteRequestQueueLock`, which guards it with a `Lightswitch` (a shared/reader lock that lets
- * multiple holders run concurrently). As a result two operations on the same key can run at once:
+ * `withWriteRequestQueueLock`, which historically guarded it with a shared/reader lock that lets
+ * multiple holders run concurrently. As a result two operations on the same key could run at once:
  * `addWriteRequestToQueue` doing `add(...)` while `updateWriteRequestQueue` iterates the same deque
  * (`for (writeRequest in this)`). A structural `add` during iteration corrupts the backing array.
  *

From 4e6578694307d2fc8a4a61a19840bd6cf32d51bb Mon Sep 17 00:00:00 2001
From: Martin Strambach <martin.strambach@gmail.com>
Date: Sun, 7 Jun 2026 22:05:52 +0200
Subject: [PATCH 3/3] Apply ktlintFormat

Signed-off-by: Martin Strambach <martin.strambach@gmail.com>
---
 .../store/store5/mutablestore/MutableStoreConcurrencyTest.kt     | 1 -
 1 file changed, 1 deletion(-)

diff --git a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/MutableStoreConcurrencyTest.kt b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/MutableStoreConcurrencyTest.kt
index 61cdffb8..2a2b643c 100644
--- a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/MutableStoreConcurrencyTest.kt
+++ b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/mutablestore/MutableStoreConcurrencyTest.kt
@@ -43,7 +43,6 @@ import kotlin.test.assertTrue
  */
 @OptIn(ExperimentalCoroutinesApi::class, ExperimentalStoreApi::class)
 class MutableStoreConcurrencyTest {
-
     private fun newMutableStore(): RealMutableStore<String, Int, Int, Int> {
         val delegate: RealStore<String, Int, Int, Int> =
             testStore(