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) { diff --git a/Flipcash/Core/Controllers/Database/Database.swift b/Flipcash/Core/Controllers/Database/Database.swift index 2fcce265c..08075feb0 100644 --- a/Flipcash/Core/Controllers/Database/Database.swift +++ b/Flipcash/Core/Controllers/Database/Database.swift @@ -18,31 +18,86 @@ 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) - - writer.busyTimeout = 2000 // 2 sec - 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 - + + // 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`. + 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 - @@ -52,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 @@ -87,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) + } }