Fix BLE identity state races (#1428)

Converts BLEAnnounceThrottle to a lock-backed class and moves myPeerID/myPeerIDData/myNickname into a lock-backed BLELocalIdentityStateStore snapshot, so announces can never observe a split peer-ID/wire-ID during panic rotation. Serializes sendAnnounce onto the messageQueue barrier and moves the panic-reset pendingNoiseSessionQueues clear onto collectionsQueue (the queue every other mutation site uses).

Rebased onto main after #1431/#1432; integrates with #1431's panic-suspend structure (guard retained in both the outer sendAnnounce and the deferred worker).
This commit is contained in:
jack
2026-07-26 11:38:16 +02:00
committed by GitHub
parent b59ad97dd6
commit d326cecb63
5 changed files with 201 additions and 42 deletions
+18 -9
View File
@@ -1,6 +1,13 @@
import Foundation
struct BLEAnnounceThrottle {
/// Thread-safe announce admission state.
///
/// Announce requests originate from the Bluetooth delegate queue, the
/// concurrent message queue, and the maintenance timer. Keeping the timestamp
/// behind a lock makes admission and maintenance snapshots atomic when those
/// request sources race.
final class BLEAnnounceThrottle: @unchecked Sendable {
private let lock = NSLock()
private var lastSent: Date
private let normalMinimumInterval: TimeInterval
private let forcedMinimumInterval: TimeInterval
@@ -16,16 +23,18 @@ struct BLEAnnounceThrottle {
}
func elapsed(since now: Date) -> TimeInterval {
now.timeIntervalSince(lastSent)
lock.withLock { now.timeIntervalSince(lastSent) }
}
mutating func shouldSend(force: Bool, now: Date) -> Bool {
let minimumInterval = force ? forcedMinimumInterval : normalMinimumInterval
guard elapsed(since: now) >= minimumInterval else {
return false
}
func shouldSend(force: Bool, now: Date) -> Bool {
lock.withLock {
let minimumInterval = force ? forcedMinimumInterval : normalMinimumInterval
guard now.timeIntervalSince(lastSent) >= minimumInterval else {
return false
}
lastSent = now
return true
lastSent = now
return true
}
}
}
@@ -0,0 +1,55 @@
import BitFoundation
import Foundation
struct BLELocalIdentitySnapshot: Equatable, Sendable {
let peerID: PeerID
let peerIDData: Data
let nickname: String
}
/// Lock-backed local identity state shared by the transport's message,
/// Bluetooth, maintenance, and main-actor entry points.
///
/// `peerID` and its binary wire representation must change as one unit during
/// panic rotation. A snapshot also gives announce construction one consistent
/// view of the nickname and identity instead of reading three independently
/// mutable properties across queues.
final class BLELocalIdentityStateStore: @unchecked Sendable {
private let lock = NSLock()
private var state: BLELocalIdentitySnapshot
init(
peerID: PeerID = PeerID(str: ""),
nickname: String = "anon"
) {
state = BLELocalIdentitySnapshot(
peerID: peerID,
peerIDData: Data(hexString: peerID.id) ?? Data(),
nickname: nickname
)
}
func snapshot() -> BLELocalIdentitySnapshot {
lock.withLock { state }
}
func setNickname(_ nickname: String) {
lock.withLock {
state = BLELocalIdentitySnapshot(
peerID: state.peerID,
peerIDData: state.peerIDData,
nickname: nickname
)
}
}
func replacePeerIdentity(with peerID: PeerID) {
lock.withLock {
state = BLELocalIdentitySnapshot(
peerID: peerID,
peerIDData: Data(hexString: peerID.id) ?? Data(),
nickname: state.nickname
)
}
}
}
+37 -29
View File
@@ -139,7 +139,7 @@ final class BLEService: NSObject {
private let incomingFileStore = BLEIncomingFileStore()
// Simple announce throttling
private var announceThrottle = BLEAnnounceThrottle()
private let announceThrottle = BLEAnnounceThrottle()
// Application state tracking (thread-safe)
#if os(iOS)
@@ -171,9 +171,7 @@ final class BLEService: NSObject {
private let identityManager: SecureIdentityStateManagerProtocol
private let keychain: KeychainManagerProtocol
private let idBridge: NostrIdentityBridge
/// Binary form of `myPeerID`; same contract mutated only inside a
/// `messageQueue` barrier via `refreshPeerIdentity()`.
private var myPeerIDData: Data = Data()
private let localIdentityState = BLELocalIdentityStateStore()
// MARK: - Advertising Privacy
// No Local Name by default for maximum privacy. No rotating alias.
@@ -514,7 +512,9 @@ final class BLEService: NSObject {
) {
gossipSyncManager?.stop()
gossipSyncManager = nil
messageQueue.sync(flags: .barrier) {
// pendingNoiseSessionQueues is owned by collectionsQueue everywhere
// else, so clear it there too rather than on messageQueue.
collectionsQueue.sync(flags: .barrier) {
pendingNoiseSessionQueues.removeAll()
}
@@ -559,7 +559,9 @@ final class BLEService: NSObject {
}
// Keep the transport silent until the application-level transaction
// has also removed its media and committed both recovery markers.
myNickname = currentNickname
// Set through the identity store directly (not setNickname(_:), which
// would force-send an announce and break that silence).
localIdentityState.setNickname(currentNickname)
messageDeduplicator.reset()
messageQueue.async(flags: .barrier) { [weak self] in
self?.selfBroadcastTracker.removeAll()
@@ -638,20 +640,17 @@ final class BLEService: NSObject {
// MARK: Identity
/// Derived from the Noise identity fingerprint; rotated only via
/// `refreshPeerIdentity()` (e.g. panic reset), which performs the swap
/// inside a `messageQueue` barrier so concurrent queue work never sees a
/// half-updated identity. Externally read-only no out-of-band mutation
/// may bypass that derivation.
private(set) var myPeerID = PeerID(str: "")
/// Externally read-only; mutate via `setNickname(_:)`, which also
/// broadcasts the change to peers.
private(set) var myNickname: String = "anon"
/// Derived from the Noise identity fingerprint. Reads can originate from
/// the main actor, message queue, Bluetooth queue, and maintenance timer,
/// so all three local identity fields live in one lock-backed snapshot.
var myPeerID: PeerID { localIdentityState.snapshot().peerID }
var myNickname: String { localIdentityState.snapshot().nickname }
private var myPeerIDData: Data { localIdentityState.snapshot().peerIDData }
/// Sole mutator for `myNickname`: updates the stored value and force-sends
/// an announce so peers learn the new name.
func setNickname(_ nickname: String) {
self.myNickname = nickname
localIdentityState.setNickname(nickname)
// Send announce to notify peers of nickname change (force send)
sendAnnounce(forceSend: true)
}
@@ -709,10 +708,11 @@ final class BLEService: NSObject {
}
func stopServices() {
let localIdentity = localIdentityState.snapshot()
// Send leave message synchronously to ensure delivery
var leavePacket = BitchatPacket(
type: MessageType.leave.rawValue,
senderID: myPeerIDData,
senderID: localIdentity.peerIDData,
recipientID: nil,
timestamp: UInt64(Date().timeIntervalSince1970 * 1000),
payload: Data(),
@@ -1849,6 +1849,19 @@ final class BLEService: NSObject {
return true
}
private func sendAnnounce(forceSend: Bool = false) {
guard !isPanicSuspended else { return }
// Announce construction reads the replaceable Noise service and several
// related state snapshots. Serialize the whole operation with identity
// rotation instead of letting CoreBluetooth and maintenance callbacks
// execute it directly on their own queues.
messageQueue.async(flags: .barrier) { [weak self] in
self?.sendAnnounceNow(forceSend: forceSend)
}
}
private func sendAnnounceNow(forceSend: Bool) {
// Re-check on the serialized queue: a panic suspend may have started
// after this announce was scheduled but before it runs.
guard !isPanicSuspended else { return }
// Throttle announces to prevent flooding
if !announceThrottle.shouldSend(force: forceSend, now: Date()) {
@@ -1869,8 +1882,9 @@ final class BLEService: NSObject {
)
}
let localIdentity = localIdentityState.snapshot()
let announcement = AnnouncementPacket(
nickname: myNickname,
nickname: localIdentity.nickname,
noisePublicKey: noisePub,
signingPublicKey: signingPub,
directNeighbors: connectedPeerIDs,
@@ -1886,7 +1900,7 @@ final class BLEService: NSObject {
// Create packet with signature using the noise private key
let packet = BitchatPacket(
type: MessageType.announce.rawValue,
senderID: myPeerIDData,
senderID: localIdentity.peerIDData,
recipientID: nil,
timestamp: UInt64(Date().timeIntervalSince1970 * 1000),
payload: payload,
@@ -1900,14 +1914,7 @@ final class BLEService: NSObject {
return
}
// Call directly if on messageQueue, otherwise dispatch
if DispatchQueue.getSpecific(key: messageQueueKey) != nil {
broadcastPacket(signedPacket)
} else {
messageQueue.async { [weak self] in
self?.broadcastPacket(signedPacket)
}
}
broadcastPacket(signedPacket)
// Ensure our own announce is included in sync state
gossipSyncManager?.onPublicPacketSeen(signedPacket)
@@ -3681,8 +3688,9 @@ extension BLEService {
private func refreshPeerIdentity() {
let swap = {
let fingerprint = self.noiseService.getIdentityFingerprint()
self.myPeerID = PeerID(str: fingerprint.prefix(16))
self.myPeerIDData = Data(hexString: self.myPeerID.id) ?? Data()
self.localIdentityState.replacePeerIdentity(
with: PeerID(str: fingerprint.prefix(16))
)
self.meshTopology.reset()
}
if DispatchQueue.getSpecific(key: messageQueueKey) != nil {