Forward-merge: release/v1.1.3 → main

CHANGELOG.md

@@ -2,6 +2,11 @@
 
 All notable changes to this project will be documented in this file.
 
+## [1.1.3] - 2025-08-24
+
+### Fixed
+- **Serialized `.apply()` and `.commit()` writes**: Previously, rapid sequences of `.apply()` and `.commit()` could interleave and cause `AEADBadTagException` or a commit deadlock. SafeBox now enforces strict write sequencing, ensuring only one disk write is active at a time. ([#60](https://github.com/harrytmthy/safebox/issues/60))
+
 ## [1.1.2] - 2025-08-19
 
 ### Fixed

README.md

@@ -53,7 +53,7 @@ Compared to EncryptedSharedPreferences:
 
 ```kotlin
 dependencies {
-    implementation("io.github.harrytmthy:safebox:1.1.2")
+    implementation("io.github.harrytmthy:safebox:1.1.3")
 }
 ```
 

safebox/build.gradle.kts

@@ -23,7 +23,7 @@ plugins {
 }
 
 group = "io.github.harrytmthy"
-version = "1.1.2"
+version = "1.1.3"
 
 android {
     namespace = "com.harrytmthy.safebox"

safebox/src/androidTest/java/com/harrytmthy/safebox/SafeBoxTest.kt

@@ -25,13 +25,16 @@ import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.test.UnconfinedTestDispatcher
+import kotlinx.coroutines.test.runTest
+import kotlinx.coroutines.withTimeout
 import org.junit.After
 import org.junit.runner.RunWith
 import kotlin.test.Test
 import kotlin.test.assertContentEquals
 import kotlin.test.assertEquals
 import kotlin.test.assertNull
 import kotlin.test.assertTrue
+import kotlin.time.Duration.Companion.seconds
 
 @OptIn(ExperimentalCoroutinesApi::class)
 @RunWith(AndroidJUnit4::class)
@@ -162,6 +165,27 @@ class SafeBoxTest {
         assertContentEquals(expectedValueChanges, changedValues)
     }
 
+    @Test
+    fun apply_then_commit_shouldHaveCorrectOrder() = runTest {
+        safeBox = createSafeBox(ioDispatcher = Dispatchers.IO)
+
+        withTimeout(10.seconds) {
+            safeBox.edit().putInt("0", 0).apply()
+            safeBox.edit().putInt("1", 1).apply()
+            assertTrue(safeBox.edit().clear().commit())
+            safeBox.edit().putInt("2", 2).apply()
+            safeBox.edit().putInt("3", 3).apply()
+            assertTrue(safeBox.edit().clear().commit())
+            safeBox.edit().putInt("4", 4).apply()
+        }
+
+        assertEquals(4, safeBox.getInt("4", -1))
+        assertEquals(-1, safeBox.getInt("3", -1))
+        assertEquals(-1, safeBox.getInt("2", -1))
+        assertEquals(-1, safeBox.getInt("1", -1))
+        assertEquals(-1, safeBox.getInt("0", -1))
+    }
+
     private fun createSafeBox(
         ioDispatcher: CoroutineDispatcher = UnconfinedTestDispatcher(),
         stateListener: SafeBoxStateListener? = null,

safebox/src/main/java/com/harrytmthy/safebox/SafeBox.kt

@@ -40,7 +40,6 @@ import com.harrytmthy.safebox.strategy.ValueFallbackStrategy
 import com.harrytmthy.safebox.strategy.ValueFallbackStrategy.WARN
 import kotlinx.coroutines.CompletableDeferred
 import kotlinx.coroutines.CoroutineDispatcher
-import kotlinx.coroutines.CoroutineExceptionHandler
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
@@ -85,38 +84,34 @@ public class SafeBox private constructor(
 
     private val listeners = CopyOnWriteArrayList<OnSharedPreferenceChangeListener>()
 
-    private val commitMutex = Mutex()
+    private val writeMutex = Mutex()
 
-    private val applyMutex = Mutex()
-
-    @Volatile
-    private var applyCompleted = CompletableDeferred<Unit>().apply { complete(Unit) }
+    private val writeBarrier = AtomicReference(CompletableDeferred<Unit>().apply { complete(Unit) })
 
     private val delegate = object : Delegate {
 
         private val updateLock = Any()
 
-        private val exceptionHandler = CoroutineExceptionHandler { _, throwable ->
-            Log.e("SafeBox", "Failed to apply changes.", throwable)
-            applyCompleted.complete(Unit)
-        }
-
         override fun commit(entries: LinkedHashMap<String, Action>, cleared: Boolean): Boolean {
             val entriesToWrite = LinkedHashMap(entries)
             synchronized(updateLock) {
                 entries.clear() // Prevents stale mutations on reused editor instance
                 updateEntries(entriesToWrite, cleared)
             }
+            val currentWriteBarrier = CompletableDeferred<Unit>()
+            val previousWriteBarrier = writeBarrier.getAndSet(currentWriteBarrier)
             return stateManager.launchCommitWithWritingState {
                 try {
-                    applyCompleted.await()
-                    commitMutex.withLock {
+                    previousWriteBarrier.await()
+                    writeMutex.withLock {
                         applyChanges(entriesToWrite, cleared)
                     }
                     true
                 } catch (e: Exception) {
                     Log.e("SafeBox", "Failed to commit changes.", e)
                     false
+                } finally {
+                    currentWriteBarrier.complete(Unit)
                 }
             }
         }
@@ -127,12 +122,19 @@ public class SafeBox private constructor(
                 entries.clear() // Prevents stale mutations on reused editor instance
                 updateEntries(entriesToWrite, cleared)
             }
-            stateManager.launchApplyWithWritingState(exceptionHandler) {
-                applyCompleted = CompletableDeferred()
-                applyMutex.withLock {
-                    applyChanges(entriesToWrite, cleared)
+            val currentWriteBarrier = CompletableDeferred<Unit>()
+            val previousWriteBarrier = writeBarrier.getAndSet(currentWriteBarrier)
+            stateManager.launchApplyWithWritingState {
+                try {
+                    previousWriteBarrier.await()
+                    writeMutex.withLock {
+                        applyChanges(entriesToWrite, cleared)
+                    }
+                } catch (e: Exception) {
+                    Log.e("SafeBox", "Failed to commit changes.", e)
+                } finally {
+                    currentWriteBarrier.complete(Unit)
                 }
-                applyCompleted.complete(Unit)
             }
         }
 

safebox/src/main/java/com/harrytmthy/safebox/state/SafeBoxStateManager.kt

@@ -23,11 +23,9 @@ import com.harrytmthy.safebox.state.SafeBoxState.STARTING
 import com.harrytmthy.safebox.state.SafeBoxState.WRITING
 import kotlinx.coroutines.CompletableDeferred
 import kotlinx.coroutines.CoroutineDispatcher
-import kotlinx.coroutines.CoroutineExceptionHandler
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
 import java.util.concurrent.atomic.AtomicInteger
-import java.util.concurrent.atomic.AtomicReference
 
 /**
  * Manages the lifecycle state of a [SafeBox] instance and coordinates concurrent read/write access.
@@ -54,67 +52,51 @@ internal class SafeBoxStateManager(
 
     private val initialReadCompleted = CompletableDeferred<Unit>()
 
-    private val writeCompleted = AtomicReference<CompletableDeferred<Unit>>()
-
     fun setStateListener(stateListener: SafeBoxStateListener?) {
         this.stateListener = stateListener
     }
 
     inline fun launchWithStartingState(crossinline block: suspend () -> Unit) {
         updateState(STARTING)
         safeBoxScope.launch(ioDispatcher) {
-            block()
-            if (concurrentWriteCount.get() == 0) {
-                updateState(IDLE)
-            } else {
-                updateState(WRITING)
+            try {
+                block()
+            } finally {
+                initialReadCompleted.complete(Unit)
+                if (concurrentWriteCount.get() == 0) {
+                    updateState(IDLE)
+                }
             }
-            initialReadCompleted.complete(Unit)
         }
     }
 
-    inline fun launchCommitWithWritingState(crossinline block: suspend () -> Boolean): Boolean {
-        if (concurrentWriteCount.incrementAndGet() == 1) {
-            writeCompleted.set(CompletableDeferred())
-            if (initialReadCompleted.isCompleted) {
-                updateState(WRITING)
-            }
+    inline fun launchCommitWithWritingState(crossinline block: suspend () -> Boolean): Boolean =
+        runBlocking {
+            withStateTransition(block)
         }
-        return runBlocking {
-            initialReadCompleted.await()
-            val result = block()
-            finalizeWriting()
-            result
+
+    inline fun launchApplyWithWritingState(crossinline block: suspend () -> Unit) {
+        safeBoxScope.launch(ioDispatcher) {
+            withStateTransition(block)
         }
     }
 
-    inline fun launchApplyWithWritingState(
-        exceptionHandler: CoroutineExceptionHandler,
-        crossinline block: suspend () -> Unit,
-    ) {
+    private suspend inline fun <T> withStateTransition(crossinline block: suspend () -> T): T {
+        initialReadCompleted.await()
         if (concurrentWriteCount.incrementAndGet() == 1) {
-            writeCompleted.set(CompletableDeferred())
-            if (initialReadCompleted.isCompleted) {
-                updateState(WRITING)
-            }
+            updateState(WRITING)
         }
-        safeBoxScope.launch(ioDispatcher + exceptionHandler) {
-            initialReadCompleted.await()
+        return try {
             block()
-        }.invokeOnCompletion {
-            finalizeWriting()
+        } finally {
+            if (concurrentWriteCount.decrementAndGet() == 0) {
+                updateState(IDLE)
+            }
         }
     }
 
     private fun updateState(newState: SafeBoxState) {
         stateListener?.onStateChanged(newState)
         SafeBoxGlobalStateObserver.updateState(fileName, newState)
     }
-
-    private fun finalizeWriting() {
-        if (concurrentWriteCount.decrementAndGet() == 0) {
-            updateState(IDLE)
-            writeCompleted.get()?.complete(Unit)
-        }
-    }
 }