Repository navigation
fix(database): share one SQLite writer per owner and take write locks up front - #759
Merged
Merged
Conversation
…ctions `replaceConversationFeed` reads the doomed-conversation set before it deletes, and `persistMessages` reads the catch-up cursor before it advances it. Both ran in a DEFERRED transaction, which takes a read lock first and upgrades on the first write. SQLite does not consult the busy handler on that upgrade — it returns SQLITE_BUSY immediately when another connection holds the write lock, so the 2-second busy timeout never applies to exactly the case it was armed for. BEGIN IMMEDIATE takes the write lock at the start, where the busy handler does apply. `Database.transaction(silent:_:)` gets the same treatment: every caller that goes through it reads and writes inside the block.
…de3) `completeLogin` builds a fresh `SessionContainer` every time it runs, and each one opened its own `Database` on the same file. The previous session's controllers keep their `Database` alive until they are released, so a launch that logs in repeatedly ends up with several writer connections competing for one SQLite write lock. The crash report behind this had five `completeLogin` calls in a single cold launch. `DatabaseStore` owns the open path — store location, App Group migration, schema rebuild — and caches the result per owner, so the second login gets the connection the first one opened. The open path moves out of `SessionAuthenticator` unchanged; only the caching around it is new.
…iter (6a4fde3) Opens a second `Database` on the same file, holds BEGIN IMMEDIATE for 200ms, and runs `replaceConversationFeed` against it. On a DEFERRED transaction the snapshot upgrade fails at once; with BEGIN IMMEDIATE the busy handler waits out the rival and the feed commits.
…e count The five `completeLogin` calls behind 6a4fde3 were only visible because a debug build was attached. At `.info` the count reaches a release crash report, which is what distinguishes "logged in twice" from the runaway loop this was.
The brief and plan were written against the pre-refactor tree. A closing note says which paths moved under the App Group / FlipcashStore work so the file references read as of their triage date rather than as current.
bmc08gt
added a commit
that referenced
this pull request
Sep 11, 2026
…discrete-curve * origin/main: (27 commits) fix(database): share one SQLite writer per owner and take write locks up front (#759) feat(chat): declare the payment action on tip DM payments (#752) refactor(chat): drop the deprecated new_messages overlay (#757) feat(notifications): write prefetched messages into the shared store (#756) refactor(store): move the persistence layer into a shared FlipcashStore package (#755) feat(database): move the SQLite store into the App Group container (#754) feat(database): open the store on demand, close it on background (#753) feat(nse): extension crash reporting, a WAL checkpoint, and on-device push hooks (#751) feat(home): long-press the You tab to open the account switcher (#749) fix(tests): reset Photos access before the previous app instance lingers (#746) chore: bump version to 2026.9.2 (#745) revert: back out the Coinbase Stable Swapper authority migration (#747) (#750) fix(swap): follow the Coinbase Stable Swapper authority migration (#747) fix(tests): cancel a cash link through the details screen (#744) fix(chat): make the whole Send Cash pill tappable while it stands alone (#743) fix(username): drop a leading @ in the validator (#742) fix(chat): scope the send-button spring to the button (#741) fix(transactions): tighten the details card stack and drop the header badge (#740) fix(transactions): draw View in Chat as a card, not the primary action (#739) feat(chat): flash the message a reply-quote jump lands on (#738) ... # Conflicts: # Code.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved # FlipcashCore/Package.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bugsnag 6a4fde3, "Failed to persist conversation state [replace-feed]" with
database is locked (code: 5). The event's log stream shows one cold launch runningcompleteLoginfive times for the same owner. Each call built aSessionContainerwith its ownDatabase, so several writer connections shared one SQLite file and their history syncs raced.replaceConversationFeedruns a transaction that reads the doomed conversation ids before deleting them. UnderBEGIN DEFERREDthat first read takes a WAL snapshot, and when the delete then needs the write lock while another connection holds it, SQLite returnsSQLITE_BUSYat once because upgrading a snapshot cannot wait. The busy handler never runs.Changes:
Containerowns a newDatabaseStorethat opens oneDatabaseper owner and hands the same instance back on every later request. It carries the whole open path that used to live inSessionAuthenticator.initializeDatabase— App Group resolution, the legacy-directory migration, and the schema-version check — so the only behavioural change is the cache.SessionAuthenticator.createSessionContainernow reads from the store, andinitializeDatabaseand its helpers are gone.replaceConversationFeed,persistMessages, and the sharedDatabase.transactionhelper runBEGIN IMMEDIATE, so a read-then-write transaction takes the write lock first and waits on the busy handler instead of failing.Regression_6a4fde3suite, four tests: the rival-writer reproduction holds an immediate transaction on a second connection to the same file for 200 ms and asserts the feed replacement lands; the other three cover the seconds-vs-millisecondsbusyTimeoutand the store's per-owner identity.Supersedes #748. That branch was written against the pre-refactor tree and stopped rebasing once the App Group /
FlipcashStorework in #753–#757 landed —Database.swiftmoved packages and the open path it edited no longer exists — so this is the same fix re-authored on top of it. That stack already fixedbusyTimeout— SQLite.swift reads it in seconds, so the old2000armed a 33-minute wait — so the commit for it is dropped here and the suite asserts the value instead.This matches what Android already does:
FlipcashDatabase.init()is a synchronized singleton that returns the existing instance for the same database name, and Room'sbeginTransactionisBEGIN EXCLUSIVE, so its read-then-write DAO transactions never hit the deferred snapshot upgrade.Two things this does not resolve. Why one launch called
completeLoginfive times is still unknown, which is what the info log is for. Andlogout()still does not tear down the previousSessionContainer, so an in-flight write from a stale container now lands on the shared live store rather than failing; that is pre-existing and left for a separate change.Triage brief and plan are in
.claude/plans/2026-09-09-bugsnag-6a4fde3*.md.