Skip to content

feat(database): open the store on demand, close it on background - #753

Merged
bmc08gt merged 3 commits into
feat/push-preload-groundworkfrom
feat/database-connection-lifecycle
Sep 11, 2026
Merged

feat(database): open the store on demand, close it on background#753
bmc08gt merged 3 commits into
feat/push-preload-groundworkfrom
feat/database-connection-lifecycle

Conversation

@bmc08gt

@bmc08gt bmc08gt commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Database opened a reader and a writer in init and held both until the process died. That is harmless while the app is the only thing on the file. It is the pattern iOS kills with 0xdead10cc once the store moves into the App Group and the notification service extension opens it too — so the store needs a close before it can move.

What changed

reader and writer are now throwing computed properties over optional storage guarded by an NSLock. close() checkpoints the write-ahead log and drops both connections; the next access reopens the store and reapplies the pragmas.

No production call site changed. All 121 reader./writer. uses in Database+*.swift and Schema.swift were already inside a try expression, which is why this shape won over a withWriter { } closure that would have rewritten every one of them. The eight uses in tests reading writer.totalChanges outside a try are the whole of the churn.

AppDelegate.scenePhaseChanged(.background) calls close() under a background-task assertion. .active has no counterpart on purpose: reopening is lazy, so an app that never comes back costs nothing and a close that lands at an awkward moment repairs itself on the next read.

journal_mode lives in the database header, but cache_size, foreign_keys and the busy timeout are per-connection and do not survive a close. The new tests assert them after a cycle rather than trusting the open path to have run.

The busy timeout was wrong by a factor of a thousand

Connection.busyTimeout is a Double of seconds that SQLite.swift multiplies by 1000 before calling sqlite3_busy_timeout, so busyTimeout = 2000 asked for 2000 seconds — next to a comment reading // 2 sec. It is the first commit on the branch, on its own.

Nothing has hit that ceiling while one process owns the store, which is why it has gone unnoticed. It stops being academic when the extension opens the same file: a 33-minute wait in a process that lives about thirty seconds is a hang.

What close() does not promise

Connection releases its handle from deinit and exposes no close(), so dropping the references only closes the store if nothing else is holding a connection. A caller partway through transaction(_:) keeps one alive until it returns, and the store closes when it does.

Under the app's own lifecycle that window is milliseconds at the background transition. It matters more with a second process on the file, and the extension's side of that is a later PR.

Stacked

Branches off feat/push-preload-groundwork (#751), which carries checkpoint(). Retarget to main once that merges.

SQLite.swift's `Connection.busyTimeout` is a `Double` of seconds that gets
multiplied by 1000 on the way to `sqlite3_busy_timeout`, so `busyTimeout = 2000`
asked both connections to block for 2000 seconds — 33 minutes — where the comment
beside it said two.

Nothing has hit that ceiling while one process owns the store. It stops being
academic once the notification service extension opens the same file: a wait that
long is indistinguishable from a hang, and the extension has about thirty seconds
to live.
`Database` opened a reader and a writer in `init` and held both for the life of
the process, with no way to give them back. `close()` supplies the other half:
checkpoint the write-ahead log, drop both connections, and let the next `reader`
or `writer` access reopen the store and reapply the pragmas.

Dropping the references is what closes the store. SQLite.swift's `Connection`
releases its handle from `deinit` and exposes no `close()` of its own, so a
connection another caller is still holding closes when that caller returns
rather than here. That is also why nothing pairs with `close()`: a call that
lands at an awkward moment costs a reopen instead of leaving a dead object
behind.

`reader` and `writer` become throwing computed properties. All 121 uses in
`Database+*.swift` and `Schema.swift` already sat inside a `try` expression and
did not change; the eight in tests reading `writer.totalChanges` did not, and
now do.

`journal_mode` lives in the database header, but `cache_size`, `foreign_keys`
and the busy timeout are per-connection and are gone after a close, so
`pragmasAreReappliedOnReopen` and `busyTimeoutIsTwoSeconds` assert them through
a cycle rather than trusting the open path to have run.
`scenePhaseChanged(.background)` now checkpoints and closes the store under a
background-task assertion. `.active` has no counterpart on purpose: the
connections reopen on the first read, so an app that never comes back costs
nothing and a close that lands at an awkward moment repairs itself.

The assertion covers the checkpoint, which is file I/O proportional to the
write-ahead log. Being suspended partway through it leaves the log on disk for
the next launch to replay rather than damaging the store, so the assertion buys
a faster next launch, not correctness.
@bmc08gt bmc08gt self-assigned this Sep 10, 2026
@bmc08gt
bmc08gt merged commit d499977 into feat/push-preload-groundwork Sep 11, 2026
bmc08gt added a commit that referenced this pull request Sep 11, 2026
* fix(database): set the busy timeout in seconds, not milliseconds

SQLite.swift's `Connection.busyTimeout` is a `Double` of seconds that gets
multiplied by 1000 on the way to `sqlite3_busy_timeout`, so `busyTimeout = 2000`
asked both connections to block for 2000 seconds — 33 minutes — where the comment
beside it said two.

Nothing has hit that ceiling while one process owns the store. It stops being
academic once the notification service extension opens the same file: a wait that
long is indistinguishable from a hang, and the extension has about thirty seconds
to live.

* feat(database): open the store on demand and close it on request

`Database` opened a reader and a writer in `init` and held both for the life of
the process, with no way to give them back. `close()` supplies the other half:
checkpoint the write-ahead log, drop both connections, and let the next `reader`
or `writer` access reopen the store and reapply the pragmas.

Dropping the references is what closes the store. SQLite.swift's `Connection`
releases its handle from `deinit` and exposes no `close()` of its own, so a
connection another caller is still holding closes when that caller returns
rather than here. That is also why nothing pairs with `close()`: a call that
lands at an awkward moment costs a reopen instead of leaving a dead object
behind.

`reader` and `writer` become throwing computed properties. All 121 uses in
`Database+*.swift` and `Schema.swift` already sat inside a `try` expression and
did not change; the eight in tests reading `writer.totalChanges` did not, and
now do.

`journal_mode` lives in the database header, but `cache_size`, `foreign_keys`
and the busy timeout are per-connection and are gone after a close, so
`pragmasAreReappliedOnReopen` and `busyTimeoutIsTwoSeconds` assert them through
a cycle rather than trusting the open path to have run.

* feat(app): close the database when the app enters the background

`scenePhaseChanged(.background)` now checkpoints and closes the store under a
background-task assertion. `.active` has no counterpart on purpose: the
connections reopen on the first read, so an app that never comes back costs
nothing and a close that lands at an awkward moment repairs itself.

The assertion covers the checkpoint, which is file I/O proportional to the
write-ahead log. Being suspended partway through it leaves the log on disk for
the next launch to replay rather than damaging the store, so the assertion buys
a faster next launch, not correctness.
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