From 6a48f7711e61cda933ece549c6787bd4b4279de6 Mon Sep 17 00:00:00 2001 From: torlando-tech <239676438+torlando-tech@users.noreply.github.com> Date: Tue, 27 Oct 2026 01:28:43 +0000 Subject: [PATCH 1/2] fix: suspend app GRDB access before background suspension --- ARCHITECTURE.md | 4 + Sources/ColumbaApp/App/ColumbaApp.swift | 6 + Sources/ColumbaApp/Services/AppServices.swift | 18 + .../Services/BackgroundPropagationSync.swift | 204 +++++++++-- .../BackgroundPropagationSyncTests.swift | 342 +++++++++++++++++- Tests/ColumbaAppTests/MessageStoreTests.swift | 103 ++++++ 6 files changed, 648 insertions(+), 29 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 9308b858..9b1dee49 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -34,6 +34,10 @@ Both hosts use the same per-identity App Group directory, shared keychain identi Python projects inbound messages into the shared store. `NodeInboundReplay` runs the common app field-processing path, including reactions, replies, telemetry, and attachments. Propagation synchronization awaits replay catch-up. The durable event inbox remains bounded and is acknowledged only after consumption. Stable send IDs and the durable outbox carry complete fields and propagated fallback across host/process restarts. +App-side GRDB pools opt into explicit suspension notifications. `DatabaseSuspensionController` bridges process-level UIKit background/foreground notifications to GRDB: without foreground execution or a live system background-task grant, writes are interrupted and new lock-taking accesses are refused. The bridge is installed at app launch, independently of SwiftUI scenes. Notifications remain inside the app process; backgrounding the UI does not suspend the Network Extension's database. + +Background-task deliveries own separate database-access grants, including deliveries retained before the handler is ready. Completion releases a grant before reporting task completion; expiration revokes it before waiting for cancellation cleanup. Overlapping grants cannot suspend each other's work, and ending a grant cannot suspend a foreground app. Startup waits for permitted access immediately before opening new repository pools, because a pool cannot observe a suspension notification sent before it existed. Interrupted writes retain GRDB's error semantics; the lifecycle bridge does not silently report success or retry them while suspended. + The persisted propagation configuration is restored before recovering outbox sends when iOS starts the extension without the app. Live relay selection, sync/cancellation, discovery, transfer limits, and peer identity retention cross typed commands through the same dispatcher in either mode. Propagation uploads and downloads share the router's outbound relay link. The Python sync bridge waits for a pre-existing pending link within the sync deadline, then explicitly starts retrieval; it never treats the previous sync's terminal state as the current result. Already-closed links are discarded on retry, while a link that closes during the wait reports `link_failed`. Failed operations report zero received messages rather than a previous sync's count. diff --git a/Sources/ColumbaApp/App/ColumbaApp.swift b/Sources/ColumbaApp/App/ColumbaApp.swift index e1cb1877..8927c90e 100644 --- a/Sources/ColumbaApp/App/ColumbaApp.swift +++ b/Sources/ColumbaApp/App/ColumbaApp.swift @@ -52,6 +52,12 @@ struct ColumbaApp: App { init() { _ = Self.launchHostingMode + #if os(iOS) + // GRDB's opt-in flag listens to explicit suspension notifications, + // not UIKit lifecycle events. Install the process-wide bridge even + // when a background launch never creates a SwiftUI scene. + DatabaseSuspensionController.shared.observeApplicationLifecycle() + #endif // Privacy hygiene + bounded storage: wipe diag.log (and its rotated // sibling) before ANY launch work so logs left by pre-#186 builds, // which leaked received-message plaintext on every inbound message, diff --git a/Sources/ColumbaApp/Services/AppServices.swift b/Sources/ColumbaApp/Services/AppServices.swift index 5bb9bd51..9310798f 100644 --- a/Sources/ColumbaApp/Services/AppServices.swift +++ b/Sources/ColumbaApp/Services/AppServices.swift @@ -1496,6 +1496,12 @@ public final class AppServices { Self.migrateLXMFDatabaseToAppGroupIfNeeded(identityHashHex: newIdentity.hexHash) let grdbPath = Self.grdbDatabaseFilePath(for: newIdentity.hexHash) self.grdbDatabasePath = grdbPath + #if os(iOS) + // Newly opened pools cannot receive a suspension notification sent + // before they existed. Wait for foreground or an authorized BG task + // before opening them; no await may separate this gate from setup. + try await DatabaseSuspensionController.shared.waitUntilAccessAllowed() + #endif let messageRepository = try MessageRepository(grdbPath: grdbPath) self.messageRepository = messageRepository // Open the identity-scoped call-history store. A repository left over @@ -1509,6 +1515,9 @@ public final class AppServices { self.callHistoryRepository = nil } do { + #if os(iOS) + try await DatabaseSuspensionController.shared.waitUntilAccessAllowed() + #endif let callHistory = try CallHistoryRepository(grdbPath: grdbPath) self.callHistoryRepository = callHistory DiagLog.log("[INIT] call-history repository ready at \(grdbPath)") @@ -3900,6 +3909,9 @@ public final class AppServices { Self.migrateLXMFDatabaseToAppGroupIfNeeded(identityHashHex: identityHash) let grdbPath = Self.grdbDatabaseFilePath(for: identityHash) self.grdbDatabasePath = grdbPath + #if os(iOS) + try await DatabaseSuspensionController.shared.waitUntilAccessAllowed() + #endif let messageRepository = try MessageRepository(grdbPath: grdbPath) self.messageRepository = messageRepository // Open the identity-scoped call-history store. A repository left over @@ -3913,6 +3925,9 @@ public final class AppServices { self.callHistoryRepository = nil } do { + #if os(iOS) + try await DatabaseSuspensionController.shared.waitUntilAccessAllowed() + #endif let callHistory = try CallHistoryRepository(grdbPath: grdbPath) self.callHistoryRepository = callHistory DiagLog.log("[INIT] call-history repository ready at \(grdbPath)") @@ -4758,6 +4773,9 @@ public final class AppServices { Self.migrateLXMFDatabaseToAppGroupIfNeeded(identityHashHex: existingIdentity.hexHash) let grdbPath = Self.grdbDatabaseFilePath(for: existingIdentity.hexHash) self.grdbDatabasePath = grdbPath + #if os(iOS) + try await DatabaseSuspensionController.shared.waitUntilAccessAllowed() + #endif let messageRepository = try MessageRepository(grdbPath: grdbPath) self.messageRepository = messageRepository let recoveredRetryCount = try await messageRepository.recoverInterruptedRetries() diff --git a/Sources/ColumbaApp/Services/BackgroundPropagationSync.swift b/Sources/ColumbaApp/Services/BackgroundPropagationSync.swift index 471f0c35..8619ed63 100644 --- a/Sources/ColumbaApp/Services/BackgroundPropagationSync.swift +++ b/Sources/ColumbaApp/Services/BackgroundPropagationSync.swift @@ -19,6 +19,7 @@ #if os(iOS) import BackgroundTasks import Foundation +import GRDB import RNSAPI import UIKit import os @@ -28,6 +29,113 @@ private let backgroundPropagationLogger = Logger( category: "BackgroundPropagationSync" ) +/// GRDB suspension is process-local: the app must not hold database locks +/// without foreground execution or an unexpired system background-task grant. +@MainActor +final class DatabaseSuspensionController { + static let shared = DatabaseSuspensionController( + isForeground: UIApplication.shared.applicationState != .background + ) + + private let notificationCenter: NotificationCenter + private var isForeground: Bool + private var backgroundAccessTokens: Set = [] + private var accessAllowed: Bool + private var lifecycleObservers: [NSObjectProtocol] = [] + private var accessWaiters: [UUID: CheckedContinuation] = [:] + + init(isForeground: Bool, notificationCenter: NotificationCenter = .default) { + self.isForeground = isForeground + self.notificationCenter = notificationCenter + accessAllowed = isForeground + notificationCenter.post( + name: isForeground ? Database.resumeNotification : Database.suspendNotification, + object: nil + ) + } + + deinit { + for observer in lifecycleObservers { + notificationCenter.removeObserver(observer) + } + } + + func observeApplicationLifecycle() { + guard lifecycleObservers.isEmpty else { return } + lifecycleObservers = [ + notificationCenter.addObserver( + forName: UIApplication.didEnterBackgroundNotification, object: nil, queue: .main + ) { [weak self] _ in + MainActor.assumeIsolated { self?.setForeground(false) } + }, + notificationCenter.addObserver( + forName: UIApplication.willEnterForegroundNotification, object: nil, queue: .main + ) { [weak self] _ in + MainActor.assumeIsolated { self?.setForeground(true) } + } + ] + } + + func setForeground(_ isForeground: Bool) { + self.isForeground = isForeground + updateSuspension() + } + + func beginBackgroundAccess() -> UUID { + let token = UUID() + backgroundAccessTokens.insert(token) + updateSuspension() + return token + } + + func endBackgroundAccess(_ token: UUID) { + guard backgroundAccessTokens.remove(token) != nil else { return } + updateSuspension() + } + + /// Startup must not open a new pool after it has missed the process's + /// suspension notification. Call immediately before synchronous creation. + func waitUntilAccessAllowed() async throws { + try Task.checkCancellation() + while !accessAllowed { + let identifier = UUID() + try await withTaskCancellationHandler { + try await withCheckedThrowingContinuation { (continuation: CheckedContinuation) in + if Task.isCancelled { + continuation.resume(throwing: CancellationError()) + } else { + accessWaiters[identifier] = continuation + } + } + } onCancel: { + Task { @MainActor [weak self] in + self?.accessWaiters.removeValue(forKey: identifier)? + .resume(throwing: CancellationError()) + } + } + try Task.checkCancellation() + } + try Task.checkCancellation() + } + + private func updateSuspension() { + let allowed = isForeground || !backgroundAccessTokens.isEmpty + guard allowed != accessAllowed else { return } + accessAllowed = allowed + notificationCenter.post( + name: allowed ? Database.resumeNotification : Database.suspendNotification, + object: nil + ) + if allowed { + let waiters = accessWaiters + accessWaiters = [:] + for continuation in waiters.values { + continuation.resume() + } + } + } +} + /// Two system-scheduled task kinds that both trigger the SAME sync workflow. /// Both lanes use the user-configured cadence: registering both multiplies /// the grant opportunities (daytime active refresh vs charging/overnight @@ -190,6 +298,7 @@ private extension UIScene.ActivationState { protocol BackgroundTaskHandle: AnyObject { var expirationHandler: (() -> Void)? { get set } var isCompleted: Bool { get } + var isExpired: Bool { get } func setTaskCompleted(success: Bool) } @@ -198,6 +307,7 @@ private final class LaunchOwnedBackgroundTask: BackgroundTaskHandle, @unchecked private let lock = NSLock() private var storedExpirationHandler: (() -> Void)? private var completed = false + private var expired = false init(task: BGTask) { self.task = task @@ -213,21 +323,31 @@ private final class LaunchOwnedBackgroundTask: BackgroundTaskHandle, @unchecked lock.withLock { completed } } + var isExpired: Bool { + lock.withLock { expired } + } + func setTaskCompleted(success: Bool) { - let shouldComplete = lock.withLock { - guard !completed else { return false } + let completionSuccess: Bool? = lock.withLock { + guard !completed else { return nil } completed = true storedExpirationHandler = nil - return true + return success && !expired } - if shouldComplete { + if let completionSuccess { task.expirationHandler = nil - task.setTaskCompleted(success: success) + task.setTaskCompleted(success: completionSuccess) } } private func expire() { - if let handler = expirationHandler { + let expiration: (shouldExpire: Bool, handler: (() -> Void)?) = lock.withLock { + guard !completed, !expired else { return (false, nil) } + expired = true + return (true, storedExpirationHandler) + } + guard expiration.shouldExpire else { return } + if let handler = expiration.handler { handler() } else { setTaskCompleted(success: false) @@ -248,6 +368,8 @@ final class BackgroundTaskCoordinator { private final class TaskState { let task: any BackgroundTaskHandle var operation: Task? + var backgroundAccessToken: UUID? + var expired = false var completed = false init(task: any BackgroundTaskHandle) { @@ -258,8 +380,13 @@ final class BackgroundTaskCoordinator { static let shared = BackgroundTaskCoordinator() private var handler: Handler? + private let suspensionController: DatabaseSuspensionController private var states: [ObjectIdentifier: TaskState] = [:] + init(suspensionController: DatabaseSuspensionController? = nil) { + self.suspensionController = suspensionController ?? .shared + } + func installHandler(_ handler: @escaping Handler) { self.handler = handler DiagLog.log("[BG-SYNC] coordinator handler installed retained=\(states.count)") @@ -269,8 +396,9 @@ final class BackgroundTaskCoordinator { } func receive(_ task: any BackgroundTaskHandle) { - guard !task.isCompleted else { - DiagLog.log("[BG-SYNC] coordinator ignored task already completed during launch handoff") + guard !task.isCompleted, !task.isExpired else { + if !task.isCompleted { task.setTaskCompleted(success: false) } + DiagLog.log("[BG-SYNC] coordinator ignored task completed or expired during launch handoff") return } let identifier = ObjectIdentifier(task) @@ -280,44 +408,78 @@ final class BackgroundTaskCoordinator { states[identifier] = state DiagLog.log("[BG-SYNC] coordinator received handlerReady=\(handler != nil)") task.expirationHandler = { [weak self, weak state] in - Task { @MainActor in - guard let self, let state else { return } - DiagLog.log("[BG-SYNC] task expiration requested") - if let operation = state.operation { - // Let the operation's structured completion path report failure - // only after its Reticulum/Python cancellation cleanup returns. - operation.cancel() - } else { - self.complete(state, success: false) + if Thread.isMainThread { + MainActor.assumeIsolated { + guard let self, let state else { return } + self.expire(state) + } + } else { + Task { @MainActor in + guard let self, let state else { return } + self.expire(state) } } } - guard !task.isCompleted else { - states.removeValue(forKey: identifier) + guard !task.isCompleted, !task.isExpired, !state.completed else { + complete(state, success: false) return } + // Cold-launch initialization may access GRDB before a handler is ready. + // Retained deliveries own a grant too, until expiration or completion. + state.backgroundAccessToken = suspensionController.beginBackgroundAccess() if handler != nil { start(state) } } private func start(_ state: TaskState) { - guard let handler, state.operation == nil, !state.completed else { return } + guard let handler, state.operation == nil, !state.completed, !state.expired else { return } + guard !state.task.isCompleted, !state.task.isExpired else { + expire(state) + return + } DiagLog.log("[BG-SYNC] coordinator starting workflow") state.operation = Task { @MainActor [weak self, weak state] in guard let self, let state else { return } + guard !state.expired, !state.task.isExpired, !state.task.isCompleted else { + self.complete(state, success: false) + return + } let success = await handler() self.complete(state, success: success && !Task.isCancelled) } } + private func expire(_ state: TaskState) { + guard !state.completed, !state.expired else { return } + state.expired = true + DiagLog.log("[BG-SYNC] task expiration requested") + // Cancellation may wait indefinitely for Python cleanup. Revoke database + // access now, not when that cleanup eventually reports completion. + releaseBackgroundAccess(state) + if let operation = state.operation { + operation.cancel() + } else { + complete(state, success: false) + } + } + + private func releaseBackgroundAccess(_ state: TaskState) { + guard let token = state.backgroundAccessToken else { return } + state.backgroundAccessToken = nil + suspensionController.endBackgroundAccess(token) + } + private func complete(_ state: TaskState, success: Bool) { guard !state.completed else { return } state.completed = true + releaseBackgroundAccess(state) DiagLog.log("[BG-SYNC] coordinator completing success=\(success)") state.task.expirationHandler = nil - state.task.setTaskCompleted(success: success) + if !state.task.isCompleted { + state.task.setTaskCompleted(success: success && !state.expired && !state.task.isExpired) + } states.removeValue(forKey: ObjectIdentifier(state.task)) } } diff --git a/Tests/ColumbaAppTests/BackgroundPropagationSyncTests.swift b/Tests/ColumbaAppTests/BackgroundPropagationSyncTests.swift index 2537a9a0..e4623829 100644 --- a/Tests/ColumbaAppTests/BackgroundPropagationSyncTests.swift +++ b/Tests/ColumbaAppTests/BackgroundPropagationSyncTests.swift @@ -4,11 +4,151 @@ // import XCTest +import GRDB +import UIKit import RNSAPI @testable import ColumbaApp @MainActor final class BackgroundPropagationSyncTests: XCTestCase { + func testDatabaseSuspensionKeepsAccessUntilLastOverlappingGrantEnds() { + let notifications = DatabaseSuspensionNotifications() + let controller = DatabaseSuspensionController( + isForeground: false, notificationCenter: notifications.center + ) + let first = controller.beginBackgroundAccess() + let second = controller.beginBackgroundAccess() + XCTAssertNotEqual(first, second) + XCTAssertEqual(notifications.accessStates, [false, true]) + + controller.endBackgroundAccess(first) + controller.endBackgroundAccess(first) + controller.endBackgroundAccess(UUID()) + XCTAssertEqual(notifications.accessStates, [false, true]) + + controller.endBackgroundAccess(second) + controller.endBackgroundAccess(second) + XCTAssertEqual(notifications.accessStates, [false, true, false]) + } + + func testForegroundTransitionDuringBackgroundGrantKeepsDatabaseUsable() { + let notifications = DatabaseSuspensionNotifications() + let controller = DatabaseSuspensionController( + isForeground: true, notificationCenter: notifications.center + ) + let first = controller.beginBackgroundAccess() + controller.setForeground(false) + XCTAssertEqual(notifications.accessStates, [true]) + controller.endBackgroundAccess(first) + XCTAssertEqual(notifications.accessStates, [true, false]) + + let second = controller.beginBackgroundAccess() + controller.setForeground(true) + controller.endBackgroundAccess(second) + XCTAssertEqual(notifications.accessStates, [true, false, true]) + controller.setForeground(false) + XCTAssertEqual(notifications.accessStates, [true, false, true, false]) + } + + func testDatabaseStartupWaitsForBackgroundGrantOrForeground() async throws { + for useGrant in [true, false] { + let notifications = DatabaseSuspensionNotifications() + let controller = DatabaseSuspensionController( + isForeground: false, notificationCenter: notifications.center + ) + let started = expectation(description: "startup waiting") + var returned = false + let startup = Task { @MainActor in + started.fulfill() + try await controller.waitUntilAccessAllowed() + returned = true + } + await fulfillment(of: [started], timeout: 2) + XCTAssertFalse(returned) + + if useGrant { + let token = controller.beginBackgroundAccess() + try await startup.value + controller.endBackgroundAccess(token) + XCTAssertEqual(notifications.accessStates, [false, true, false]) + } else { + controller.setForeground(true) + try await startup.value + XCTAssertEqual(notifications.accessStates, [false, true]) + } + XCTAssertTrue(returned) + } + } + + func testDatabaseStartupRechecksGrantThatExpiresBeforeWaiterResumes() async throws { + let controller = DatabaseSuspensionController( + isForeground: false, notificationCenter: NotificationCenter() + ) + let started = expectation(description: "startup waiting") + var returned = false + let startup = Task { @MainActor in + started.fulfill() + try await controller.waitUntilAccessAllowed() + returned = true + } + await fulfillment(of: [started], timeout: 2) + + let token = controller.beginBackgroundAccess() + controller.endBackgroundAccess(token) + // Let the resumed continuation re-enter the suspended gate. It must + // not treat the now-expired grant as permission to create a pool. + let checked = expectation(description: "expired grant checked") + Task { @MainActor in + XCTAssertFalse(returned) + checked.fulfill() + } + await fulfillment(of: [checked], timeout: 2) + controller.setForeground(true) + try await startup.value + XCTAssertTrue(returned) + } + + func testDatabaseStartupCancellationDoesNotWaitForPermissionOrResumeTwice() async { + let notifications = DatabaseSuspensionNotifications() + let controller = DatabaseSuspensionController( + isForeground: false, notificationCenter: notifications.center + ) + let started = expectation(description: "startup waiting") + let startup = Task { @MainActor in + started.fulfill() + try await controller.waitUntilAccessAllowed() + } + await fulfillment(of: [started], timeout: 2) + startup.cancel() + + do { + try await startup.value + XCTFail("Canceled startup must not open the database") + } catch { + XCTAssertTrue(error is CancellationError) + } + // Resuming after cancellation must not resume the removed continuation. + controller.setForeground(true) + XCTAssertEqual(notifications.accessStates, [false, true]) + } + + func testAlreadyCanceledDatabaseStartupDoesNotUseForegroundPermission() async { + let controller = DatabaseSuspensionController( + isForeground: true, notificationCenter: NotificationCenter() + ) + let startup = Task { @MainActor in + try await controller.waitUntilAccessAllowed() + } + startup.cancel() + + do { + try await startup.value + XCTFail("Permission must not override cancellation") + } catch { + XCTAssertTrue(error is CancellationError) + } + } + func testSchedulePolicyDisablesRefreshWhenPeriodicSyncIsOff() { XCTAssertNil( BackgroundPropagationSchedulePolicy.nextDelay( @@ -380,7 +520,9 @@ final class BackgroundPropagationSyncTests: XCTestCase { } func testCoordinatorCompletesEachDeliveredTaskKindIndependently() async { - let coordinator = BackgroundTaskCoordinator() + let coordinator = BackgroundTaskCoordinator(suspensionController: DatabaseSuspensionController( + isForeground: true, notificationCenter: NotificationCenter() + )) let refreshTask = FakeBackgroundTaskHandle() let processingTask = FakeBackgroundTaskHandle() var handlerRuns = 0 @@ -585,38 +727,179 @@ final class BackgroundPropagationSyncTests: XCTestCase { } func testTaskReceivedBeforeHandlerRunsAfterHandlerInstallation() async { - let coordinator = BackgroundTaskCoordinator() + let notifications = DatabaseSuspensionNotifications() + let controller = DatabaseSuspensionController( + isForeground: false, notificationCenter: notifications.center + ) + let coordinator = BackgroundTaskCoordinator(suspensionController: controller) let task = FakeBackgroundTaskHandle() coordinator.receive(task) XCTAssertTrue(task.completions.isEmpty) + XCTAssertEqual(notifications.accessStates, [false, true]) - coordinator.installHandler { true } + coordinator.installHandler { + XCTAssertEqual(notifications.accessStates, [false, true]) + return true + } await task.waitForCompletion() XCTAssertEqual(task.completions, [true]) + XCTAssertEqual(notifications.accessStates, [false, true, false]) } - func testExpirationWaitsForOperationCleanupThenCompletesOnceAndSuppressesLateSuccess() async { - let coordinator = BackgroundTaskCoordinator() + func testExpirationRevokesGrantBeforeCleanupThenCompletesOnceWithoutLateResume() async { + let notifications = DatabaseSuspensionNotifications() + let controller = DatabaseSuspensionController( + isForeground: false, notificationCenter: notifications.center + ) + let coordinator = BackgroundTaskCoordinator(suspensionController: controller) let task = FakeBackgroundTaskHandle() let gate = AsyncGate() + let started = expectation(description: "handler started") coordinator.installHandler { + started.fulfill() await gate.wait() return true } coordinator.receive(task) + await fulfillment(of: [started], timeout: 2) + let expiration = task.expirationHandler - task.expirationHandler?() - await Task.yield() - XCTAssertTrue(task.completions.isEmpty) + task.expire() + expiration?() + XCTAssertEqual(notifications.accessStates, [false, true, false]) + XCTAssertTrue(task.completions.isEmpty, "Cleanup still owns exactly-once completion") gate.open() await task.waitForCompletion() + expiration?() + XCTAssertEqual(task.completions, [false]) + XCTAssertEqual(notifications.accessStates, [false, true, false]) + } + + func testRetainedTaskExpirationReleasesGrantAndNeverStartsAfterHandlerInstallation() async { + let notifications = DatabaseSuspensionNotifications() + let controller = DatabaseSuspensionController( + isForeground: false, notificationCenter: notifications.center + ) + let coordinator = BackgroundTaskCoordinator(suspensionController: controller) + let task = FakeBackgroundTaskHandle() + task.onCompletion = { _ in + XCTAssertEqual(notifications.accessStates, [false, true, false]) + } + + coordinator.receive(task) + let expiration = task.expirationHandler + task.expire() + expiration?() + coordinator.installHandler { + XCTFail("An expired retained delivery must not run or reacquire access") + return true + } + coordinator.receive(task) await Task.yield() XCTAssertEqual(task.completions, [false]) + XCTAssertEqual(notifications.accessStates, [false, true, false]) + } + + func testTaskExpiredBeforeMainActorHandoffNeverResumesDatabase() { + let notifications = DatabaseSuspensionNotifications() + let controller = DatabaseSuspensionController( + isForeground: false, notificationCenter: notifications.center + ) + let coordinator = BackgroundTaskCoordinator(suspensionController: controller) + let task = FakeBackgroundTaskHandle() + task.expire() + + coordinator.receive(task) + coordinator.installHandler { + XCTFail("Expired launch delivery must not initialize the workflow") + return true + } + + XCTAssertEqual(task.completions, [false]) + XCTAssertEqual(notifications.accessStates, [false]) + } + + func testExpirationBeforeQueuedHandlerStartsNeverRunsWorkflow() async { + let notifications = DatabaseSuspensionNotifications() + let controller = DatabaseSuspensionController( + isForeground: false, notificationCenter: notifications.center + ) + let coordinator = BackgroundTaskCoordinator(suspensionController: controller) + let task = FakeBackgroundTaskHandle() + coordinator.installHandler { + XCTFail("Expiration must win over a queued handler") + return true + } + + coordinator.receive(task) + task.expire() + await task.waitForCompletion() + + XCTAssertEqual(task.completions, [false]) + XCTAssertEqual(notifications.accessStates, [false, true, false]) + } + + func testOverlappingTaskExpirationKeepsOtherGrantAndForegroundUsable() async { + let notifications = DatabaseSuspensionNotifications() + let controller = DatabaseSuspensionController( + isForeground: false, notificationCenter: notifications.center + ) + let coordinator = BackgroundTaskCoordinator(suspensionController: controller) + let first = FakeBackgroundTaskHandle() + let second = FakeBackgroundTaskHandle() + let gate = AsyncGate() + let started = expectation(description: "remaining handler started") + + coordinator.receive(first) + coordinator.receive(second) + first.expire() + XCTAssertEqual(first.completions, [false]) + XCTAssertEqual(notifications.accessStates, [false, true]) + + coordinator.installHandler { + started.fulfill() + await gate.wait() + return true + } + await fulfillment(of: [started], timeout: 2) + controller.setForeground(true) + second.expire() + XCTAssertEqual(notifications.accessStates, [false, true]) + + gate.open() + await second.waitForCompletion() + XCTAssertEqual(second.completions, [false]) + XCTAssertEqual(notifications.accessStates, [false, true]) + controller.setForeground(false) + XCTAssertEqual(notifications.accessStates, [false, true, false]) + } + + func testTaskSuccessAndFailureReleaseGrantBeforeExactlyOnceCompletion() async { + for success in [true, false] { + let notifications = DatabaseSuspensionNotifications() + let controller = DatabaseSuspensionController( + isForeground: false, notificationCenter: notifications.center + ) + let coordinator = BackgroundTaskCoordinator(suspensionController: controller) + let task = FakeBackgroundTaskHandle() + task.onCompletion = { _ in + XCTAssertEqual(notifications.accessStates, [false, true, false]) + } + coordinator.installHandler { success } + coordinator.receive(task) + let lateExpiration = task.expirationHandler + await task.waitForCompletion() + lateExpiration?() + coordinator.receive(task) + + XCTAssertEqual(task.completions, [success]) + XCTAssertEqual(notifications.accessStates, [false, true, false]) + } } private func makeMessage( @@ -657,12 +940,15 @@ private actor EventRecorder { private final class FakeBackgroundTaskHandle: BackgroundTaskHandle { var expirationHandler: (() -> Void)? + var onCompletion: ((Bool) -> Void)? + private(set) var isExpired = false private(set) var completions: [Bool] = [] private var continuation: CheckedContinuation? var isCompleted: Bool { !completions.isEmpty } func setTaskCompleted(success: Bool) { completions.append(success) + onCompletion?(success) continuation?.resume() continuation = nil } @@ -673,24 +959,64 @@ private final class FakeBackgroundTaskHandle: BackgroundTaskHandle { self.continuation = continuation } } + + func expire() { + isExpired = true + expirationHandler?() + } } @MainActor private final class AsyncGate { private var continuation: CheckedContinuation? + private var isOpen = false func wait() async { + guard !isOpen else { return } await withCheckedContinuation { continuation in self.continuation = continuation } } func open() { + isOpen = true continuation?.resume() continuation = nil } } +private final class DatabaseSuspensionNotifications: @unchecked Sendable { + let center = NotificationCenter() + private let lock = NSLock() + private var states: [Bool] = [] + private var observers: [NSObjectProtocol] = [] + + init() { + observers = [ + center.addObserver(forName: Database.resumeNotification, object: nil, queue: nil) { [weak self] _ in + self?.record(true) + }, + center.addObserver(forName: Database.suspendNotification, object: nil, queue: nil) { [weak self] _ in + self?.record(false) + } + ] + } + + deinit { + for observer in observers { + center.removeObserver(observer) + } + } + + var accessStates: [Bool] { + lock.withLock { states } + } + + private func record(_ allowed: Bool) { + lock.withLock { states.append(allowed) } + } +} + @MainActor private final class ControlledPropagationSync { var started: XCTestExpectation? diff --git a/Tests/ColumbaAppTests/MessageStoreTests.swift b/Tests/ColumbaAppTests/MessageStoreTests.swift index 3d0d2fb9..d470875c 100644 --- a/Tests/ColumbaAppTests/MessageStoreTests.swift +++ b/Tests/ColumbaAppTests/MessageStoreTests.swift @@ -21,6 +21,9 @@ import XCTest import GRDB import RNSAPI +#if os(iOS) +import UIKit +#endif @testable import ColumbaApp final class MessageStoreTests: XCTestCase { @@ -424,6 +427,106 @@ final class MessageStoreTests: XCTestCase { } } +// GRDB 6.29 observes suspension only on NotificationCenter.default, with no +// pool-specific object filtering. Keep these process-wide transitions on the +// main actor and always restore resumed access, even when an assertion fails. +@MainActor +final class MessageRepositorySuspensionTests: XCTestCase { + private func temporaryDatabaseURL() throws -> URL { + let directory = FileManager.default.temporaryDirectory + .appendingPathComponent("columba-repository-suspension-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + return directory.appendingPathComponent("lxmf-swift.db") + } + + private func assertAnnouncedNameWriteIsSuspended( + _ repository: MessageRepository, + destination: Data, + file: StaticString = #filePath, + line: UInt = #line + ) async throws { + do { + _ = try await repository.applyAnnouncedDisplayName( + destination, + displayName: "Announced Peer" + ) + XCTFail("A suspended repository must reject its announced-name write", file: file, line: line) + } catch let error as DatabaseError { + XCTAssertTrue( + error.resultCode == .SQLITE_ABORT || error.resultCode == .SQLITE_INTERRUPT, + "Expected a suspension error, received \(error)", + file: file, + line: line + ) + } + } + + func testAnnouncedNameWriteAbortsWhileSuspendedAndResumesWithoutOverwritingNickname() async throws { + let databaseURL = try temporaryDatabaseURL() + defer { try? FileManager.default.removeItem(at: databaseURL.deletingLastPathComponent()) } + let center = NotificationCenter.default + defer { center.post(name: Database.resumeNotification, object: nil) } + let repository = try MessageRepository(grdbPath: databaseURL.path) + let destination = Data([0x05, 0xc5, 0x7e, 0x42] + Array(repeating: 0xaa, count: 12)) + let customDestination = Data(repeating: 0xbb, count: 16) + let fallbackName = "Peer 05c57e42" + try await repository.ensureConversation(destination, displayName: fallbackName) + try await repository.ensureConversation(customDestination, displayName: nil) + try await repository.updateDisplayName(customDestination, displayName: "My Server") + + center.post(name: Database.suspendNotification, object: nil) + try await assertAnnouncedNameWriteIsSuspended(repository, destination: destination) + center.post(name: Database.resumeNotification, object: nil) + + let afterAbort = try await repository.fetchConversation(destination) + XCTAssertEqual(try XCTUnwrap(afterAbort).displayName, fallbackName, + "The rejected write must not have committed") + let applied = try await repository.applyAnnouncedDisplayName(destination, displayName: "Announced Peer") + XCTAssertTrue(applied) + let afterResume = try await repository.fetchConversation(destination) + XCTAssertEqual(try XCTUnwrap(afterResume).displayName, "Announced Peer") + + let customApplied = try await repository.applyAnnouncedDisplayName( + customDestination, + displayName: "Announced Replacement" + ) + XCTAssertFalse(customApplied) + let customConversation = try await repository.fetchConversation(customDestination) + XCTAssertEqual(try XCTUnwrap(customConversation).displayName, "My Server") + } + + #if os(iOS) + func testApplicationBackgroundNotificationSuspendsRealRepositoryUntilForeground() async throws { + let databaseURL = try temporaryDatabaseURL() + defer { try? FileManager.default.removeItem(at: databaseURL.deletingLastPathComponent()) } + let center = NotificationCenter.default + defer { + center.post(name: UIApplication.willEnterForegroundNotification, object: UIApplication.shared) + center.post(name: Database.resumeNotification, object: nil) + } + let repository = try MessageRepository(grdbPath: databaseURL.path) + let destination = Data([0x05, 0xc5, 0x7e, 0x42] + Array(repeating: 0xcc, count: 12)) + let fallbackName = "Peer 05c57e42" + try await repository.ensureConversation(destination, displayName: fallbackName) + + // The hosted app installs the real process lifecycle observers at init. + // Do not issue a GRDB suspension directly: missing UIKit-to-GRDB wiring + // must fail this regression even if the pools already opt into GRDB. + center.post(name: UIApplication.didEnterBackgroundNotification, object: UIApplication.shared) + try await assertAnnouncedNameWriteIsSuspended(repository, destination: destination) + center.post(name: UIApplication.willEnterForegroundNotification, object: UIApplication.shared) + + let afterAbort = try await repository.fetchConversation(destination) + XCTAssertEqual(try XCTUnwrap(afterAbort).displayName, fallbackName, + "The backgrounded write must not have committed") + let applied = try await repository.applyAnnouncedDisplayName(destination, displayName: "Announced Peer") + XCTAssertTrue(applied) + let afterForeground = try await repository.fetchConversation(destination) + XCTAssertEqual(try XCTUnwrap(afterForeground).displayName, "Announced Peer") + } + #endif +} + // MARK: - Schema introspection helpers (test-only) // // The store's `dbPool` is private and not reachable from a test extension, so From 38869e62346e953f923616f16ec94073a2d8c6df Mon Sep 17 00:00:00 2001 From: torlando-tech <239676438+torlando-tech@users.noreply.github.com> Date: Tue, 27 Oct 2026 01:40:54 +0000 Subject: [PATCH 2/2] fix: protect background call-history writes with bounded execution grants --- ARCHITECTURE.md | 2 + .../Services/BackgroundPropagationSync.swift | 52 +++++++++++ Sources/ColumbaApp/Services/CallManager.swift | 26 +++++- .../CallManagerCallKitTests.swift | 91 +++++++++++++++++++ 4 files changed, 166 insertions(+), 5 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 9b1dee49..9ae372d5 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -38,6 +38,8 @@ App-side GRDB pools opt into explicit suspension notifications. `DatabaseSuspens Background-task deliveries own separate database-access grants, including deliveries retained before the handler is ready. Completion releases a grant before reporting task completion; expiration revokes it before waiting for cancellation cleanup. Overlapping grants cannot suspend each other's work, and ending a grant cannot suspend a foreground app. Startup waits for permitted access immediately before opening new repository pools, because a pool cannot observe a suspension notification sent before it existed. Interrupted writes retain GRDB's error semantics; the lifecycle bridge does not silently report success or retry them while suspended. +Call-history writes have a separate bounded UIKit background task and matching database-access token, acquired synchronously before joining the FIFO write chain. This covers insertion, connection milestones, and final outcomes while background audio keeps a call alive. Completion releases database access before ending the system task; expiration revokes access immediately, and an expired queued write does not start. A denied system grant never resumes the database. + The persisted propagation configuration is restored before recovering outbox sends when iOS starts the extension without the app. Live relay selection, sync/cancellation, discovery, transfer limits, and peer identity retention cross typed commands through the same dispatcher in either mode. Propagation uploads and downloads share the router's outbound relay link. The Python sync bridge waits for a pre-existing pending link within the sync deadline, then explicitly starts retrieval; it never treats the previous sync's terminal state as the current result. Already-closed links are discarded on retry, while a link that closes during the wait reports `link_failed`. Failed operations report zero received messages rather than a previous sync's count. diff --git a/Sources/ColumbaApp/Services/BackgroundPropagationSync.swift b/Sources/ColumbaApp/Services/BackgroundPropagationSync.swift index 8619ed63..49c3b66a 100644 --- a/Sources/ColumbaApp/Services/BackgroundPropagationSync.swift +++ b/Sources/ColumbaApp/Services/BackgroundPropagationSync.swift @@ -136,6 +136,58 @@ final class DatabaseSuspensionController { } } +/// Protects a queued call-history write with a bounded UIKit execution grant. +/// Audio may keep a call alive in the background, but does not itself resume +/// GRDB. Acquire before enqueueing; release before ending the system grant. +@MainActor +final class CallHistoryBackgroundTask { + typealias BeginTask = @MainActor (@escaping @Sendable () -> Void) -> UIBackgroundTaskIdentifier + + private let controller: DatabaseSuspensionController + private let endTask: @MainActor (UIBackgroundTaskIdentifier) -> Void + private var taskIdentifier: UIBackgroundTaskIdentifier = .invalid + private var accessToken: UUID? + private(set) var isActive = true + + init?( + controller: DatabaseSuspensionController? = nil, + beginTask: BeginTask = { + UIApplication.shared.beginBackgroundTask(withName: "call-history", expirationHandler: $0) + }, + endTask: @escaping @MainActor (UIBackgroundTaskIdentifier) -> Void = { + UIApplication.shared.endBackgroundTask($0) + } + ) { + self.controller = controller ?? .shared + self.endTask = endTask + let identifier = beginTask { [weak self] in + // UIKit invokes expiration handlers on the main thread. + MainActor.assumeIsolated { self?.finish() } + } + guard identifier != .invalid else { return nil } + guard isActive else { + endTask(identifier) + return nil + } + taskIdentifier = identifier + accessToken = self.controller.beginBackgroundAccess() + } + + func finish() { + guard isActive else { return } + isActive = false + if let token = accessToken { + accessToken = nil + controller.endBackgroundAccess(token) + } + if taskIdentifier != .invalid { + let identifier = taskIdentifier + taskIdentifier = .invalid + endTask(identifier) + } + } +} + /// Two system-scheduled task kinds that both trigger the SAME sync workflow. /// Both lanes use the user-configured cadence: registering both multiplies /// the grant opportunities (daytime active refresh vs charging/overnight diff --git a/Sources/ColumbaApp/Services/CallManager.swift b/Sources/ColumbaApp/Services/CallManager.swift index a357a81f..3f3b6ffb 100644 --- a/Sources/ColumbaApp/Services/CallManager.swift +++ b/Sources/ColumbaApp/Services/CallManager.swift @@ -195,7 +195,8 @@ public final class CallManager { // is discarded — nothing from it is still pending, because every // terminal path (ended / failed / reset) finalizes before clearing. let repo = callHistoryRepository - historyWriteChain = Task { + historyWriteChain = nil + enqueueHistoryWrite { guard let repo else { return } try? await repo.insertAttempt( callAttemptId: id, localIdentityHash: hex, @@ -215,10 +216,25 @@ public final class CallManager { /// caller (a history write must never block a call); a nil repository /// (Model B / pre-wiring) is a silent no-op. private func enqueueHistoryWrite(_ body: @escaping @Sendable () async -> Void) { - if let existing = historyWriteChain { - historyWriteChain = Task { await existing.value; await body() } - } else { - historyWriteChain = Task { await body() } + guard callHistoryRepository != nil else { return } + #if os(iOS) + // Acquire synchronously, before the queued task yields or audio stops. + // The system grant bounds database access even if an earlier write stalls. + guard let backgroundTask = CallHistoryBackgroundTask() else { + logger.warning("[CALL] history write skipped: background execution unavailable") + return + } + #endif + let previous = historyWriteChain + historyWriteChain = Task { + #if os(iOS) + defer { backgroundTask.finish() } + #endif + await previous?.value + #if os(iOS) + guard backgroundTask.isActive else { return } + #endif + await body() } } diff --git a/Tests/ColumbaAppTests/CallManagerCallKitTests.swift b/Tests/ColumbaAppTests/CallManagerCallKitTests.swift index 8d4feeb6..d1f7d5be 100644 --- a/Tests/ColumbaAppTests/CallManagerCallKitTests.swift +++ b/Tests/ColumbaAppTests/CallManagerCallKitTests.swift @@ -14,6 +14,8 @@ import CallKit import Foundation +import GRDB +import UIKit import RNSAPI import XCTest @@ -170,6 +172,95 @@ final class CallManagerCallKitTests: XCTestCase { // MARK: - Call-history lifecycle finalization (issue #167) + func testBackgroundCallFailurePersistsOutcomeAndReleasesDatabaseAccess() async throws { + let directory = FileManager.default.temporaryDirectory + .appendingPathComponent("background-call-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + let repo = try CallHistoryRepository(grdbPath: directory.appendingPathComponent("history.db").path) + let manager = CallManager() + manager.callKitManager = MockCallKitReporter() + manager.callHistoryRepository = repo + let localHash = String(repeating: "b", count: 32) + manager.localIdentityHashHex = localHash + let center = NotificationCenter.default + defer { + center.post(name: UIApplication.willEnterForegroundNotification, object: UIApplication.shared) + center.post(name: Database.resumeNotification, object: nil) + } + + center.post(name: UIApplication.didEnterBackgroundNotification, object: UIApplication.shared) + // A background audio callback can fail a call after the UI has gone + // away. Both insertion and terminal persistence must be protected. + manager.beginCallAttempt(direction: .outgoing, peerDisplayName: "Peer") + let attemptID = try XCTUnwrap(manager.currentCallAttemptId) + manager.failOutgoingCall("Peer disconnected") + await manager.shutdown() // drains the real FIFO history-write chain + + do { + try await repo.recordEnd(attemptID, at: Date(), outcome: .failed) + XCTFail("Completed history writes must release their database access grants") + } catch let error as DatabaseError { + XCTAssertTrue(error.resultCode == .SQLITE_ABORT || error.resultCode == .SQLITE_INTERRUPT) + } + center.post(name: UIApplication.willEnterForegroundNotification, object: UIApplication.shared) + let record = try await repo.fetchRecord(attemptID, localIdentityHash: localHash) + let final = try XCTUnwrap(record, "Background call history must not be lost") + XCTAssertNotNil(final.endedAt) + XCTAssertEqual(final.outcome, .failed) + await repo.close() + try FileManager.default.removeItem(at: directory) + } + + func testCallHistoryGrantDenialAndExpirationDoNotLeaveDatabaseResumed() async throws { + let directory = FileManager.default.temporaryDirectory + .appendingPathComponent("call-grant-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + let repo = try CallHistoryRepository(grdbPath: directory.appendingPathComponent("history.db").path) + let controller = DatabaseSuspensionController(isForeground: false) + defer { controller.setForeground(true) } + let denied = CallHistoryBackgroundTask(controller: controller, beginTask: { _ in .invalid }) + XCTAssertNil(denied) + do { + try await repo.insertAttempt( + callAttemptId: "denied", localIdentityHash: "local", remoteIdentityHash: "remote", + direction: .outgoing, peerDisplayNameSnapshot: nil, codecProfileCode: 0, attemptedAt: Date() + ) + XCTFail("A denied UIKit grant must not resume the database") + } catch let error as DatabaseError { + XCTAssertTrue(error.resultCode == .SQLITE_ABORT || error.resultCode == .SQLITE_INTERRUPT) + } + + var expiration: (@Sendable () -> Void)? + var ended: [UIBackgroundTaskIdentifier] = [] + let identifier = UIBackgroundTaskIdentifier(rawValue: 42) + let grant = try XCTUnwrap(CallHistoryBackgroundTask( + controller: controller, + beginTask: { expiration = $0; return identifier }, + endTask: { ended.append($0) } + )) + try await repo.insertAttempt( + callAttemptId: "active", localIdentityHash: "local", remoteIdentityHash: "remote", + direction: .outgoing, peerDisplayNameSnapshot: nil, codecProfileCode: 0, attemptedAt: Date() + ) + try XCTUnwrap(expiration)() + XCTAssertFalse(grant.isActive) + do { + try await repo.recordEnd("active", at: Date(), outcome: .failed) + XCTFail("Expiration must revoke database access before work finishes") + } catch let error as DatabaseError { + XCTAssertTrue(error.resultCode == .SQLITE_ABORT || error.resultCode == .SQLITE_INTERRUPT) + } + grant.finish() // late completion must not end the UIKit grant twice + XCTAssertEqual(ended, [identifier]) + controller.setForeground(true) + let active = try await repo.fetchRecord("active", localIdentityHash: "local") + XCTAssertNil(try XCTUnwrap(active).endedAt) + let missing = try await repo.fetchRecord("denied", localIdentityHash: "local") + XCTAssertNil(missing) + await repo.close() + try FileManager.default.removeItem(at: directory) + } + /// A CallKit provider reset while a call is active must FINALIZE the /// history attempt (end time + outcome) — otherwise the row is stranded /// with no end and the Voice list shows a dead call "in progress" forever.