From 7be9aa9de49d1ab9b6eee2cc85ddf47adca25934 Mon Sep 17 00:00:00 2001 From: krishrathi1 Date: Sun, 16 Aug 2026 13:21:43 +0530 Subject: [PATCH] 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