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.
This commit is contained in:
krishrathi1 2026-08-16 13:21:43 +05:30
parent 9b84b36122
commit 7be9aa9de4

View File

@ -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