mirror of
https://github.com/permissionlesstech/bitchat.git
synced 2026-07-24 22:45:19 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
@@ -150,10 +150,6 @@ final class SecureIdentityStateManager: SecureIdentityStateManagerProtocol {
|
|||||||
|
|
||||||
// Thread safety
|
// Thread safety
|
||||||
private let queue = DispatchQueue(label: "bitchat.identity.state", attributes: .concurrent)
|
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<UInt8>()
|
|
||||||
|
|
||||||
// Debouncing for keychain saves.
|
// Debouncing for keychain saves.
|
||||||
// A DispatchSourceTimer on `queue` is used rather than Timer.scheduledTimer:
|
// A DispatchSourceTimer on `queue` is used rather than Timer.scheduledTimer:
|
||||||
@@ -174,7 +170,6 @@ final class SecureIdentityStateManager: SecureIdentityStateManagerProtocol {
|
|||||||
|
|
||||||
init(_ keychain: KeychainManagerProtocol) {
|
init(_ keychain: KeychainManagerProtocol) {
|
||||||
self.keychain = keychain
|
self.keychain = keychain
|
||||||
queue.setSpecific(key: Self.queueSpecificKey, value: 1)
|
|
||||||
|
|
||||||
// Retrieve (or, only on genuine first run, generate) the cache
|
// Retrieve (or, only on genuine first run, generate) the cache
|
||||||
// encryption key. We MUST distinguish "key doesn't exist yet" from a
|
// 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
|
/// Schedules a debounced save. Always invoked on `queue` under a barrier, so
|
||||||
/// the timer/pendingSave bookkeeping is serialized with all other state.
|
/// the timer/pendingSave bookkeeping is serialized with all other state.
|
||||||
private func saveIdentityCache() {
|
private func saveIdentityCache() {
|
||||||
// Mark that we need to save
|
// Mark that we need to save
|
||||||
pendingSave = true
|
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`.
|
// Cancel any pending timer and schedule a fresh one on `queue`.
|
||||||
saveTimer?.cancel()
|
saveTimer?.cancel()
|
||||||
let timer = DispatchSource.makeTimerSource(queue: queue)
|
let timer = DispatchSource.makeTimerSource(queue: queue)
|
||||||
@@ -293,21 +305,16 @@ final class SecureIdentityStateManager: SecureIdentityStateManagerProtocol {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Force immediate save (for app termination). Runs synchronously on `queue`
|
// Force immediate save (for app termination / lifecycle events). Runs the
|
||||||
// so the write completes before the caller proceeds (e.g. app exit).
|
// 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() {
|
func forceSave() {
|
||||||
let work = {
|
saveTimer?.cancel()
|
||||||
self.saveTimer?.cancel()
|
saveTimer = nil
|
||||||
self.saveTimer = nil
|
performSave()
|
||||||
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)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// MARK: - Social Identity Management
|
// MARK: - Social Identity Management
|
||||||
|
|||||||
@@ -136,10 +136,12 @@ final class BLEService: NSObject {
|
|||||||
private var maintenanceTimer: DispatchSourceTimer? // Single timer for all maintenance tasks
|
private var maintenanceTimer: DispatchSourceTimer? // Single timer for all maintenance tasks
|
||||||
private var maintenanceCounter = 0 // Track maintenance cycles
|
private var maintenanceCounter = 0 // Track maintenance cycles
|
||||||
/// Whether real CoreBluetooth managers were initialized. When false (unit
|
/// Whether real CoreBluetooth managers were initialized. When false (unit
|
||||||
/// tests), the periodic maintenance timer is not started: it only drains BLE
|
/// tests), periodic mesh background work is not started — the maintenance
|
||||||
/// writes/notifications and re-announces, which is meaningless without
|
/// timer and the gossip-sync timers only drain BLE writes/notifications,
|
||||||
/// Bluetooth and would otherwise run its cross-queue work in-process.
|
/// re-announce, and sign/broadcast sync packets, all meaningless without
|
||||||
private var maintenanceTimerEnabled = false
|
/// 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)
|
// MARK: - Connection budget & scheduling (central role)
|
||||||
private var connectionScheduler = BLEConnectionScheduler<CBPeripheral>()
|
private var connectionScheduler = BLEConnectionScheduler<CBPeripheral>()
|
||||||
@@ -240,7 +242,7 @@ final class BLEService: NSObject {
|
|||||||
|
|
||||||
// Single maintenance timer for all periodic tasks (dispatch-based for
|
// Single maintenance timer for all periodic tasks (dispatch-based for
|
||||||
// determinism). Only run it when real Bluetooth managers exist.
|
// determinism). Only run it when real Bluetooth managers exist.
|
||||||
maintenanceTimerEnabled = initializeBluetoothManagers
|
meshBackgroundEnabled = initializeBluetoothManagers
|
||||||
startMaintenanceTimer()
|
startMaintenanceTimer()
|
||||||
|
|
||||||
// Publish initial empty state
|
// Publish initial empty state
|
||||||
@@ -271,7 +273,12 @@ final class BLEService: NSObject {
|
|||||||
|
|
||||||
let manager = GossipSyncManager(myPeerID: myPeerID, config: config, requestSyncManager: requestSyncManager)
|
let manager = GossipSyncManager(myPeerID: myPeerID, config: config, requestSyncManager: requestSyncManager)
|
||||||
manager.delegate = self
|
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
|
gossipSyncManager = manager
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -439,7 +446,7 @@ final class BLEService: NSObject {
|
|||||||
/// `startServices()` — the latter matters after a panic reset, where
|
/// `startServices()` — the latter matters after a panic reset, where
|
||||||
/// `stopServices()` cancels and nils the timer.
|
/// `stopServices()` cancels and nils the timer.
|
||||||
private func startMaintenanceTimer() {
|
private func startMaintenanceTimer() {
|
||||||
guard maintenanceTimerEnabled, maintenanceTimer == nil else { return }
|
guard meshBackgroundEnabled, maintenanceTimer == nil else { return }
|
||||||
let timer = DispatchSource.makeTimerSource(queue: bleQueue)
|
let timer = DispatchSource.makeTimerSource(queue: bleQueue)
|
||||||
timer.schedule(deadline: .now() + TransportConfig.bleMaintenanceInterval,
|
timer.schedule(deadline: .now() + TransportConfig.bleMaintenanceInterval,
|
||||||
repeating: TransportConfig.bleMaintenanceInterval,
|
repeating: TransportConfig.bleMaintenanceInterval,
|
||||||
|
|||||||
@@ -1180,19 +1180,25 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele
|
|||||||
// No need to force UserDefaults synchronization
|
// No need to force UserDefaults synchronization
|
||||||
|
|
||||||
// Reinitialize Nostr with new identity
|
// Reinitialize Nostr with new identity
|
||||||
// This will generate new Nostr keys derived from new Noise keys
|
// This will generate new Nostr keys derived from new Noise keys.
|
||||||
Task { @MainActor in
|
// Skipped under tests: connecting the shared relay singleton starts
|
||||||
// Small delay to ensure cleanup completes
|
// real network/reconnect work that never completes and would keep the
|
||||||
try? await Task.sleep(nanoseconds: TransportConfig.uiAsyncShortSleepNs) // 0.1 seconds
|
// 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
|
// Reinitialize Nostr relay manager with new identity. Reuse the
|
||||||
// shared singleton — every other component (NostrTransport, geohash
|
// shared singleton — every other component (NostrTransport, geohash
|
||||||
// subscriptions, AppRuntime observers) is bound to `.shared`, so
|
// subscriptions, AppRuntime observers) is bound to `.shared`, so
|
||||||
// creating a fresh instance here would split relay state and leave
|
// creating a fresh instance here would split relay state and leave
|
||||||
// sends running against a disconnected manager.
|
// sends running against a disconnected manager.
|
||||||
nostrRelayManager = NostrRelayManager.shared
|
nostrRelayManager = NostrRelayManager.shared
|
||||||
setupNostrMessageHandling()
|
setupNostrMessageHandling()
|
||||||
nostrRelayManager?.connect()
|
nostrRelayManager?.connect()
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Delete ALL media files (incoming and outgoing) in background
|
// Delete ALL media files (incoming and outgoing) in background
|
||||||
|
|||||||
Reference in New Issue
Block a user