From 1977e4d4fe0ebe7b99196d3e35e6a4353b965f5d Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 10 Sep 2026 16:29:20 -0400 Subject: [PATCH 1/3] fix(database): set the busy timeout in seconds, not milliseconds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- Flipcash/Core/Controllers/Database/Database.swift | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Flipcash/Core/Controllers/Database/Database.swift b/Flipcash/Core/Controllers/Database/Database.swift index 2fcce265c..4b05e917a 100644 --- a/Flipcash/Core/Controllers/Database/Database.swift +++ b/Flipcash/Core/Controllers/Database/Database.swift @@ -33,13 +33,15 @@ nonisolated class Database: @unchecked Sendable { self.writer = try Connection(url.path) - writer.busyTimeout = 2000 // 2 sec + // Seconds, not milliseconds: SQLite.swift multiplies by 1000 before handing + // the value to `sqlite3_busy_timeout`. + writer.busyTimeout = 2 try writer.run("PRAGMA journal_mode = WAL;") try writer.run("PRAGMA cache_size = 10000;") try writer.run("PRAGMA foreign_keys = ON;") self.reader = try Connection(url.path, readonly: true) - reader.busyTimeout = 2000 // 2 Sec + reader.busyTimeout = 2 try createTablesIfNeeded() } From e0a28b0c8334367a89ae461d03429820199ec40e Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 10 Sep 2026 16:29:34 -0400 Subject: [PATCH 2/3] 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. --- .../Core/Controllers/Database/Database.swift | 118 +++++++++++++++--- .../Database+BalanceUpsertTests.swift | 16 +-- .../Database/Database+LiveSupplyTests.swift | 4 +- .../Database/Database+MintUpsertTests.swift | 12 +- FlipcashTests/DatabaseLifecycleTests.swift | 91 ++++++++++++++ 5 files changed, 206 insertions(+), 35 deletions(-) diff --git a/Flipcash/Core/Controllers/Database/Database.swift b/Flipcash/Core/Controllers/Database/Database.swift index 4b05e917a..08075feb0 100644 --- a/Flipcash/Core/Controllers/Database/Database.swift +++ b/Flipcash/Core/Controllers/Database/Database.swift @@ -18,32 +18,85 @@ typealias Expression = SQLite.Expression // despite Database itself being a reference type. Marking it // `@unchecked Sendable` lets background write paths (e.g. RatesController's // rate persistence queue) capture it without escaping Swift 6 isolation. +// The connections themselves are mutable now that they can be closed and +// reopened, so `lock` — not isolation — is what makes that state safe. // FOLLOW-UP: Remove @unchecked when SQLite.swift declares Connection: Sendable. nonisolated class Database: @unchecked Sendable { - let reader: Connection - let writer: Connection - private let storeURL: URL + + /// Both are `nil` while the store is closed, and are guarded by `lock` — the two + /// accessors below are the only things that touch them. + private var _reader: Connection? + private var _writer: Connection? + + private let lock = NSLock() + + /// The write connection, opening the store first if it is currently closed. + var writer: Connection { + get throws { + lock.lock() + defer { lock.unlock() } + + if let existing = _writer { + return existing + } + + let connection = try Self.openWriter(at: storeURL) + _writer = connection + return connection + } + } + + /// The read connection, opening the store first if it is currently closed. + var reader: Connection { + get throws { + lock.lock() + defer { lock.unlock() } + + if let existing = _reader { + return existing + } + + let connection = try Self.openReader(at: storeURL) + _reader = connection + return connection + } + } // MARK: - Init - init(url: URL) throws { self.storeURL = url - - self.writer = try Connection(url.path) - + + // Opening both here keeps an unusable store path failing at `init`, where it + // has always failed, rather than deferring it to whichever query runs first. + _ = try writer + _ = try reader + + try createTablesIfNeeded() + } + + private static func openWriter(at url: URL) throws -> Connection { + let connection = try Connection(url.path) + // Seconds, not milliseconds: SQLite.swift multiplies by 1000 before handing // the value to `sqlite3_busy_timeout`. - writer.busyTimeout = 2 - try writer.run("PRAGMA journal_mode = WAL;") - try writer.run("PRAGMA cache_size = 10000;") - try writer.run("PRAGMA foreign_keys = ON;") - - self.reader = try Connection(url.path, readonly: true) - reader.busyTimeout = 2 - - try createTablesIfNeeded() + connection.busyTimeout = 2 + + // `journal_mode` is persisted in the database header, but the other two are + // per-connection and have to be set again every time the store is reopened. + try connection.run("PRAGMA journal_mode = WAL;") + try connection.run("PRAGMA cache_size = 10000;") + try connection.run("PRAGMA foreign_keys = ON;") + + return connection + } + + private static func openReader(at url: URL) throws -> Connection { + let connection = try Connection(url.path, readonly: true) + connection.busyTimeout = 2 + return connection } // MARK: - Transaction - @@ -54,11 +107,12 @@ nonisolated class Database: @unchecked Sendable { @inline(__always) func transaction(silent: Bool = false, _ block: (Database) throws -> Void) rethrows { do { - let startChangeCount = writer.totalChanges - try writer.transaction { [unowned self] in + let connection = try writer + let startChangeCount = connection.totalChanges + try connection.transaction { [unowned self] in try block(self) } - let endChangeCount = writer.totalChanges + let endChangeCount = connection.totalChanges // There are instances where we want to commit // the transaction but avoid notifying the UI @@ -89,13 +143,39 @@ nonisolated class Database: @unchecked Sendable { // MARK: - Lifecycle - + private static let checkpointPragma = "PRAGMA wal_checkpoint(TRUNCATE);" + /// Flushes the write-ahead log back into the main database file and truncates it. /// /// TRUNCATE rather than PASSIVE: a passive checkpoint gives up silently when any /// reader is mid-transaction, which is the case that leaves the WAL growing without /// bound. This blocks up to `busyTimeout` instead, and throws when it cannot finish. func checkpoint() throws { - try writer.run("PRAGMA wal_checkpoint(TRUNCATE);") + try writer.run(Self.checkpointPragma) + } + + /// Checkpoints the write-ahead log and drops both connections. + /// + /// 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 someone else still holds — a caller partway through `transaction(_:)`, + /// say — closes when that caller returns rather than here. + /// + /// Nothing pairs with this. The next `reader` or `writer` access reopens the store + /// and reapplies the pragmas, which is what lets a close arriving at an awkward + /// moment heal itself instead of leaving the caller with a dead object. + func close() throws { + lock.lock() + defer { + _reader = nil + _writer = nil + lock.unlock() + } + + // Checkpoint before the writer goes. A WAL left on disk is replayed by whichever + // process opens the store next, which is correct but makes that open cost time + // proportional to the log rather than to what the caller wanted to read. + try _writer?.run(Self.checkpointPragma) } // MARK: - Versioning - diff --git a/FlipcashTests/Database/Database+BalanceUpsertTests.swift b/FlipcashTests/Database/Database+BalanceUpsertTests.swift index 4c25bf2f3..8b6562e6e 100644 --- a/FlipcashTests/Database/Database+BalanceUpsertTests.swift +++ b/FlipcashTests/Database/Database+BalanceUpsertTests.swift @@ -19,9 +19,9 @@ struct DatabaseBalanceUpsertTests { try db.insertBalance(quarks: 1_000, mint: mint, costBasis: 2.5, date: .now) - let before = db.writer.totalChanges + let before = try db.writer.totalChanges try db.insertBalance(quarks: 1_000, mint: mint, costBasis: 2.5, date: .now + 60) - #expect(db.writer.totalChanges == before) + #expect(try db.writer.totalChanges == before) } @Test("Changed quarks still update the stored balance") @@ -33,9 +33,9 @@ struct DatabaseBalanceUpsertTests { try db.insert(mints: [.makeLaunchpad(address: mint)], date: .now) try db.insertBalance(quarks: 1_000, mint: mint, costBasis: 2.5, date: .now) - let before = db.writer.totalChanges + let before = try db.writer.totalChanges try db.insertBalance(quarks: 2_000, mint: mint, costBasis: 2.5, date: .now + 60) - #expect(db.writer.totalChanges > before) + #expect(try db.writer.totalChanges > before) #expect(try db.getBalances().first?.quarks == 2_000) } @@ -48,9 +48,9 @@ struct DatabaseBalanceUpsertTests { try db.insert(mints: [.makeLaunchpad(address: mint)], date: .now) try db.insertBalance(quarks: 1_000, mint: mint, costBasis: 2.5, date: .now) - let before = db.writer.totalChanges + let before = try db.writer.totalChanges try db.insertBalance(quarks: 1_000, mint: mint, costBasis: 3.0, date: .now + 60) - #expect(db.writer.totalChanges > before) + #expect(try db.writer.totalChanges > before) #expect(try db.getBalances().first?.costBasis == 3.0) } @@ -59,8 +59,8 @@ struct DatabaseBalanceUpsertTests { let (db, url) = try Database.makeTemp() defer { Database.removeTemp(at: url) } - let before = db.writer.totalChanges + let before = try db.writer.totalChanges try db.insertBalance(quarks: 1_000, mint: .jeffy, costBasis: 0, date: .now) - #expect(db.writer.totalChanges > before) + #expect(try db.writer.totalChanges > before) } } diff --git a/FlipcashTests/Database/Database+LiveSupplyTests.swift b/FlipcashTests/Database/Database+LiveSupplyTests.swift index 7d3725b6b..0faa4064a 100644 --- a/FlipcashTests/Database/Database+LiveSupplyTests.swift +++ b/FlipcashTests/Database/Database+LiveSupplyTests.swift @@ -87,12 +87,12 @@ struct DatabaseLiveSupplyTests { date: .now ) - let before = db.writer.totalChanges + let before = try db.writer.totalChanges try db.updateLiveSupply( updates: [ReserveStateUpdate(mint: mint, supplyFromBonding: 500)], date: .now + 60 ) - #expect(db.writer.totalChanges == before) + #expect(try db.writer.totalChanges == before) } @Test("A supply delivered over a NULL column still writes") diff --git a/FlipcashTests/Database/Database+MintUpsertTests.swift b/FlipcashTests/Database/Database+MintUpsertTests.swift index 19885e1bf..8202122e0 100644 --- a/FlipcashTests/Database/Database+MintUpsertTests.swift +++ b/FlipcashTests/Database/Database+MintUpsertTests.swift @@ -146,9 +146,9 @@ struct DatabaseMintUpsertTests { try db.insert(mints: [original], date: .now) let stored = try #require(try db.getMintMetadata(mint: original.address)) - let before = db.writer.totalChanges + let before = try db.writer.totalChanges try db.insert(mints: [original], date: .now + 60) - #expect(db.writer.totalChanges == before) + #expect(try db.writer.totalChanges == before) #expect(try db.getMintMetadata(mint: original.address) == stored) } @@ -162,9 +162,9 @@ struct DatabaseMintUpsertTests { let renamed = MintMetadata.makeLaunchpad(address: mint, name: "Renamed Token") - let before = db.writer.totalChanges + let before = try db.writer.totalChanges try db.insert(mints: [renamed], date: .now + 60) - #expect(db.writer.totalChanges > before) + #expect(try db.writer.totalChanges > before) #expect(try db.getMintMetadata(mint: mint)?.name == "Renamed Token") } @@ -176,9 +176,9 @@ struct DatabaseMintUpsertTests { try db.insert(mints: [.makeLaunchpad(address: mint)], date: .now) - let before = db.writer.totalChanges + let before = try db.writer.totalChanges try db.insert(mints: [Self.makeStaticMint(address: mint)], date: .now + 60) - #expect(db.writer.totalChanges > before) + #expect(try db.writer.totalChanges > before) } @Test("Balance is visible after mint upsert without launchpadMetadata") diff --git a/FlipcashTests/DatabaseLifecycleTests.swift b/FlipcashTests/DatabaseLifecycleTests.swift index 38aa73201..985614fee 100644 --- a/FlipcashTests/DatabaseLifecycleTests.swift +++ b/FlipcashTests/DatabaseLifecycleTests.swift @@ -59,4 +59,95 @@ struct DatabaseLifecycleTests { #expect(size(of: walURL) == 0) } + + @Test("closing checkpoints the write-ahead log") + func closeEmptiesWAL() throws { + let (database, walURL) = try makeDatabase() + try database.writer.run("CREATE TABLE probe (id INTEGER PRIMARY KEY, value TEXT);") + for index in 0..<200 { + try database.writer.run("INSERT INTO probe (value) VALUES (?);", "row-\(index)") + } + #expect(size(of: walURL) > 0) + + try database.close() + + #expect(size(of: walURL) == 0) + } + + @Test("a read after a close reopens the store with its rows intact") + func readAfterCloseReopens() throws { + let (database, _) = try makeDatabase() + try database.writer.run("CREATE TABLE probe (id INTEGER PRIMARY KEY, value TEXT);") + try database.writer.run("INSERT INTO probe (value) VALUES (?);", "kept") + + try database.close() + + let value = try database.reader.scalar("SELECT value FROM probe LIMIT 1;") as? String + #expect(value == "kept") + } + + @Test("a write after a close lands, and survives a second close") + func writeAfterCloseSurvivesAnotherCycle() throws { + let (database, _) = try makeDatabase() + try database.writer.run("CREATE TABLE probe (id INTEGER PRIMARY KEY, value TEXT);") + + try database.close() + try database.writer.run("INSERT INTO probe (value) VALUES (?);", "after-close") + try database.close() + + let value = try database.reader.scalar("SELECT value FROM probe LIMIT 1;") as? String + #expect(value == "after-close") + } + + @Test("a transaction after a close reopens and commits") + func transactionAfterCloseCommits() throws { + let (database, _) = try makeDatabase() + try database.writer.run("CREATE TABLE probe (id INTEGER PRIMARY KEY, value TEXT);") + + try database.close() + try database.transaction(silent: true) { db in + try db.writer.run("INSERT INTO probe (value) VALUES (?);", "committed") + } + + let value = try database.reader.scalar("SELECT value FROM probe LIMIT 1;") as? String + #expect(value == "committed") + } + + @Test("the per-connection pragmas are reapplied when the store reopens") + func pragmasAreReappliedOnReopen() throws { + let (database, _) = try makeDatabase() + + try database.close() + + let writer = try database.writer + #expect(try writer.scalar("PRAGMA foreign_keys;") as? Int64 == 1) + #expect(try writer.scalar("PRAGMA cache_size;") as? Int64 == 10_000) + #expect(try writer.scalar("PRAGMA journal_mode;") as? String == "wal") + } + + /// `PRAGMA busy_timeout` reports milliseconds; SQLite.swift's `busyTimeout` property + /// is in seconds and multiplies by 1000 on the way to `sqlite3_busy_timeout`. This is + /// the assertion that catches the two being confused. + @Test("both connections wait two seconds on a busy store, before and after a reopen") + func busyTimeoutIsTwoSeconds() throws { + let (database, _) = try makeDatabase() + + #expect(try database.writer.scalar("PRAGMA busy_timeout;") as? Int64 == 2_000) + #expect(try database.reader.scalar("PRAGMA busy_timeout;") as? Int64 == 2_000) + + try database.close() + + #expect(try database.writer.scalar("PRAGMA busy_timeout;") as? Int64 == 2_000) + #expect(try database.reader.scalar("PRAGMA busy_timeout;") as? Int64 == 2_000) + } + + @Test("closing twice is not an error") + func closeIsIdempotent() throws { + let (database, _) = try makeDatabase() + + try database.close() + try database.close() + + #expect(try database.reader.scalar("SELECT 1;") as? Int64 == 1) + } } From e2e69074509189e4877d67ebbc0e14482eb01836 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 10 Sep 2026 16:29:34 -0400 Subject: [PATCH 3/3] 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. --- Flipcash/Core/AppDelegate.swift | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/Flipcash/Core/AppDelegate.swift b/Flipcash/Core/AppDelegate.swift index 27e277060..f04a714f3 100644 --- a/Flipcash/Core/AppDelegate.swift +++ b/Flipcash/Core/AppDelegate.swift @@ -129,6 +129,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate { sessionContainer?.session.didEnterBackground() container.preferences.appDidEnterBackground() sessionContainer?.pushController.clearBadgeCount() + closeDatabase() case .active: logger.info("scenePhase → active") container.client.warmUpChannel() @@ -147,6 +148,38 @@ class AppDelegate: UIResponder, UIApplicationDelegate { } } + /// Checkpoints and closes the store on the way to the background. + /// + /// `.active` has no counterpart on purpose: the connections reopen on the first + /// read after the app comes back, so a return that never happens costs nothing and + /// a close that lands at an awkward moment repairs itself. + /// + /// The background-task 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. + private func closeDatabase() { + guard let database = sessionContainer?.database else { + return + } + + var identifier = UIBackgroundTaskIdentifier.invalid + identifier = UIApplication.shared.beginBackgroundTask(withName: "database.close") { + UIApplication.shared.endBackgroundTask(identifier) + identifier = .invalid + } + + do { + try database.close() + } catch { + logger.error("Failed to close the database", metadata: ["error": "\(error)"]) + } + + if identifier != .invalid { + UIApplication.shared.endBackgroundTask(identifier) + } + } + // MARK: - Deep Links - func handleOpenURL(url: URL) {