Skip to content

fix(database): share one SQLite writer per owner and take write locks up front - #759

Merged
bmc08gt merged 5 commits into
mainfrom
port/database-per-owner
Sep 11, 2026
Merged

bmc08gt merged 5 commits into
mainfrom
port/database-per-owner

Conversation

@bmc08gt

@bmc08gt bmc08gt commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Bugsnag 6a4fde3, "Failed to persist conversation state [replace-feed]" with database is locked (code: 5). The event's log stream shows one cold launch running completeLogin five times for the same owner. Each call built a SessionContainer with its own Database, so several writer connections shared one SQLite file and their history syncs raced.

replaceConversationFeed runs a transaction that reads the doomed conversation ids before deleting them. Under BEGIN DEFERRED that first read takes a WAL snapshot, and when the delete then needs the write lock while another connection holds it, SQLite returns SQLITE_BUSY at once because upgrading a snapshot cannot wait. The busy handler never runs.

Changes:

  • Container owns a new DatabaseStore that opens one Database per owner and hands the same instance back on every later request. It carries the whole open path that used to live in SessionAuthenticator.initializeDatabase — App Group resolution, the legacy-directory migration, and the schema-version check — so the only behavioural change is the cache. SessionAuthenticator.createSessionContainer now reads from the store, and initializeDatabase and its helpers are gone.
  • replaceConversationFeed, persistMessages, and the shared Database.transaction helper run BEGIN IMMEDIATE, so a read-then-write transaction takes the write lock first and waits on the busy handler instead of failing.
  • The two login-path logs move from debug to info with the attempt count in metadata, so the next release report shows how many times login ran.
  • New Regression_6a4fde3 suite, 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-milliseconds busyTimeout and the store's per-owner identity.

Supersedes #748. That branch was written against the pre-refactor tree and stopped rebasing once the App Group / FlipcashStore work in #753–#757 landed — Database.swift moved 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 fixed busyTimeout — SQLite.swift reads it in seconds, so the old 2000 armed 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's beginTransaction is BEGIN EXCLUSIVE, so its read-then-write DAO transactions never hit the deferred snapshot upgrade.

Two things this does not resolve. Why one launch called completeLogin five times is still unknown, which is what the info log is for. And logout() still does not tear down the previous SessionContainer, 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.

…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 bmc08gt self-assigned this Sep 11, 2026
@bmc08gt
bmc08gt merged commit 1323b63 into main Sep 11, 2026
1 check passed
@bmc08gt
bmc08gt deleted the port/database-per-owner branch September 11, 2026 18:19
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant