From 7be9aa9de49d1ab9b6eee2cc85ddf47adca25934 Mon Sep 17 00:00:00 2001 From: krishrathi1 Date: Sun, 16 Aug 2026 13:21:43 +0530 Subject: [PATCH 1/4] Ignore a connect timeout superseded by a newer attempt on the same peripheral beginCentralConnection's timeout closure only checked isConnecting && !isConnected before tearing down a link -- it never checked whether the attempt it was scheduled for was still the current one. A disconnect followed by a reconnect while the original timeout is still armed (ordinary BLE usage: walking behind an obstacle, a pocket, a crowd) starts a fresh attempt via beginConnecting, which is still mid-flight and healthy when the stale timer from the earlier attempt fires. That timer finds isConnecting == true (true for the new attempt) and cancels the connection out from under it, then applies recordConnectionTimeout's scoring penalty and discovery-ignore cooldown to a peer that never actually timed out -- degrading mesh connectivity for a healthy, in-range peer, repeatably, with nothing a user would recognize as a bug. Captures the Date passed to beginConnecting for this specific attempt and compares it against the link state's lastConnectionAttempt inside the timeout closure; beginConnecting (and armPendingBackgroundConnects, which sets it nil) both overwrite that field on every new attempt for the same peripheralID, so a mismatch means a newer attempt has since started and this timer's own attempt is no longer the live one. No Xcode/Swift toolchain available here, verified by tracing the state machine rather than compiling. No automated regression test: this class has no existing test coverage (grepped bitchatTests -- only its BLEConnectionScheduler dependency is unit-tested in isolation, not BLERadioController's own connect/timeout logic), and CoreBluetooth's CBCentralManager/CBPeripheral have no public initializers, so a from-scratch mock harness would itself be unverified without a compiler. The fix is 12 lines added around one guard clause and reviews by inspection; flagging the coverage gap rather than shipping an unverified test alongside an unverified fix. --- bitchat/Services/BLE/BLERadioController.swift | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/bitchat/Services/BLE/BLERadioController.swift b/bitchat/Services/BLE/BLERadioController.swift index cf9d6c2d..7f5b0c09 100644 --- a/bitchat/Services/BLE/BLERadioController.swift +++ b/bitchat/Services/BLE/BLERadioController.swift @@ -231,7 +231,14 @@ final class BLERadioController { guard delegate?.radioIsPanicSuspended() == false else { return } let peripheral = candidate.peripheral let peripheralID = candidate.peripheralID - linkStateStore.beginConnecting(to: peripheral, at: Date()) + // 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) peripheral.delegate = peripheralDelegate let options: [String: Any] = [ CBConnectPeripheralOptionNotifyOnConnectionKey: true, @@ -247,6 +254,20 @@ final class BLERadioController { let state = self.linkStateStore.state(forPeripheralID: peripheralID), state.isConnecting && !state.isConnected else { return } + // 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) return From 822679e75e921c7941a39ef6b40cb88a9fe68fe3 Mon Sep 17 00:00:00 2001 From: krishrathi1 Date: Sat, 29 Aug 2026 16:34:15 +0530 Subject: [PATCH 2/4] 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() + } +} From 55d7cd460ff85cc317ffcd7fea160a7e300dbb77 Mon Sep 17 00:00:00 2001 From: krishrathi1 Date: Sat, 29 Aug 2026 16:41:01 +0530 Subject: [PATCH 3/4] Fix Swift compiler error in BLEConnectTimeoutPolicyTests (missing count parameter) --- bitchatTests/Services/BLEConnectTimeoutPolicyTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bitchatTests/Services/BLEConnectTimeoutPolicyTests.swift b/bitchatTests/Services/BLEConnectTimeoutPolicyTests.swift index 05bd2c4e..10bc3145 100644 --- a/bitchatTests/Services/BLEConnectTimeoutPolicyTests.swift +++ b/bitchatTests/Services/BLEConnectTimeoutPolicyTests.swift @@ -91,7 +91,7 @@ struct BLEConnectTimeoutPolicyTests { private final class CBPeripheralMock { static func create() -> CBPeripheral { let dummy = UnsafeMutableRawPointer.allocate(byteCount: 256, alignment: 16) - dummy.initializeMemory(as: UInt8.self, repeating: 0) + dummy.initializeMemory(as: UInt8.self, repeating: 0, count: 256) return Unmanaged.fromOpaque(dummy).takeUnretainedValue() } } From 929a86ce2ba882d59209783335291b832e5dd153 Mon Sep 17 00:00:00 2001 From: krishrathi1 Date: Sat, 29 Aug 2026 23:01:53 +0530 Subject: [PATCH 4/4] Refactor BLEConnectTimeoutPolicy to use primitive attributes and eliminate CBPeripheral test mock crash --- bitchat/Services/BLE/BLELinkStateStore.swift | 11 +-- bitchat/Services/BLE/BLERadioController.swift | 13 ++-- .../BLEConnectTimeoutPolicyTests.swift | 71 ++++++------------- 3 files changed, 34 insertions(+), 61 deletions(-) diff --git a/bitchat/Services/BLE/BLELinkStateStore.swift b/bitchat/Services/BLE/BLELinkStateStore.swift index 7bed020b..c286ffcf 100644 --- a/bitchat/Services/BLE/BLELinkStateStore.swift +++ b/bitchat/Services/BLE/BLELinkStateStore.swift @@ -19,17 +19,18 @@ struct BLEPeripheralLinkState { } struct BLEConnectTimeoutPolicy { - /// Pure decision: given a captured attempt token and current link state, + /// Pure decision: given a captured attempt token and current link state attributes, /// returns true if the timeout closure should act on this peripheral attempt. static func shouldExecuteConnectTimeout( capturedAttemptToken: UInt64, - state: BLEPeripheralLinkState?, + isConnecting: Bool, + isConnected: Bool, + currentAttemptToken: UInt64, isPeripheralConnected: Bool ) -> Bool { - guard let state = state else { return false } - guard state.isConnecting && !state.isConnected else { return false } + guard isConnecting && !isConnected else { return false } guard !isPeripheralConnected else { return false } - guard state.attemptToken == capturedAttemptToken else { return false } + guard currentAttemptToken == capturedAttemptToken else { return false } return true } } diff --git a/bitchat/Services/BLE/BLERadioController.swift b/bitchat/Services/BLE/BLERadioController.swift index 82026493..880622fe 100644 --- a/bitchat/Services/BLE/BLERadioController.swift +++ b/bitchat/Services/BLE/BLERadioController.swift @@ -253,11 +253,14 @@ final class BLERadioController { // 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 { + guard let state, + BLEConnectTimeoutPolicy.shouldExecuteConnectTimeout( + capturedAttemptToken: attemptToken, + isConnecting: state.isConnecting, + isConnected: state.isConnected, + currentAttemptToken: state.attemptToken, + 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) } diff --git a/bitchatTests/Services/BLEConnectTimeoutPolicyTests.swift b/bitchatTests/Services/BLEConnectTimeoutPolicyTests.swift index 10bc3145..50f0a8dd 100644 --- a/bitchatTests/Services/BLEConnectTimeoutPolicyTests.swift +++ b/bitchatTests/Services/BLEConnectTimeoutPolicyTests.swift @@ -1,39 +1,18 @@ 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, + isConnecting: true, + isConnected: false, + currentAttemptToken: currentToken, isPeripheralConnected: false ) @@ -44,19 +23,11 @@ struct BLEConnectTimeoutPolicyTests { 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, + isConnecting: true, + isConnected: false, + currentAttemptToken: token, isPeripheralConnected: false ) @@ -67,31 +38,29 @@ struct BLEConnectTimeoutPolicyTests { func connectedStateIgnoresTimeout() { let token: UInt64 = 5 - let mockState = BLEPeripheralLinkState( - peripheral: CBPeripheralMock.create(), - characteristic: nil, + let result = BLEConnectTimeoutPolicy.shouldExecuteConnectTimeout( + capturedAttemptToken: token, isConnecting: false, isConnected: true, - lastConnectionAttempt: Date(), - attemptToken: token, - assembler: NotificationStreamAssembler() + currentAttemptToken: token, + isPeripheralConnected: true ) + #expect(!result) + } + + @Test + func peripheralConnectedStateIgnoresTimeout() { + let token: UInt64 = 5 + let result = BLEConnectTimeoutPolicy.shouldExecuteConnectTimeout( capturedAttemptToken: token, - state: mockState, + isConnecting: true, + isConnected: false, + currentAttemptToken: token, 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, count: 256) - return Unmanaged.fromOpaque(dummy).takeUnretainedValue() - } -}