From 4cecffca98355848469793737bfbfc7dcbfb9c4c Mon Sep 17 00:00:00 2001 From: jack Date: Tue, 8 Jul 2025 02:22:05 +0200 Subject: [PATCH] Fix thread safety crashes in BluetoothMeshService Fixed multiple threading issues that were causing crashes when users sent messages: - Added NSLock to synchronize access to recentlySentMessages Set - Fixed thread-unsafe access to activePeers Set using existing lock - Removed force unwrapping that could cause crashes - Added defensive input validation for message sending methods These fixes address crashes reported in TestFlight where the app would crash when typing and sending messages. --- bitchat/Services/BluetoothMeshService.swift | 48 +++++++++++++++---- .../Services/MessageRetentionService.swift | 5 +- 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/bitchat/Services/BluetoothMeshService.swift b/bitchat/Services/BluetoothMeshService.swift index 3488add4..09942d2e 100644 --- a/bitchat/Services/BluetoothMeshService.swift +++ b/bitchat/Services/BluetoothMeshService.swift @@ -74,6 +74,7 @@ class BluetoothMeshService: NSObject { private var cachedMessagesSentToPeer: Set = [] // Track which peers have already received cached messages private var receivedMessageTimestamps: [String: Date] = [:] // Track timestamps of received messages for debugging private var recentlySentMessages: Set = [] // Short-term cache to prevent any duplicate sends + private let recentlySentMessagesLock = NSLock() // Thread safety for recentlySentMessages private var lastMessageFromPeer: [String: Date] = [:] // Track last message time from each peer for connection prioritization // Battery and range optimizations @@ -490,6 +491,9 @@ class BluetoothMeshService: NSObject { } func sendMessage(_ content: String, mentions: [String] = [], channel: String? = nil, to recipientID: String? = nil) { + // Defensive check for empty content + guard !content.isEmpty else { return } + messageQueue.async { [weak self] in guard let self = self else { return } @@ -532,12 +536,21 @@ class BluetoothMeshService: NSObject { // Track this message to prevent duplicate sends let msgID = "\(packet.timestamp)-\(self.myPeerID)-\(packet.payload.prefix(32).hashValue)" - if !self.recentlySentMessages.contains(msgID) { + + self.recentlySentMessagesLock.lock() + let shouldSend = !self.recentlySentMessages.contains(msgID) + if shouldSend { self.recentlySentMessages.insert(msgID) - + } + self.recentlySentMessagesLock.unlock() + + if shouldSend { // Clean up old entries after 10 seconds self.messageQueue.asyncAfter(deadline: .now() + 10.0) { [weak self] in - self?.recentlySentMessages.remove(msgID) + guard let self = self else { return } + self.recentlySentMessagesLock.lock() + self.recentlySentMessages.remove(msgID) + self.recentlySentMessagesLock.unlock() } // Add random delay before initial send @@ -559,6 +572,9 @@ class BluetoothMeshService: NSObject { func sendPrivateMessage(_ content: String, to recipientPeerID: String, recipientNickname: String, messageID: String? = nil) { + // Defensive checks + guard !content.isEmpty, !recipientPeerID.isEmpty, !recipientNickname.isEmpty else { return } + messageQueue.async { [weak self] in guard let self = self else { return } @@ -617,7 +633,11 @@ class BluetoothMeshService: NSObject { // Check if recipient is offline and cache if they're a favorite - if !self.activePeers.contains(recipientPeerID) { + self.activePeersLock.lock() + let isRecipientOffline = !self.activePeers.contains(recipientPeerID) + self.activePeersLock.unlock() + + if isRecipientOffline { if let publicKeyData = self.encryptionService.getPeerIdentityKey(recipientPeerID) { let fingerprint = self.getPublicKeyFingerprint(publicKeyData) if self.delegate?.isFavorite(fingerprint: fingerprint) ?? false { @@ -630,12 +650,21 @@ class BluetoothMeshService: NSObject { // Track to prevent duplicate sends let msgID = "\(packet.timestamp)-\(self.myPeerID)-\(packet.payload.prefix(32).hashValue)" - if !self.recentlySentMessages.contains(msgID) { + + self.recentlySentMessagesLock.lock() + let shouldSend = !self.recentlySentMessages.contains(msgID) + if shouldSend { self.recentlySentMessages.insert(msgID) - + } + self.recentlySentMessagesLock.unlock() + + if shouldSend { // Clean up after 10 seconds self.messageQueue.asyncAfter(deadline: .now() + 10.0) { [weak self] in - self?.recentlySentMessages.remove(msgID) + guard let self = self else { return } + self.recentlySentMessagesLock.lock() + self.recentlySentMessages.remove(msgID) + self.recentlySentMessagesLock.unlock() } // Message tracking is now done in ChatViewModel to ensure consistent message IDs @@ -798,7 +827,10 @@ class BluetoothMeshService: NSObject { do { let sealedBox = try AES.GCM.seal(contentData, using: channelKey) - let encryptedData = sealedBox.combined! + guard let encryptedData = sealedBox.combined else { + // Encryption failed to produce combined data + return + } // Create message with encrypted content let message = BitchatMessage( diff --git a/bitchat/Services/MessageRetentionService.swift b/bitchat/Services/MessageRetentionService.swift index 24be6843..5775146d 100644 --- a/bitchat/Services/MessageRetentionService.swift +++ b/bitchat/Services/MessageRetentionService.swift @@ -31,7 +31,10 @@ class MessageRetentionService { private init() { // Get documents directory - documentsDirectory = FileManager.default.urls(for: .documentDirectory, in: .userDomainMask).first! + guard let docsDir = FileManager.default.urls(for: .documentDirectory, in: .userDomainMask).first else { + fatalError("Unable to access documents directory") + } + documentsDirectory = docsDir messagesDirectory = documentsDirectory.appendingPathComponent("Messages", isDirectory: true) // Create messages directory if it doesn't exist