mirror of
https://github.com/permissionlesstech/bitchat.git
synced 2026-07-25 01:05:19 +00:00
Fix restore-path main↔bleQueue deadlock and courier drop amplification (#1425)
* Fix restore-path main↔bleQueue deadlock and courier drop amplification A device froze permanently in a two-phone test. Debugger stacks showed an ABBA deadlock: the main actor was in bleQueue.sync (delivery-ack send → broadcastPacket → readLinkState) while bleQueue was in main.sync (captureBluetoothStatus reading backgroundTimeRemaining). The load that lined the two edges up came from a courier-drop amplification storm: drop dedup was in-memory only while the outbox driving 120s re-deposits is persisted, so every relaunch republished the same undelivered DM as a fresh 24h relay drop and every gateway relaunch re-fetched the whole backlog — ~20 copies of one DM delivered in 40ms, each triggering decrypt + delivery + ack + handshake work. Fixes, in rank order: - Edge B (P0): captureBluetoothStatus no longer main.syncs from bleQueue; backgroundTimeRemaining is sampled on main and cached behind a lock. Invariant documented: bleQueue must NEVER sync-dispatch to main. - Edge A (P0, defense in depth): sendDeliveryAck / sendReadReceipt / sendPrivateMessage / sendNoisePayload / triggerHandshake hop to messageQueue like sendMessage, so no main-actor call path reaches readLinkState's bleQueue.sync. - Drop dedup (P1): publishedDropKeys and seenDropEventIDs persist across relaunches (new BridgeDropDedupStore, entries expire with the 24h NIP-40 drop window; wiped on panic) — one drop per message ID per 24h regardless of relaunch count. - Receiver dedup (P1): openCourierEnvelope dedups on the inner private message ID before delivery, so a duplicate copy costs one decrypt and never re-delivers, re-acks, or re-triggers a handshake. - Handshake gating (P2): queued acks initiate a Noise handshake only for reachable peers; mail from absent/rotated identities no longer turns each copy into a mesh-wide handshake flood (the ack stays queued and flushes when a session eventually establishes). - Outbox (P3): re-enqueueing a queued message ID carries over its depositedCourierKeys so resends stop re-burning the same courier slots. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Review fixes: offline-drop durability, gateway handoff retry, restore-log freshness, coalesced persist Adversarial review of the storm/deadlock PR surfaced four issues: - Offline blackhole (must-fix): a deposit made while relays were down persisted its dedup key even though the drop only sat in the in-memory pending queue — app killed before reconnect meant the relaunch lost the drop but the persisted key blocked every re-deposit for 24h. The persisted snapshot now excludes keys still pending; they become durable only when flushPendingDrops actually publishes them. - Gateway handoff: seen-event IDs were consumed before the deliverToPeer handoff; a failed handoff (peer walked away) permanently dropped the event for a single-gateway island. deliverToPeer now reports whether the handoff was attempted, and a failure releases the seen slot so a relaunch or backlog redelivery retries. - Restore-path logs: central/peripheral-restore captures logged the init sentinel bgRemaining=∞. The cache is now seeded in init's main-thread branch and restore captures route through the sampler, which refreshes the cached budget before logging. - Persist cost: the dedup record was a full JSON encode + atomic write on the main actor per mutation (once per event during a backlog re-fetch). Writes now coalesce behind a 1s window, flushed immediately on background/terminate; panic wipe stays immediate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Remove BoundedIDSet.remove, orphaned by the ExpiringIDSet migration The drop-dedup sets that needed slot release moved to ExpiringIDSet; remaining BoundedIDSet users only insert and check. Periphery caught it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: jack <jackjackbits@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
jack
co-authored by
jack
Claude Fable 5
jack
Claude Fable 5
parent
304460ee83
commit
f8ab0a7dc0
@@ -129,6 +129,15 @@ final class BLEService: NSObject {
|
||||
// Application state tracking (thread-safe)
|
||||
#if os(iOS)
|
||||
private var isAppActive: Bool = true // Assume active initially
|
||||
/// Last `UIApplication.shared.backgroundTimeRemaining` sampled on the
|
||||
/// main thread, cached so bleQueue status logs can read it without ever
|
||||
/// dispatching to main (see `captureBluetoothStatus` for the invariant).
|
||||
private let backgroundTimeLock = NSLock()
|
||||
private var _cachedBackgroundTimeRemaining: TimeInterval = .greatestFiniteMagnitude
|
||||
private var cachedBackgroundTimeRemaining: TimeInterval {
|
||||
backgroundTimeLock.lock(); defer { backgroundTimeLock.unlock() }
|
||||
return _cachedBackgroundTimeRemaining
|
||||
}
|
||||
#endif
|
||||
|
||||
// MARK: - Core BLE Objects
|
||||
@@ -176,6 +185,13 @@ final class BLEService: NSObject {
|
||||
|
||||
// Ingress link tracking for duplicate and last-hop suppression
|
||||
private var ingressLinks = BLEIngressLinkRegistry()
|
||||
// Inner message IDs of recently opened courier envelopes. Redundant
|
||||
// copies of one message ride different envelopes (each seal uses a fresh
|
||||
// ephemeral key, and bridge drops multiply across relays/couriers), so
|
||||
// envelope-level dedup can't catch them; dedup on the inner ID before
|
||||
// delivery so a duplicate costs one decrypt instead of a delivery + ack
|
||||
// + handshake each. Owned by collectionsQueue barriers.
|
||||
private var openedCourierMessageIDs = BoundedIDSet(capacity: TransportConfig.courierOpenedMessageIDCap)
|
||||
private let logRateLimiter = BLELogRateLimiter(defaultMinimumInterval: 5)
|
||||
|
||||
private var pendingPeripheralWrites = BLEOutboundWriteBuffer()
|
||||
@@ -265,12 +281,18 @@ final class BLEService: NSObject {
|
||||
|
||||
// Set up application state tracking (iOS only)
|
||||
#if os(iOS)
|
||||
// Check initial state on main thread
|
||||
// Check initial state on main thread. The background-budget cache is
|
||||
// seeded here too: a background-restore launch captures Bluetooth
|
||||
// status before any lifecycle notification fires, and the init-time
|
||||
// sentinel would log a meaningless bgRemaining=∞ for exactly the
|
||||
// wake window that matters.
|
||||
if Thread.isMainThread {
|
||||
isAppActive = UIApplication.shared.applicationState == .active
|
||||
refreshCachedBackgroundTimeRemaining()
|
||||
} else {
|
||||
DispatchQueue.main.sync {
|
||||
isAppActive = UIApplication.shared.applicationState == .active
|
||||
refreshCachedBackgroundTimeRemaining()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -777,7 +799,11 @@ final class BLEService: NSObject {
|
||||
}
|
||||
|
||||
func triggerHandshake(with peerID: PeerID) {
|
||||
initiateNoiseHandshake(with: peerID)
|
||||
// Callers are on the main actor; the handshake broadcast sync-waits
|
||||
// on bleQueue for link state, so hop off main first.
|
||||
messageQueue.async { [weak self] in
|
||||
self?.initiateNoiseHandshake(with: peerID)
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: Noise identity/session access (narrow Transport wrappers)
|
||||
@@ -932,6 +958,15 @@ final class BLEService: NSObject {
|
||||
|
||||
|
||||
func sendReadReceipt(_ receipt: ReadReceipt, to peerID: PeerID) {
|
||||
// Hop like sendMessage: callers are often on the main actor, and the
|
||||
// send path sync-waits on bleQueue for link state — the main thread
|
||||
// must never block on bleQueue (see captureBluetoothStatus).
|
||||
if DispatchQueue.getSpecific(key: messageQueueKey) == nil {
|
||||
messageQueue.async { [weak self] in
|
||||
self?.sendReadReceipt(receipt, to: peerID)
|
||||
}
|
||||
return
|
||||
}
|
||||
let payload = BLENoisePayloadFactory.readReceipt(originalMessageID: receipt.originalMessageID)
|
||||
|
||||
if noiseService.hasEstablishedSession(with: peerID) {
|
||||
@@ -942,11 +977,15 @@ final class BLEService: NSObject {
|
||||
SecureLogger.error("Failed to send read receipt: \(error)")
|
||||
}
|
||||
} else {
|
||||
// Queue for after handshake and initiate if needed
|
||||
// Queue for after handshake; initiate only while the peer is
|
||||
// around to answer (see sendDeliveryAck — absent senders must
|
||||
// not turn queued acks into handshake floods).
|
||||
collectionsQueue.sync(flags: .barrier) {
|
||||
pendingNoiseSessionQueues.appendTypedPayload(payload, for: peerID)
|
||||
}
|
||||
if !noiseService.hasSession(with: peerID) { initiateNoiseHandshake(with: peerID) }
|
||||
if !noiseService.hasSession(with: peerID), isPeerReachable(peerID) {
|
||||
initiateNoiseHandshake(with: peerID)
|
||||
}
|
||||
SecureLogger.debug("🕒 Queued READ receipt for \(peerID.id.prefix(8))… until handshake completes", category: .session)
|
||||
}
|
||||
}
|
||||
@@ -1395,6 +1434,15 @@ final class BLEService: NSObject {
|
||||
}
|
||||
|
||||
func sendDeliveryAck(for messageID: String, to peerID: PeerID) {
|
||||
// Hop like sendMessage: callers are often on the main actor, and the
|
||||
// send path sync-waits on bleQueue for link state — the main thread
|
||||
// must never block on bleQueue (see captureBluetoothStatus).
|
||||
if DispatchQueue.getSpecific(key: messageQueueKey) == nil {
|
||||
messageQueue.async { [weak self] in
|
||||
self?.sendDeliveryAck(for: messageID, to: peerID)
|
||||
}
|
||||
return
|
||||
}
|
||||
let payload = BLENoisePayloadFactory.delivered(messageID: messageID)
|
||||
|
||||
if noiseService.hasEstablishedSession(with: peerID) {
|
||||
@@ -1404,11 +1452,18 @@ final class BLEService: NSObject {
|
||||
SecureLogger.error("Failed to send delivery ACK: \(error)")
|
||||
}
|
||||
} else {
|
||||
// Queue for after handshake and initiate if needed
|
||||
// Queue for after handshake; initiate only while the peer is
|
||||
// around to answer — couriered/bridged mail routinely arrives
|
||||
// from absent (or rotated) identities, and every duplicate copy
|
||||
// initiating a handshake broadcast turns one undeliverable ack
|
||||
// into a mesh-wide flood. The queued ack flushes whenever a
|
||||
// session eventually establishes.
|
||||
collectionsQueue.sync(flags: .barrier) {
|
||||
pendingNoiseSessionQueues.appendTypedPayload(payload, for: peerID)
|
||||
}
|
||||
if !noiseService.hasSession(with: peerID) { initiateNoiseHandshake(with: peerID) }
|
||||
if !noiseService.hasSession(with: peerID), isPeerReachable(peerID) {
|
||||
initiateNoiseHandshake(with: peerID)
|
||||
}
|
||||
SecureLogger.debug("🕒 Queued DELIVERED ack for \(peerID.id.prefix(8))… until handshake completes", category: .session)
|
||||
}
|
||||
}
|
||||
@@ -1680,7 +1735,10 @@ extension BLEService: CBCentralManagerDelegate {
|
||||
recentPeripheralCache.record(peripheral, peripheralID: identifier, at: Date())
|
||||
}
|
||||
|
||||
captureBluetoothStatus(context: "central-restore")
|
||||
// Via the sampler (not a direct capture): it refreshes the cached
|
||||
// background budget on main first, so the restore log shows the real
|
||||
// wake window instead of the init sentinel.
|
||||
logBluetoothStatus("central-restore")
|
||||
|
||||
if central.state == .poweredOn {
|
||||
startScanning()
|
||||
@@ -2439,7 +2497,8 @@ extension BLEService: CBPeripheralManagerDelegate {
|
||||
}
|
||||
}
|
||||
|
||||
captureBluetoothStatus(context: "peripheral-restore")
|
||||
// Via the sampler for a fresh background budget (see central-restore).
|
||||
logBluetoothStatus("peripheral-restore")
|
||||
|
||||
if peripheral.state == .poweredOn && !peripheral.isAdvertising {
|
||||
peripheral.startAdvertising(buildAdvertisementData())
|
||||
@@ -2720,19 +2779,37 @@ extension BLEService {
|
||||
}
|
||||
|
||||
private func logBluetoothStatus(_ context: String) {
|
||||
bleQueue.async { [weak self] in
|
||||
guard let self = self else { return }
|
||||
self.captureBluetoothStatus(context: context)
|
||||
}
|
||||
scheduleBluetoothStatusSample(after: 0, context: context)
|
||||
}
|
||||
|
||||
private func scheduleBluetoothStatusSample(after delay: TimeInterval, context: String) {
|
||||
bleQueue.asyncAfter(deadline: .now() + delay) { [weak self] in
|
||||
guard let self = self else { return }
|
||||
self.captureBluetoothStatus(context: context)
|
||||
#if os(iOS)
|
||||
// Sample the main-actor background budget first (async hop, never a
|
||||
// sync wait), then log from bleQueue off the cache — bleQueue must
|
||||
// never block on main (see captureBluetoothStatus).
|
||||
DispatchQueue.main.asyncAfter(deadline: .now() + delay) { [weak self] in
|
||||
guard let self else { return }
|
||||
self.refreshCachedBackgroundTimeRemaining()
|
||||
self.bleQueue.async { self.captureBluetoothStatus(context: context) }
|
||||
}
|
||||
#else
|
||||
bleQueue.asyncAfter(deadline: .now() + delay) { [weak self] in
|
||||
self?.captureBluetoothStatus(context: context)
|
||||
}
|
||||
#endif
|
||||
}
|
||||
|
||||
#if os(iOS)
|
||||
/// Main thread only (reads main-actor UIApplication state).
|
||||
private func refreshCachedBackgroundTimeRemaining() {
|
||||
dispatchPrecondition(condition: .onQueue(.main))
|
||||
let seconds = UIApplication.shared.backgroundTimeRemaining
|
||||
backgroundTimeLock.lock()
|
||||
_cachedBackgroundTimeRemaining = seconds
|
||||
backgroundTimeLock.unlock()
|
||||
}
|
||||
#endif
|
||||
|
||||
private func captureBluetoothStatus(context: String) {
|
||||
assert(DispatchQueue.getSpecific(key: bleQueueKey) != nil, "captureBluetoothStatus must run on bleQueue")
|
||||
|
||||
@@ -2750,11 +2827,15 @@ extension BLEService {
|
||||
}
|
||||
|
||||
#if os(iOS)
|
||||
var backgroundDescriptor = ""
|
||||
var backgroundSeconds: TimeInterval = 0
|
||||
DispatchQueue.main.sync {
|
||||
backgroundSeconds = UIApplication.shared.backgroundTimeRemaining
|
||||
}
|
||||
// INVARIANT: bleQueue must NEVER sync-dispatch to the main thread.
|
||||
// The main actor sync-waits on bleQueue along the send paths
|
||||
// (readLinkState), so a main.sync here completes an ABBA deadlock —
|
||||
// field-verified as a permanent freeze when a courier-drop storm put
|
||||
// an ack send (main → bleQueue.sync) up against a status capture
|
||||
// (bleQueue → main.sync). backgroundTimeRemaining is main-actor
|
||||
// state, so it is sampled on main and cached.
|
||||
let backgroundSeconds = cachedBackgroundTimeRemaining
|
||||
let backgroundDescriptor: String
|
||||
if backgroundSeconds == .greatestFiniteMagnitude {
|
||||
backgroundDescriptor = " bgRemaining=∞"
|
||||
} else {
|
||||
@@ -3044,6 +3125,15 @@ extension BLEService {
|
||||
|
||||
|
||||
private func sendNoisePayload(_ typedPayload: Data, to peerID: PeerID) {
|
||||
// Hop like sendMessage: the Transport-facing wrappers (verify/vouch/
|
||||
// group payloads) call this from the main actor, and the send path
|
||||
// sync-waits on bleQueue for link state.
|
||||
if DispatchQueue.getSpecific(key: messageQueueKey) == nil {
|
||||
messageQueue.async { [weak self] in
|
||||
self?.sendNoisePayload(typedPayload, to: peerID)
|
||||
}
|
||||
return
|
||||
}
|
||||
guard noiseService.hasSession(with: peerID) else {
|
||||
// No session yet - queue the payload SYNCHRONOUSLY before initiating handshake
|
||||
// to prevent race where fast handshake completion drains empty queue
|
||||
@@ -3291,6 +3381,23 @@ extension BLEService {
|
||||
SecureLogger.warning("⚠️ Courier envelope carried unsupported payload type", category: .session)
|
||||
return
|
||||
}
|
||||
let payload = Data(typedPayload.dropFirst())
|
||||
guard let innerMessageID = PrivateMessagePacket.decode(from: payload)?.messageID else {
|
||||
SecureLogger.warning("⚠️ Courier envelope carried undecodable private message", category: .session)
|
||||
return
|
||||
}
|
||||
// Redundant copies of one message arrive as distinct envelopes
|
||||
// (fresh seal each: mesh couriers, bridge drops across relays),
|
||||
// so dedup here on the inner message ID — before delivery, ack,
|
||||
// and handshake work. A duplicate costs only the decrypt above
|
||||
// and at most one ack ever goes out per message ID.
|
||||
let firstOpen = collectionsQueue.sync(flags: .barrier) {
|
||||
openedCourierMessageIDs.insert(innerMessageID)
|
||||
}
|
||||
guard firstOpen else {
|
||||
SecureLogger.debug("📦 Dropping duplicate courier envelope for message \(innerMessageID.prefix(8))…", category: .session)
|
||||
return
|
||||
}
|
||||
// Couriered mail arrives while the sender is absent, so the UI's
|
||||
// block check can't resolve their fingerprint from a live session.
|
||||
// Gate here, where the full static key is in hand.
|
||||
@@ -3306,7 +3413,6 @@ extension BLEService {
|
||||
let shortID = PeerID(publicKey: senderStaticKey)
|
||||
let isKnownOnMesh = collectionsQueue.sync { peerRegistry.info(for: shortID) != nil }
|
||||
let senderPeerID = isKnownOnMesh ? shortID : PeerID(hexData: senderStaticKey)
|
||||
let payload = Data(typedPayload.dropFirst())
|
||||
SecureLogger.debug("📦 Opened courier envelope from \(senderPeerID.id.prefix(8))…", category: .session)
|
||||
sfMetrics?.record(.courierOpened)
|
||||
notifyUI { [weak self] in
|
||||
@@ -3745,6 +3851,7 @@ extension BLEService {
|
||||
#if os(iOS)
|
||||
@objc private func appDidBecomeActive() {
|
||||
isAppActive = true
|
||||
refreshCachedBackgroundTimeRemaining()
|
||||
// Restart scanning with allow duplicates when app becomes active
|
||||
if centralManager?.state == .poweredOn {
|
||||
centralManager?.stopScan()
|
||||
@@ -3758,6 +3865,7 @@ extension BLEService {
|
||||
|
||||
@objc private func appDidEnterBackground() {
|
||||
isAppActive = false
|
||||
refreshCachedBackgroundTimeRemaining()
|
||||
// Restart scanning without allow duplicates in background
|
||||
if centralManager?.state == .poweredOn {
|
||||
centralManager?.stopScan()
|
||||
@@ -3851,6 +3959,15 @@ extension BLEService {
|
||||
// MARK: Private Message Handling
|
||||
|
||||
private func sendPrivateMessage(_ content: String, to recipientID: PeerID, messageID: String) {
|
||||
// Hop like sendMessage: the Transport-facing wrappers call this from
|
||||
// the main actor (router sends, favorite notifications), and the send
|
||||
// path sync-waits on bleQueue for link state.
|
||||
if DispatchQueue.getSpecific(key: messageQueueKey) == nil {
|
||||
messageQueue.async { [weak self] in
|
||||
self?.sendPrivateMessage(content, to: recipientID, messageID: messageID)
|
||||
}
|
||||
return
|
||||
}
|
||||
// Sessions and wire recipient IDs are keyed by the short 16-hex form;
|
||||
// callers may pass the full 64-hex noise key (mirrors sendFilePrivate).
|
||||
let recipientID = recipientID.toShort()
|
||||
|
||||
Reference in New Issue
Block a user