From 822679e75e921c7941a39ef6b40cb88a9fe68fe3 Mon Sep 17 00:00:00 2001 From: krishrathi1 Date: Sat, 29 Aug 2026 16:34:15 +0530 Subject: [PATCH] Use monotonic attemptToken for BLE connect timeout & add unit tests --- bitchat/Services/BLE/BLELinkStateStore.swift | 26 ++++- bitchat/Services/BLE/BLERadioController.swift | 43 ++++---- .../BLEConnectTimeoutPolicyTests.swift | 97 +++++++++++++++++++ 3 files changed, 138 insertions(+), 28 deletions(-) create mode 100644 bitchatTests/Services/BLEConnectTimeoutPolicyTests.swift diff --git a/bitchat/Services/BLE/BLELinkStateStore.swift b/bitchat/Services/BLE/BLELinkStateStore.swift index 79440a64..7bed020b 100644 --- a/bitchat/Services/BLE/BLELinkStateStore.swift +++ b/bitchat/Services/BLE/BLELinkStateStore.swift @@ -8,6 +8,7 @@ struct BLEPeripheralLinkState { var isConnecting: Bool var isConnected: Bool var lastConnectionAttempt: Date? + var attemptToken: UInt64 = 0 /// When didConnect last fired for this link. Nil for links restored /// already-connected (their connect predates this process), which is /// exactly the signal redundant-link consolidation needs: a restored @@ -17,6 +18,22 @@ struct BLEPeripheralLinkState { var assembler: NotificationStreamAssembler } +struct BLEConnectTimeoutPolicy { + /// Pure decision: given a captured attempt token and current link state, + /// returns true if the timeout closure should act on this peripheral attempt. + static func shouldExecuteConnectTimeout( + capturedAttemptToken: UInt64, + state: BLEPeripheralLinkState?, + isPeripheralConnected: Bool + ) -> Bool { + guard let state = state else { return false } + guard state.isConnecting && !state.isConnected else { return false } + guard !isPeripheralConnected else { return false } + guard state.attemptToken == capturedAttemptToken else { return false } + return true + } +} + struct BLEDirectLinkState: Equatable { let hasPeripheral: Bool let hasCentral: Bool @@ -104,7 +121,10 @@ final class BLELinkStateStore { return state } - func beginConnecting(to peripheral: CBPeripheral, at date: Date) { + @discardableResult + func beginConnecting(to peripheral: CBPeripheral, at date: Date = Date()) -> UInt64 { + let peripheralID = peripheral.identifier.uuidString + let nextAttemptToken = (peripherals[peripheralID]?.attemptToken ?? 0) + 1 setPeripheralState( BLEPeripheralLinkState( peripheral: peripheral, @@ -112,10 +132,12 @@ final class BLELinkStateStore { isConnecting: true, isConnected: false, lastConnectionAttempt: date, + attemptToken: nextAttemptToken, assembler: NotificationStreamAssembler() ), - for: peripheral.identifier.uuidString + for: peripheralID ) + return nextAttemptToken } func markConnected(_ peripheral: CBPeripheral, at now: Date = Date()) { diff --git a/bitchat/Services/BLE/BLERadioController.swift b/bitchat/Services/BLE/BLERadioController.swift index 7f5b0c09..82026493 100644 --- a/bitchat/Services/BLE/BLERadioController.swift +++ b/bitchat/Services/BLE/BLERadioController.swift @@ -231,14 +231,9 @@ final class BLERadioController { guard delegate?.radioIsPanicSuspended() == false else { return } let peripheral = candidate.peripheral let peripheralID = candidate.peripheralID - // Captured so the timeout closure below can tell this specific - // attempt apart from a later one: linkStateStore.beginConnecting - // overwrites lastConnectionAttempt on every call for this - // peripheralID (disconnect -> reconnect included), but nothing - // previously distinguished which attempt a given timeout closure was - // scheduled for. - let attemptStartedAt = Date() - linkStateStore.beginConnecting(to: peripheral, at: attemptStartedAt) + // Captured monotonically increasing attempt token so the timeout closure + // below can tell this specific attempt apart from a later one. + let attemptToken = linkStateStore.beginConnecting(to: peripheral, at: Date()) peripheral.delegate = peripheralDelegate let options: [String: Any] = [ CBConnectPeripheralOptionNotifyOnConnectionKey: true, @@ -250,26 +245,22 @@ final class BLERadioController { SecureLogger.debug("\(logPrefix): \(candidate.name) [RSSI:\(candidate.rssi)]", category: .session) queue.asyncAfter(deadline: .now() + TransportConfig.bleConnectTimeoutSeconds) { [weak self] in - guard let self, - let state = self.linkStateStore.state(forPeripheralID: peripheralID), - state.isConnecting && !state.isConnected else { return } + guard let self else { return } + let state = self.linkStateStore.state(forPeripheralID: peripheralID) // A disconnect + immediate reconnect between this attempt and now - // starts a new attempt with a fresh lastConnectionAttempt (via - // beginConnecting or armPendingBackgroundConnects, which sets it - // nil). Without this check, this now-stale timeout would cancel - // that newer, still-live attempt out from under it and apply a - // connection-timeout penalty to a peer that never actually timed - // out — reproducible any time a peer drops and reconnects while - // this timer is still armed (a few seconds of marginal signal is - // enough). - guard state.lastConnectionAttempt == attemptStartedAt else { - SecureLogger.debug("⏱️ Timeout fired for a superseded connection attempt, ignoring: \(candidate.name)", category: .session) - return - } - - guard peripheral.state != .connected else { - SecureLogger.debug("⏱️ Timeout fired but peripheral already connected: \(candidate.name)", category: .session) + // starts a new attempt with an incremented attemptToken. Without + // this check, this now-stale timeout would cancel that newer, + // still-live attempt out from under it and apply a + // connection-timeout penalty to a peer that never actually timed out. + guard BLEConnectTimeoutPolicy.shouldExecuteConnectTimeout( + capturedAttemptToken: attemptToken, + state: state, + isPeripheralConnected: peripheral.state == .connected + ) else { + if let state, state.attemptToken != attemptToken { + SecureLogger.debug("⏱️ Timeout fired for a superseded connection attempt, ignoring: \(candidate.name)", category: .session) + } return } diff --git a/bitchatTests/Services/BLEConnectTimeoutPolicyTests.swift b/bitchatTests/Services/BLEConnectTimeoutPolicyTests.swift new file mode 100644 index 00000000..05bd2c4e --- /dev/null +++ b/bitchatTests/Services/BLEConnectTimeoutPolicyTests.swift @@ -0,0 +1,97 @@ +import Foundation +import CoreBluetooth +import Testing +@testable import bitchat + +struct BLEConnectTimeoutPolicyTests { + @Test + func validConnectingAttemptTriggersTimeout() { + let token: UInt64 = 42 + + // When state is nil -> false + #expect(!BLEConnectTimeoutPolicy.shouldExecuteConnectTimeout( + capturedAttemptToken: token, + state: nil, + isPeripheralConnected: false + )) + } + + @Test + func supersededAttemptTokenIgnoresTimeout() { + let capturedToken: UInt64 = 1 + let currentToken: UInt64 = 2 + + let mockState = BLEPeripheralLinkState( + peripheral: CBPeripheralMock.create(), + characteristic: nil, + isConnecting: true, + isConnected: false, + lastConnectionAttempt: Date(), + attemptToken: currentToken, + assembler: NotificationStreamAssembler() + ) + + let result = BLEConnectTimeoutPolicy.shouldExecuteConnectTimeout( + capturedAttemptToken: capturedToken, + state: mockState, + isPeripheralConnected: false + ) + + #expect(!result) + } + + @Test + func matchingAttemptTokenExecutesTimeout() { + let token: UInt64 = 5 + + let mockState = BLEPeripheralLinkState( + peripheral: CBPeripheralMock.create(), + characteristic: nil, + isConnecting: true, + isConnected: false, + lastConnectionAttempt: Date(), + attemptToken: token, + assembler: NotificationStreamAssembler() + ) + + let result = BLEConnectTimeoutPolicy.shouldExecuteConnectTimeout( + capturedAttemptToken: token, + state: mockState, + isPeripheralConnected: false + ) + + #expect(result) + } + + @Test + func connectedStateIgnoresTimeout() { + let token: UInt64 = 5 + + let mockState = BLEPeripheralLinkState( + peripheral: CBPeripheralMock.create(), + characteristic: nil, + isConnecting: false, + isConnected: true, + lastConnectionAttempt: Date(), + attemptToken: token, + assembler: NotificationStreamAssembler() + ) + + let result = BLEConnectTimeoutPolicy.shouldExecuteConnectTimeout( + capturedAttemptToken: token, + state: mockState, + isPeripheralConnected: true + ) + + #expect(!result) + } +} + +// Helper mock CBPeripheral for tests +private final class CBPeripheralMock { + static func create() -> CBPeripheral { + let dummy = UnsafeMutableRawPointer.allocate(byteCount: 256, alignment: 16) + dummy.initializeMemory(as: UInt8.self, repeating: 0) + return Unmanaged.fromOpaque(dummy).takeUnretainedValue() + } +}