From d42d5b29feaa51bb2b9937dc2b1866265ae6b0b7 Mon Sep 17 00:00:00 2001 From: ecgang Date: Fri, 31 Jul 2026 00:41:44 -0700 Subject: [PATCH] Courier spray: adopt the link-layer slices' engine contracts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The V3 slices removed `collectionsQueue`, so the spray timeouts and the taker lookup no longer had the barrier they were written against. `peerRegistry.info(for:)` is read directly now, matching every other call site on main. The timeouts move into `BLESprayTimeoutStore`, lock-backed rather than engine-confined for the same reason `BLEPrivateMediaSessionStore` is: the two ends genuinely run in different contexts. Offers are armed on the main actor inside the `notifyUI` hop that decides whether to hand mail to a peer; acks, declines, and the timeout itself run on the engine. A leaf lock is safe from both, where re-entering the engine from the main actor is not. Each method is one whole transition under the lock, so an ack and a firing timeout cannot both claim the same offer. The delay goes through `engineScheduler` instead of `messageQueue.asyncAfter` — BLEQueueContractTests caught that one, and the manual clock now drives the assume-delivered deadline instead of the wall clock. --- .periphery.baseline.json | 2 +- bitchat/Services/BLE/BLEService.swift | 47 ++++------------ .../Services/BLE/BLESprayTimeoutStore.swift | 53 +++++++++++++++++++ 3 files changed, 65 insertions(+), 37 deletions(-) create mode 100644 bitchat/Services/BLE/BLESprayTimeoutStore.swift diff --git a/.periphery.baseline.json b/.periphery.baseline.json index eed93745..a52745a1 100644 --- a/.periphery.baseline.json +++ b/.periphery.baseline.json @@ -1 +1 @@ -{"v1":{"usrs":["param-buf-arti_bootstrap_summary(_:_:)-s:3Tor22arti_bootstrap_summary33_954FD7701B4E47ABB5F166D1CF862DC9LLys5Int32VSpys4Int8VG_AEtF","param-dataDir-arti_start(_:_:)-s:3Tor10arti_start33_954FD7701B4E47ABB5F166D1CF862DC9LLys5Int32VSPys4Int8VG_s6UInt16VtF","param-len-arti_bootstrap_summary(_:_:)-s:3Tor22arti_bootstrap_summary33_954FD7701B4E47ABB5F166D1CF862DC9LLys5Int32VSpys4Int8VG_AEtF","param-socksPort-arti_start(_:_:)-s:3Tor10arti_start33_954FD7701B4E47ABB5F166D1CF862DC9LLys5Int32VSPys4Int8VG_s6UInt16VtF","s:13BitFoundation16PeerCapabilitiesV8wifiBulkACvpZ","s:13BitFoundation18KeychainReadResultO18isRecoverableErrorSbvp","s:13BitFoundation23KeychainManagerProtocolP11secureClearyySSzF","s:18bitchatTests_macOS12MockKeychainC11secureClearyySSzF","s:18bitchatTests_macOS20TrackingMockKeychainC11resetCountsyyF","s:18bitchatTests_macOS20TrackingMockKeychainC11secureClearyySSzF","s:18bitchatTests_macOS20TrackingMockKeychainC25totalSecureClearCallCountSivp","s:18bitchatTests_macOS20TrackingMockKeychainC26secureClearStringCallCountSivp","s:18bitchatTests_macOS20TrackingMockKeychainC27_secureClearStringCallCount06_AB6D1M24FD239F2969C82F4108818260LLSivp","s:18bitchatTests_macOS24FailingCacheSaveKeychain33_22380C7A11A569A0B83FA83F34C498A7LLC11secureClearyySSzF","s:18bitchatTests_macOS24MockGeohashPresenceTimer33_483587EFB96650EE130EFB09BBA2A1AALLC7handleryycvp","s:3Tor0A7ManagerC21goDormantOnBackgroundyyF","s:7bitchat10AppRuntimeC24handleScreenshotCaptured33_C8B369AD8BC1D9963A50CEDA77A4332ALLyyF","s:7bitchat10AppRuntimeC33handleDidBecomeActiveNotificationyyF","s:7bitchat10BLEServiceC18logBluetoothStatus33_69191C53E68500C17D98DBCF2BDA7100LLyySSF","s:7bitchat10BLEServiceC18logBluetoothStatusyySSF","s:7bitchat10BLEServiceC20centralRestorationID33_69191C53E68500C17D98DBCF2BDA7100LLSSvpZ","s:7bitchat10BLEServiceC22PendingSprayTimeoutKey33_69191C53E68500C17D98DBCF2BDA7100LLV012courierNoiseF010Foundation4DataVvp","s:7bitchat10BLEServiceC22PendingSprayTimeoutKey33_69191C53E68500C17D98DBCF2BDA7100LLV14ciphertextHash10Foundation4DataVvp","s:7bitchat10BLEServiceC22captureBluetoothStatus33_69191C53E68500C17D98DBCF2BDA7100LL7contextySS_tF","s:7bitchat10BLEServiceC23peripheralRestorationID33_69191C53E68500C17D98DBCF2BDA7100LLSSvpZ","s:7bitchat10BLEServiceC29scheduleBluetoothStatusSample33_69191C53E68500C17D98DBCF2BDA7100LL5after7contextySd_SStF","s:7bitchat10QRScanViewV8isActiveSbvp","s:7bitchat12CourierStoreC20PendingSprayOfferKey33_2ACC0C4A3AC49437986BFAB5D4E72E70LLV012courierNoiseG010Foundation4DataVvp","s:7bitchat12CourierStoreC20PendingSprayOfferKey33_2ACC0C4A3AC49437986BFAB5D4E72E70LLV14ciphertextHash10Foundation4DataVvp","s:7bitchat15BLEPeerRegistryV5countSivp","s:7bitchat15KeychainManagerC11secureClearyySSzF","s:7bitchat15PaymentChipViewV7openURL33_10AC50641B1EBCD52E5092A2E521D236LL7SwiftUI13OpenURLActionVvp","s:7bitchat15TransportConfigO29uiBatchDispatchStaggerSecondsSdvpZ","s:7bitchat15TransportConfigO35uiShareExtensionDismissDelaySecondsSdvpZ","s:7bitchat15TransportConfigO38bleBackgroundPendingConnectSlotReserveSivpZ","s:7bitchat17GossipSyncManagerC10persistNowyyF","s:7bitchat17NostrRelayManagerC15InboundEventKey33_E4160FE8A9A2C9D6308EAAD5A8B5CB07LLV7eventIDSSvp","s:7bitchat18BLERadioControllerC14candidateCountSivp","s:7bitchat25LocationNotesDependenciesV3now10Foundation4DateVycvp","s:7bitchat25NWPathReachabilityMonitorC7monitor33_84633C9DBCAF57538179C1E04DB8E015LL7Network0bD0CSgvp"]}} +{"v1":{"usrs":["param-buf-arti_bootstrap_summary(_:_:)-s:3Tor22arti_bootstrap_summary33_954FD7701B4E47ABB5F166D1CF862DC9LLys5Int32VSpys4Int8VG_AEtF","param-dataDir-arti_start(_:_:)-s:3Tor10arti_start33_954FD7701B4E47ABB5F166D1CF862DC9LLys5Int32VSPys4Int8VG_s6UInt16VtF","param-len-arti_bootstrap_summary(_:_:)-s:3Tor22arti_bootstrap_summary33_954FD7701B4E47ABB5F166D1CF862DC9LLys5Int32VSpys4Int8VG_AEtF","param-socksPort-arti_start(_:_:)-s:3Tor10arti_start33_954FD7701B4E47ABB5F166D1CF862DC9LLys5Int32VSPys4Int8VG_s6UInt16VtF","s:13BitFoundation16PeerCapabilitiesV8wifiBulkACvpZ","s:13BitFoundation18KeychainReadResultO18isRecoverableErrorSbvp","s:13BitFoundation23KeychainManagerProtocolP11secureClearyySSzF","s:18bitchatTests_macOS12MockKeychainC11secureClearyySSzF","s:18bitchatTests_macOS20TrackingMockKeychainC11resetCountsyyF","s:18bitchatTests_macOS20TrackingMockKeychainC11secureClearyySSzF","s:18bitchatTests_macOS20TrackingMockKeychainC25totalSecureClearCallCountSivp","s:18bitchatTests_macOS20TrackingMockKeychainC26secureClearStringCallCountSivp","s:18bitchatTests_macOS20TrackingMockKeychainC27_secureClearStringCallCount06_AB6D1M24FD239F2969C82F4108818260LLSivp","s:18bitchatTests_macOS24FailingCacheSaveKeychain33_22380C7A11A569A0B83FA83F34C498A7LLC11secureClearyySSzF","s:18bitchatTests_macOS24MockGeohashPresenceTimer33_483587EFB96650EE130EFB09BBA2A1AALLC7handleryycvp","s:3Tor0A7ManagerC21goDormantOnBackgroundyyF","s:7bitchat10AppRuntimeC24handleScreenshotCaptured33_C8B369AD8BC1D9963A50CEDA77A4332ALLyyF","s:7bitchat10AppRuntimeC33handleDidBecomeActiveNotificationyyF","s:7bitchat10BLEServiceC18logBluetoothStatus33_69191C53E68500C17D98DBCF2BDA7100LLyySSF","s:7bitchat10BLEServiceC18logBluetoothStatusyySSF","s:7bitchat10BLEServiceC20centralRestorationID33_69191C53E68500C17D98DBCF2BDA7100LLSSvpZ","s:7bitchat10BLEServiceC22captureBluetoothStatus33_69191C53E68500C17D98DBCF2BDA7100LL7contextySS_tF","s:7bitchat10BLEServiceC23peripheralRestorationID33_69191C53E68500C17D98DBCF2BDA7100LLSSvpZ","s:7bitchat10BLEServiceC29scheduleBluetoothStatusSample33_69191C53E68500C17D98DBCF2BDA7100LL5after7contextySd_SStF","s:7bitchat10QRScanViewV8isActiveSbvp","s:7bitchat12CourierStoreC20PendingSprayOfferKey33_2ACC0C4A3AC49437986BFAB5D4E72E70LLV012courierNoiseG010Foundation4DataVvp","s:7bitchat12CourierStoreC20PendingSprayOfferKey33_2ACC0C4A3AC49437986BFAB5D4E72E70LLV14ciphertextHash10Foundation4DataVvp","s:7bitchat15BLEPeerRegistryV5countSivp","s:7bitchat15KeychainManagerC11secureClearyySSzF","s:7bitchat15PaymentChipViewV7openURL33_10AC50641B1EBCD52E5092A2E521D236LL7SwiftUI13OpenURLActionVvp","s:7bitchat15TransportConfigO29uiBatchDispatchStaggerSecondsSdvpZ","s:7bitchat15TransportConfigO35uiShareExtensionDismissDelaySecondsSdvpZ","s:7bitchat15TransportConfigO38bleBackgroundPendingConnectSlotReserveSivpZ","s:7bitchat17GossipSyncManagerC10persistNowyyF","s:7bitchat17NostrRelayManagerC15InboundEventKey33_E4160FE8A9A2C9D6308EAAD5A8B5CB07LLV7eventIDSSvp","s:7bitchat18BLERadioControllerC14candidateCountSivp","s:7bitchat18BLESprayTimeoutKeyV012courierNoiseD010Foundation4DataVvp","s:7bitchat18BLESprayTimeoutKeyV14ciphertextHash10Foundation4DataVvp","s:7bitchat25LocationNotesDependenciesV3now10Foundation4DateVycvp","s:7bitchat25NWPathReachabilityMonitorC7monitor33_84633C9DBCAF57538179C1E04DB8E015LL7Network0bD0CSgvp"]}} diff --git a/bitchat/Services/BLE/BLEService.swift b/bitchat/Services/BLE/BLEService.swift index fc232103..49b9470b 100644 --- a/bitchat/Services/BLE/BLEService.swift +++ b/bitchat/Services/BLE/BLEService.swift @@ -314,18 +314,10 @@ final class BLEService: NSObject { private var meshPings = BLEMeshPingTracker() // Per-offer assume-delivered timeouts for spray copies handed to a - // `.courierAck`-capable taker, keyed by `(ciphertextHash, courierNoiseKey)` - // to match CourierStore's own pending-offer map so an ack/decline (or the - // timeout) resolves the right entry. This dictionary only holds the - // `DispatchWorkItem`s for cancellation — the offer/budget state lives in - // `CourierStore` behind its own serial queue; this side just calls - // `offerSprayCopies`/`confirmSpray`/`cancelSpray` (mirrors the ping/pong - // queue split). - private struct PendingSprayTimeoutKey: Hashable { - let ciphertextHash: Data - let courierNoiseKey: Data - } - private var pendingSprayTimeouts: [PendingSprayTimeoutKey: DispatchWorkItem] = [:] + // `.courierAck`-capable taker (lock-backed store: armed on the main actor + // inside the notifyUI hop, resolved on the engine by an ack, a decline, or + // the timeout itself). + private let sprayTimeouts = BLESprayTimeoutStore() // 5. Fragment Reassembly (necessary for messages > MTU) private var fragmentAssemblyBuffer = BLEFragmentAssemblyBuffer() @@ -4915,7 +4907,7 @@ extension BLEService { SecureLogger.debug("📦 Spray-\(kind) rejected: relayed \(kind) claims sender \(PeerID(hexData: packet.senderID).id.prefix(8))… but arrived from \(peerID.id.prefix(8))…", category: .security) return nil } - let takerInfo = collectionsQueue.sync { peerRegistry.info(for: peerID) } + let takerInfo = peerRegistry.info(for: peerID) guard let takerKey = takerInfo?.noisePublicKey else { SecureLogger.debug("📦 Spray-\(kind) from unknown peer \(peerID.id.prefix(8))… rejected", category: .session) return nil @@ -4942,10 +4934,7 @@ extension BLEService { guard packet.payload.count == CourierEnvelope.tagLength else { return } let ciphertextHash = packet.payload guard courierStore.confirmSpray(courierNoiseKey: takerKey, ciphertextHash: ciphertextHash) else { return } - let key = PendingSprayTimeoutKey(ciphertextHash: ciphertextHash, courierNoiseKey: takerKey) - collectionsQueue.sync(flags: .barrier) { - pendingSprayTimeouts.removeValue(forKey: key)?.cancel() - } + sprayTimeouts.cancel(BLESprayTimeoutKey(ciphertextHash: ciphertextHash, courierNoiseKey: takerKey)) } /// Applies a taker's signed refusal of an offered copy, restoring the @@ -4971,10 +4960,7 @@ extension BLEService { guard packet.payload.count == CourierEnvelope.tagLength else { return } let ciphertextHash = packet.payload guard courierStore.cancelSpray(ciphertextHash: ciphertextHash, courierNoiseKey: takerKey) else { return } - let key = PendingSprayTimeoutKey(ciphertextHash: ciphertextHash, courierNoiseKey: takerKey) - collectionsQueue.sync(flags: .barrier) { - pendingSprayTimeouts.removeValue(forKey: key)?.cancel() - } + sprayTimeouts.cancel(BLESprayTimeoutKey(ciphertextHash: ciphertextHash, courierNoiseKey: takerKey)) } /// Starts (or replaces) the timeout for one outstanding spray offer. When @@ -4988,25 +4974,14 @@ extension BLEService { /// refused sends an explicit decline, which restores the budget before this /// fires. private func scheduleSprayOfferTimeout(ciphertextHash: Data, courierNoiseKey: Data) { - let key = PendingSprayTimeoutKey(ciphertextHash: ciphertextHash, courierNoiseKey: courierNoiseKey) + let key = BLESprayTimeoutKey(ciphertextHash: ciphertextHash, courierNoiseKey: courierNoiseKey) let store = courierStore let timeout = DispatchWorkItem { [weak self] in - guard let self else { return } - let stillPending = self.collectionsQueue.sync(flags: .barrier) { - self.pendingSprayTimeouts.removeValue(forKey: key) != nil - } - guard stillPending else { return } + guard let self, self.sprayTimeouts.claim(key) else { return } store.confirmSpray(courierNoiseKey: courierNoiseKey, ciphertextHash: ciphertextHash) } - collectionsQueue.sync(flags: .barrier) { - // Commit-time revalidation in `offerSprayCopies` means at most one - // copy per courier is committed for this envelope, but a re-announce - // can schedule a timeout before the losing commit no-ops — cancel - // any prior timeout for this key and keep the latest. - pendingSprayTimeouts.removeValue(forKey: key)?.cancel() - pendingSprayTimeouts[key] = timeout - } - messageQueue.asyncAfter(deadline: .now() + TransportConfig.courierSprayAckTimeoutSeconds, execute: timeout) + sprayTimeouts.arm(key, timeout: timeout) + engineScheduler.schedule(after: TransportConfig.courierSprayAckTimeoutSeconds, execute: timeout) } // MARK: One-Time Prekey Bundles diff --git a/bitchat/Services/BLE/BLESprayTimeoutStore.swift b/bitchat/Services/BLE/BLESprayTimeoutStore.swift new file mode 100644 index 00000000..b0d400eb --- /dev/null +++ b/bitchat/Services/BLE/BLESprayTimeoutStore.swift @@ -0,0 +1,53 @@ +import Foundation + +/// Identifies one outstanding spray offer. Mirrors `CourierStore`'s own +/// pending-offer key so an ack, a decline, or the assume-delivered timeout +/// all resolve the same entry. +struct BLESprayTimeoutKey: Hashable { + let ciphertextHash: Data + let courierNoiseKey: Data +} + +/// Lock-backed assume-delivered timeouts for spray copies handed to a +/// `.courierAck`-capable taker. +/// +/// This holds only the `DispatchWorkItem`s, for cancellation. The offer and +/// budget state lives in `CourierStore` behind its own serial queue; the +/// transport just calls `offerSprayCopies`/`confirmSpray`/`cancelSpray`. +/// +/// Lock-backed rather than engine-confined because the two ends run in +/// different contexts: offers are armed on the main actor (inside the +/// `notifyUI` hop that decides whether to hand mail to a peer), while acks, +/// declines, and the timeout itself run on the engine queue. A leaf lock is +/// safe from both; re-entering the engine from the main actor is not. +/// +/// Every method is one whole transition under the lock, so an ack and the +/// timeout racing for the same offer cannot both claim it. +final class BLESprayTimeoutStore: @unchecked Sendable { + private let lock = NSLock() + private var timeouts: [BLESprayTimeoutKey: DispatchWorkItem] = [:] + + /// Arms the timeout for one offer, cancelling any prior timeout for the + /// same key. Commit-time revalidation in `offerSprayCopies` means at most + /// one copy per courier is committed for an envelope, but a re-announce + /// can arm a timeout before the losing commit no-ops — so the latest wins. + func arm(_ key: BLESprayTimeoutKey, timeout: DispatchWorkItem) { + lock.withLock { + timeouts.removeValue(forKey: key)?.cancel() + timeouts[key] = timeout + } + } + + /// Cancels the timeout for a resolved offer. A no-op when the timeout + /// already fired or the offer was never armed. + func cancel(_ key: BLESprayTimeoutKey) { + lock.withLock { timeouts.removeValue(forKey: key)?.cancel() } + } + + /// Claims the offer on behalf of a firing timeout. Returns false when an + /// ack or decline already resolved it, so the timeout must not commit the + /// spend a second time. + func claim(_ key: BLESprayTimeoutKey) -> Bool { + lock.withLock { timeouts.removeValue(forKey: key) != nil } + } +}