From 69a7bd72c66ab821f753a2419dd7ec341aeacf33 Mon Sep 17 00:00:00 2001 From: Paul Hitt Date: Fri, 9 Oct 2026 13:15:45 -0400 Subject: [PATCH 1/2] Two writes of one vault record no longer kill the app Found on the emulator, typing into a new conversation: the app died with "IllegalStateException: could not commit the vault record" from Vault.saveDraft on a worker thread. The draft is saved from a coroutine per keystroke on the IO pool; two writes of one record staged into the same `.tmp`, the first rename took it, and the second had nothing to rename and threw, uncaught. Main has the same code. Each write now stages under its own name, and draft saves run one at a time and in order, so an older draft cannot land after a newer one or after the send that cleared it. TwoWritesOfOneRecordDoNotCrashTest (400 writes from 8 threads): red with the shared staging name, green now. Co-Authored-By: Claude Opus 5.5 --- .../flamenet/messenger/app/KeystoreVault.kt | 10 +++-- .../messenger/app/MessengerViewModel.kt | 8 +++- .../app/TwoWritesOfOneRecordDoNotCrashTest.kt | 45 +++++++++++++++++++ 3 files changed, 59 insertions(+), 4 deletions(-) create mode 100644 app/src/test/kotlin/io/flamenet/messenger/app/TwoWritesOfOneRecordDoNotCrashTest.kt diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/KeystoreVault.kt b/app/src/main/kotlin/io/flamenet/messenger/app/KeystoreVault.kt index c1f045c..8fabbab 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/KeystoreVault.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/KeystoreVault.kt @@ -64,7 +64,7 @@ object KeystoreVault { /** * Records as files under `filesDir`, which is app-private storage. */ - private class FileStorage( + internal class FileStorage( private val root: File, ) : Vault.Storage { init { @@ -87,9 +87,13 @@ object KeystoreVault { bytes: ByteArray, ) { // Write beside and rename, so a kill mid-write leaves the previous - // record intact rather than a truncated one. + // record intact rather than a truncated one. Each write stages + // under its own name: two writes of one record (a draft saved per + // keystroke) shared one `.tmp`, the first rename took it, and the + // second threw on a worker thread, which killed the app. The last + // rename wins, as the last write should. val target = fileFor(name) - val staging = File(root, target.name + ".tmp") + val staging = File(root, target.name + "." + java.util.UUID.randomUUID() + ".tmp") staging.writeBytes(bytes) if (!staging.renameTo(target)) { staging.delete() diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt index ec33c22..99d37f7 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt @@ -4053,9 +4053,15 @@ class MessengerViewModel( fun rememberDraft(text: String) { val chat = signedIn()?.openChat ?: return val vault = vault ?: return - viewModelScope.launch(Dispatchers.IO) { vault.saveDraft(chat.vaultThreadId(), text) } + // One at a time and in order: a save per keystroke ran in parallel on + // the IO pool, so an older draft could land after the newer one, or + // after the send that cleared it. + viewModelScope.launch(draftWrites) { runCatching { vault.saveDraft(chat.vaultThreadId(), text) } } } + @OptIn(kotlinx.coroutines.ExperimentalCoroutinesApi::class) + private val draftWrites = Dispatchers.IO.limitedParallelism(1) + /** * Messages whose words match, across every conversation on this device. */ diff --git a/app/src/test/kotlin/io/flamenet/messenger/app/TwoWritesOfOneRecordDoNotCrashTest.kt b/app/src/test/kotlin/io/flamenet/messenger/app/TwoWritesOfOneRecordDoNotCrashTest.kt new file mode 100644 index 0000000..9daea4e --- /dev/null +++ b/app/src/test/kotlin/io/flamenet/messenger/app/TwoWritesOfOneRecordDoNotCrashTest.kt @@ -0,0 +1,45 @@ +package io.flamenet.messenger.app + +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import java.io.File +import java.util.concurrent.CountDownLatch +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * Found on the emulator, 2026-10-09: typing into the composer saves the draft + * from a coroutine per keystroke, and two of them wrote the same record at + * once. Both staged into one `.tmp`, the first rename took it, the + * second found nothing to rename and threw "could not commit the vault + * record" on a worker thread, which killed the app. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34]) +class TwoWritesOfOneRecordDoNotCrashTest { + @Test + fun `concurrent writes of one record all commit`() { + val root = File.createTempFile("vault", "").also { it.delete(); it.mkdirs() } + val storage = KeystoreVault.FileStorage(root) + val pool = Executors.newFixedThreadPool(8) + val start = CountDownLatch(1) + val failures = java.util.concurrent.ConcurrentLinkedQueue() + repeat(400) { i -> + pool.execute { + start.await() + runCatching { storage.write("draft", "text $i".toByteArray()) }.onFailure { failures += it } + } + } + start.countDown() + pool.shutdown() + assertTrue(pool.awaitTermination(30, TimeUnit.SECONDS)) + assertEquals(emptyList(), failures.map { it.message }) + assertTrue(String(storage.read("draft")!!).startsWith("text ")) + assertTrue(root.listFiles()!!.none { it.name.endsWith(".tmp") }, "a staging file was left behind") + root.deleteRecursively() + } +} From 8842479bb8deb3d5a8bf003bde37a929df66fa39 Mon Sep 17 00:00:00 2001 From: Paul Hitt Date: Fri, 9 Oct 2026 13:42:42 -0400 Subject: [PATCH 2/2] Another account on the same install starts with an empty vault Found on the emulator against a local relay: after a plain sign-out of luke1009 and a sign-up as mira1009 on the same install, mira saw luke's conversation with lina1009, carried andy1009's handle certificate from two accounts earlier, and sent her first message to lina on luke's ratchet session. The iPhone could not open it ("no session for envelope 95 from a26ac1d7") and mira's first contact never arrived. Main has the same sign-in path: a different account got a new device identity (android#22) and kept every other record. AccountSwitch.forgetPreviousAccountIfSwitching empties the vault when a different account (another user id or relay) signs in, before its device is made. The same account signing back in keeps everything, as before. The last account's device identity was already being replaced at that point, so nothing forgotten was still usable by it. AnotherAccountStartsWithNothingTest: 2 red, then 4/4 green. Co-Authored-By: Claude Opus 5.5 --- .../flamenet/messenger/app/AccountSwitch.kt | 28 +++++++ .../messenger/app/MessengerViewModel.kt | 1 + .../AnotherAccountStartsWithNothingTest.kt | 78 +++++++++++++++++++ 3 files changed, 107 insertions(+) create mode 100644 app/src/main/kotlin/io/flamenet/messenger/app/AccountSwitch.kt create mode 100644 app/src/test/kotlin/io/flamenet/messenger/app/AnotherAccountStartsWithNothingTest.kt diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/AccountSwitch.kt b/app/src/main/kotlin/io/flamenet/messenger/app/AccountSwitch.kt new file mode 100644 index 0000000..913192f --- /dev/null +++ b/app/src/main/kotlin/io/flamenet/messenger/app/AccountSwitch.kt @@ -0,0 +1,28 @@ +package io.flamenet.messenger.app + +import io.flamenet.e2e.Vault + +/** What happens to the vault when somebody signs in. */ +object AccountSwitch { + /** + * Before a different account takes over this install, forget the last + * one's records: its conversations, sessions, delivery keys, drafts and + * handle certificate. A plain sign-out keeps the vault so the SAME person + * can sign back in; another account on the same install got a new device + * but inherited the rest, saw the last account's conversations and sent + * on its ratchet sessions, which the far end could not open. The last + * account's device was already replaced at this point, so what is + * forgotten could not have been used by it again. + * + * True when something was forgotten. + */ + fun forgetPreviousAccountIfSwitching( + vault: Vault, + previous: Vault.Credential?, + sameAccount: Boolean, + ): Boolean { + if (previous == null || sameAccount) return false + vault.forgetEverything() + return true + } +} diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt index 99d37f7..29d10ee 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt @@ -620,6 +620,7 @@ class MessengerViewModel( (previous.userId == account.userId && previous.relayBaseUrl == config.baseUrl) val storedIdentity = if (sameAccount) vault.loadIdentity() else null if (!sameAccount) identity = null + AccountSwitch.forgetPreviousAccountIfSwitching(vault, previous, sameAccount) val device = identity diff --git a/app/src/test/kotlin/io/flamenet/messenger/app/AnotherAccountStartsWithNothingTest.kt b/app/src/test/kotlin/io/flamenet/messenger/app/AnotherAccountStartsWithNothingTest.kt new file mode 100644 index 0000000..657f410 --- /dev/null +++ b/app/src/test/kotlin/io/flamenet/messenger/app/AnotherAccountStartsWithNothingTest.kt @@ -0,0 +1,78 @@ +package io.flamenet.messenger.app + +import io.flamenet.e2e.Vault +import java.io.File +import javax.crypto.KeyGenerator +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue + +/** + * Found on the emulator against a local relay, 2026-10-09: a plain sign-out + * keeps the vault so the same person can sign back in, and a DIFFERENT + * account signing in on the same install got a new device identity but kept + * everything else. mira1009 saw luke1009's conversation with lina1009 and + * sent her first message on luke's ratchet session, which the iPhone could not open ("no session for + * envelope 95"). A different account now starts with an empty vault. + */ +class AnotherAccountStartsWithNothingTest { + private class MapStorage : Vault.Storage { + val records = mutableMapOf() + + override fun read(name: String) = records[name] + + override fun write( + name: String, + bytes: ByteArray, + ) { + records[name] = bytes + } + + override fun delete(name: String) { + records.remove(name) + } + + override fun names() = records.keys.toList() + } + + private fun vault() = Vault(MapStorage(), KeyGenerator.getInstance("AES").apply { init(256) }.generateKey()) + + private fun credential(user: Long) = Vault.Credential("http://127.0.0.1:18810", "u$user", "pw", user) + + @Test + fun `a different account finds none of the last one's records`() { + val v = vault() + v.saveCredential(credential(3)) + v.saveGroupName("g1", "luke's group") + v.saveDraft("user:4", "for lina") + assertTrue(AccountSwitch.forgetPreviousAccountIfSwitching(v, credential(3), sameAccount = false)) + assertNull(v.groupName("g1")) + assertEquals(null, v.draft("user:4")) + assertNull(runCatching { v.loadCredential() }.getOrNull()) + } + + @Test + fun `the same account keeps everything`() { + val v = vault() + v.saveCredential(credential(3)) + v.saveGroupName("g1", "luke's group") + assertFalse(AccountSwitch.forgetPreviousAccountIfSwitching(v, credential(3), sameAccount = true)) + assertNotNull(v.groupName("g1")) + } + + @Test + fun `a first sign-in has nothing to forget`() { + assertFalse(AccountSwitch.forgetPreviousAccountIfSwitching(vault(), null, sameAccount = true)) + } + + @Test + fun `sign-in forgets before the new identity is saved`() { + val vm = File("src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt").readText() + val forget = vm.indexOf("AccountSwitch.forgetPreviousAccountIfSwitching(") + val newIdentity = vm.indexOf("DeviceIdentity.create().also {") + assertTrue(forget in 0 until newIdentity, "sign-in does not forget the last account before making this one's device") + } +}