From 9390c13c28163329cd0d7fab953a9d9a3010b8c7 Mon Sep 17 00:00:00 2001 From: Andy Uhnak Date: Wed, 8 Feb 2023 10:40:02 +0000 Subject: [PATCH 1/3] Fix some crashes --- .../Crypto/MXLegacyBackgroundCrypto.swift | 2 +- .../Crypto/CryptoMachine/MXCryptoRequests.swift | 17 ++++++++++++----- MatrixSDK/Crypto/MXCryptoV2Factory.swift | 2 +- .../Status/MXKeyVerificationStateResolver.swift | 8 ++++---- MatrixSDK/Data/MXRoomSummary.m | 12 ++++++++++++ .../MXBreadcrumbsRoomListDataFetcher.swift | 4 ++-- 6 files changed, 32 insertions(+), 13 deletions(-) diff --git a/MatrixSDK/Background/Crypto/MXLegacyBackgroundCrypto.swift b/MatrixSDK/Background/Crypto/MXLegacyBackgroundCrypto.swift index 8ebbb91fc3..50741ebeef 100644 --- a/MatrixSDK/Background/Crypto/MXLegacyBackgroundCrypto.swift +++ b/MatrixSDK/Background/Crypto/MXLegacyBackgroundCrypto.swift @@ -71,7 +71,7 @@ class MXLegacyBackgroundCrypto: MXBackgroundCrypto { let decryptionResult = MXEventDecryptionResult() decryptionResult.clearEvent = olmResult.payload decryptionResult.senderCurve25519Key = olmResult.senderKey - decryptionResult.claimedEd25519Key = olmResult.keysClaimed["ed25519"] as? String + decryptionResult.claimedEd25519Key = olmResult.keysClaimed?["ed25519"] as? String decryptionResult.forwardingCurve25519KeyChain = olmResult.forwardingCurve25519KeyChain decryptionResult.isUntrusted = olmResult.isUntrusted event.setClearData(decryptionResult) diff --git a/MatrixSDK/Crypto/CryptoMachine/MXCryptoRequests.swift b/MatrixSDK/Crypto/CryptoMachine/MXCryptoRequests.swift index 38cb47fc2b..365417bdd1 100644 --- a/MatrixSDK/Crypto/CryptoMachine/MXCryptoRequests.swift +++ b/MatrixSDK/Crypto/CryptoMachine/MXCryptoRequests.swift @@ -106,15 +106,22 @@ struct MXCryptoRequests { } } + @MainActor + // Calling methods on `MXRoom` has various state side effects so should be called on the main thread func roomMessage(request: RoomMessageRequest) async throws -> String? { - var event: MXEvent? - return try await performCallbackRequest { + return try await withCheckedThrowingContinuation { cont in + var event: MXEvent? request.room.sendEvent( MXEventType(identifier: request.eventType), content: request.content, - localEcho: &event, - completion: $0 - ) + localEcho: &event) { response in + switch response { + case .success(let value): + cont.resume(returning: value) + case .failure(let error): + cont.resume(throwing: error) + } + } } } diff --git a/MatrixSDK/Crypto/MXCryptoV2Factory.swift b/MatrixSDK/Crypto/MXCryptoV2Factory.swift index b54d3a082d..14d8c7b0fa 100644 --- a/MatrixSDK/Crypto/MXCryptoV2Factory.swift +++ b/MatrixSDK/Crypto/MXCryptoV2Factory.swift @@ -100,7 +100,7 @@ import Foundation private func migrateIfNecessary(legacyStore: MXCryptoStore, updateProgress: @escaping (Double) -> Void) { guard legacyStore.cryptoVersion.rawValue < MXCryptoVersion.versionLegacyDeprecated.rawValue else { - log.debug("Legacy crypto has already been deprecatd, no need to migrate") + log.debug("Legacy crypto has already been deprecated, no need to migrate") return } diff --git a/MatrixSDK/Crypto/Verification/Status/MXKeyVerificationStateResolver.swift b/MatrixSDK/Crypto/Verification/Status/MXKeyVerificationStateResolver.swift index 452ad73154..44186f5982 100644 --- a/MatrixSDK/Crypto/Verification/Status/MXKeyVerificationStateResolver.swift +++ b/MatrixSDK/Crypto/Verification/Status/MXKeyVerificationStateResolver.swift @@ -19,20 +19,19 @@ import Foundation /// Class that computes verification state for a request_id by comparing all related events /// and taking whichever final event (e.g. cancelled, done), when the request is no longer /// active. -@objcMembers -public class MXKeyVerificationStateResolver: NSObject { +actor MXKeyVerificationStateResolver { private let myUserId: String private let aggregations: MXAggregations private var states: [String: MXKeyVerificationState] private let log = MXNamedLog(name: "MXKeyVerificationStateResolver") - public init(myUserId: String, aggregations: MXAggregations) { + init(myUserId: String, aggregations: MXAggregations) { self.myUserId = myUserId self.aggregations = aggregations self.states = [:] } - public func verificationState(flowId: String, roomId: String) async throws -> MXKeyVerificationState { + func verificationState(flowId: String, roomId: String) async throws -> MXKeyVerificationState { log.debug("->") if let state = states[flowId] { @@ -67,6 +66,7 @@ public class MXKeyVerificationStateResolver: NSObject { } } + nonisolated private func resolvedState(for events: [MXEvent]) -> MXKeyVerificationState { var defaultState = MXKeyVerificationState.transactionStarted for event in events { diff --git a/MatrixSDK/Data/MXRoomSummary.m b/MatrixSDK/Data/MXRoomSummary.m index 33f3e4257f..7c8429fc1d 100644 --- a/MatrixSDK/Data/MXRoomSummary.m +++ b/MatrixSDK/Data/MXRoomSummary.m @@ -172,6 +172,18 @@ - (void)setMatrixSession:(MXSession *)mxSession - (void)save:(BOOL)commit { + if (!NSThread.isMainThread) + { + // Saving on the main thread is not ideal, but is currently the only safe way, given the mutation + // of internal state and posting notifications observed by UI without double-checking which thread + // the notification arrives on. + MXLogFailure(@"[MXRoomSummary] save: Saving room summary should happen from the main thread") + dispatch_async(dispatch_get_main_queue(), ^{ + [self save:commit]; + }); + return; + } + _dataTypes = self.calculateDataTypes; _sentStatus = self.calculateSentStatus; _favoriteTagOrder = self.room.accountData.tags[kMXRoomTagFavourite].order; diff --git a/MatrixSDK/Data/RoomList/Common/MXBreadcrumbsRoomListDataFetcher.swift b/MatrixSDK/Data/RoomList/Common/MXBreadcrumbsRoomListDataFetcher.swift index 1a5263985f..d4cd931ef5 100644 --- a/MatrixSDK/Data/RoomList/Common/MXBreadcrumbsRoomListDataFetcher.swift +++ b/MatrixSDK/Data/RoomList/Common/MXBreadcrumbsRoomListDataFetcher.swift @@ -91,10 +91,10 @@ internal class MXBreadcrumbsRoomListDataFetcher: NSObject, MXRoomListDataFetcher if let query = fetchOptions.filterOptions.query?.lowercased(), !query.isEmpty { recentRoomIds = recentRoomIds.filter({ roomId in - guard let summary = session?.roomSummary(withRoomId: roomId) else { + guard let displayname = session?.roomSummary(withRoomId: roomId).displayname else { return false } - return summary.displayname.lowercased().contains(query) + return displayname.lowercased().contains(query) }) } From ee6e7b5bf168d10949209c5c6baaa70e30bb76ab Mon Sep 17 00:00:00 2001 From: Andy Uhnak Date: Wed, 8 Feb 2023 16:41:24 +0000 Subject: [PATCH 2/3] Do not track cancellation errors nested in context --- MatrixSDK/Utils/Logs/MXAnalyticsDestination.swift | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/MatrixSDK/Utils/Logs/MXAnalyticsDestination.swift b/MatrixSDK/Utils/Logs/MXAnalyticsDestination.swift index e98ad20816..e3615dc3ce 100644 --- a/MatrixSDK/Utils/Logs/MXAnalyticsDestination.swift +++ b/MatrixSDK/Utils/Logs/MXAnalyticsDestination.swift @@ -38,12 +38,25 @@ class MXAnalyticsDestination: BaseDestination { private func shouldTrackIssue(with context: Any?) -> Bool { // We will track all issues except for those with a cancellation error - guard let error = context as? NSError else { + guard let error = errorFromContext(context: context) else { return true } return !error.isCancelledError } + private func errorFromContext(context: Any?) -> NSError? { + if let error = context as? NSError { + return error + } else if let dictionary = context as? [AnyHashable: Any] { + for (_, value) in dictionary { + if let error = value as? NSError { + return error + } + } + } + return nil + } + private func formattedDetails(from context: Any?) -> [String: Any]? { guard let context = context else { return nil From a0d5132ae7aae6b0b21030bdee91edc1b58491f5 Mon Sep 17 00:00:00 2001 From: Andy Uhnak Date: Fri, 10 Feb 2023 09:33:29 +0000 Subject: [PATCH 3/3] Style changes --- .../Crypto/CryptoMachine/MXCryptoRequests.swift | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/MatrixSDK/Crypto/CryptoMachine/MXCryptoRequests.swift b/MatrixSDK/Crypto/CryptoMachine/MXCryptoRequests.swift index 365417bdd1..5703725543 100644 --- a/MatrixSDK/Crypto/CryptoMachine/MXCryptoRequests.swift +++ b/MatrixSDK/Crypto/CryptoMachine/MXCryptoRequests.swift @@ -40,7 +40,7 @@ struct MXCryptoRequests { } func sendToDevice(request: ToDeviceRequest) async throws { - return try await performCallbackRequest { + try await performCallbackRequest { restClient.sendDirectToDevice( payload: .init( eventType: request.eventType, @@ -54,7 +54,7 @@ struct MXCryptoRequests { } func uploadKeys(request: UploadKeysRequest) async throws -> MXKeysUploadResponse { - return try await performCallbackRequest { + try await performCallbackRequest { restClient.uploadKeys( request.deviceKeys, oneTimeKeys: request.oneTimeKeys, @@ -101,7 +101,7 @@ struct MXCryptoRequests { } func claimKeys(request: ClaimKeysRequest) async throws -> MXKeysClaimResponse { - return try await performCallbackRequest { + try await performCallbackRequest { restClient.claimOneTimeKeys(for: request.devices, completion: $0) } } @@ -109,7 +109,7 @@ struct MXCryptoRequests { @MainActor // Calling methods on `MXRoom` has various state side effects so should be called on the main thread func roomMessage(request: RoomMessageRequest) async throws -> String? { - return try await withCheckedThrowingContinuation { cont in + try await withCheckedThrowingContinuation { continuation in var event: MXEvent? request.room.sendEvent( MXEventType(identifier: request.eventType), @@ -117,16 +117,16 @@ struct MXCryptoRequests { localEcho: &event) { response in switch response { case .success(let value): - cont.resume(returning: value) + continuation.resume(returning: value) case .failure(let error): - cont.resume(throwing: error) + continuation.resume(throwing: error) } } } } func backupKeys(request: KeysBackupRequest) async throws -> [AnyHashable: Any] { - return try await performCallbackRequest { continuation in + try await performCallbackRequest { continuation in restClient.sendKeysBackup( request.keysBackupData, version: request.version,