Fixed a few more issues

• Minor tweak to config load order
• Pulled across reentrancy PR fixes
• Fixed an issue where some UI changes were occurring on background threads (causing crashes)
• Fixed an issue where editing a group name/description was performing a blocking action but not showing a loading indicator
• Fixed an issue where the group display picture modal would have it's "save" button enabled even when the picture hadn't been changed

Session.xcodeproj/project.pbxproj

@@ -7919,7 +7919,7 @@
 				CLANG_WARN__ARC_BRIDGE_CAST_NONARC = YES;
 				CLANG_WARN__DUPLICATE_METHOD_MATCH = YES;
 				CODE_SIGN_IDENTITY = "iPhone Developer";
-				CURRENT_PROJECT_VERSION = 532;
+				CURRENT_PROJECT_VERSION = 533;
 				ENABLE_BITCODE = NO;
 				ENABLE_STRICT_OBJC_MSGSEND = YES;
 				ENABLE_TESTABILITY = YES;
@@ -7995,7 +7995,7 @@
 				CLANG_WARN__ARC_BRIDGE_CAST_NONARC = YES;
 				CLANG_WARN__DUPLICATE_METHOD_MATCH = YES;
 				CODE_SIGN_IDENTITY = "iPhone Distribution";
-				CURRENT_PROJECT_VERSION = 532;
+				CURRENT_PROJECT_VERSION = 533;
 				ENABLE_BITCODE = NO;
 				ENABLE_MODULE_VERIFIER = YES;
 				ENABLE_STRICT_OBJC_MSGSEND = YES;

Session/Closed Groups/EditGroupViewModel.swift

@@ -562,8 +562,8 @@ class EditGroupViewModel: SessionTableViewModel, NavigatableStateHolder, Editabl
                                 let viewController = ModalActivityIndicatorViewController() { modalActivityIndicator in
                                     SnodeAPI
                                         .getSessionID(for: inviteByIdValue, using: dependencies)
-                                        .subscribe(on: DispatchQueue.global(qos: .userInitiated))
+                                        .subscribe(on: DispatchQueue.global(qos: .userInitiated), using: dependencies)
-                                        .receive(on: DispatchQueue.main)
+                                        .receive(on: DispatchQueue.main, using: dependencies)
                                         .sinkUntilComplete(
                                             receiveCompletion: { result in
                                                 switch result {
@@ -620,6 +620,8 @@ class EditGroupViewModel: SessionTableViewModel, NavigatableStateHolder, Editabl
                     allowAccessToHistoricMessages: dependencies[feature: .updatedGroupsAllowHistoricAccessOnInvite],
                     using: dependencies
                 )
+                .subscribe(on: DispatchQueue.global(qos: .userInitiated), using: dependencies)
+                .receive(on: DispatchQueue.main, using: dependencies)
                 .sinkUntilComplete(
                     receiveCompletion: { [weak self] result in
                         modalActivityIndicator.dismiss {
@@ -691,6 +693,8 @@ class EditGroupViewModel: SessionTableViewModel, NavigatableStateHolder, Editabl
                     memberIds: memberIds,
                     using: dependencies
                 )
+                .subscribe(on: DispatchQueue.global(qos: .userInitiated), using: dependencies)
+                .receive(on: DispatchQueue.main, using: dependencies)
                 .sinkUntilComplete(
                     receiveCompletion: { [weak self] result in
                         modalActivityIndicator.dismiss {
@@ -840,8 +844,8 @@ class EditGroupViewModel: SessionTableViewModel, NavigatableStateHolder, Editabl
                                         using: dependencies
                                     )
                                     .eraseToAnyPublisher()
-                                    .subscribe(on: DispatchQueue.global(qos: .userInitiated))
+                                    .subscribe(on: DispatchQueue.global(qos: .userInitiated), using: dependencies)
-                                    .receive(on: DispatchQueue.main)
+                                    .receive(on: DispatchQueue.main, using: dependencies)
                                     .sinkUntilComplete(
                                         receiveCompletion: { [weak self] result in
                                             modalActivityIndicator.dismiss(completion: {

Session/Conversations/Settings/ThreadSettingsViewModel.swift

@@ -1002,7 +1002,6 @@ class ThreadSettingsViewModel: SessionTableViewModel, NavigatableStateHolder, Ob
             _ memberInfo: [(id: String, profile: Profile?)],
             isRetry: Bool
         ) {
-            let viewController = ModalActivityIndicatorViewController(canCancel: false) { [dependencies, threadId] modalActivityIndicator in
             MessageSender
                 .promoteGroupMembers(
                     groupSessionId: SessionId(.group, hex: threadId),
@@ -1010,9 +1009,11 @@ class ThreadSettingsViewModel: SessionTableViewModel, NavigatableStateHolder, Ob
                     isRetry: isRetry,
                     using: dependencies
                 )
+                .showingBlockingLoading(in: self.navigatableState)
+                .subscribe(on: DispatchQueue.global(qos: .userInitiated), using: dependencies)
+                .receive(on: DispatchQueue.main, using: dependencies)
                 .sinkUntilComplete(
-                        receiveCompletion: { result in
+                    receiveCompletion: { [threadId, dependencies] result in
-                            modalActivityIndicator.dismiss {
                         switch result {
                             case .failure:
                                 viewModel?.transitionToScreen(
@@ -1063,11 +1064,8 @@ class ThreadSettingsViewModel: SessionTableViewModel, NavigatableStateHolder, Ob
                                 )
                         }
                     }
-                        }
                 )
         }
-            viewModel?.transitionToScreen(viewController, transitionType: .present)
-        }
         
         /// Show the selection list
         self.transitionToScreen(
@@ -1317,6 +1315,9 @@ class ThreadSettingsViewModel: SessionTableViewModel, NavigatableStateHolder, Ob
                                 groupDescription: finalDescription,
                                 using: dependencies
                             )
+                            .showingBlockingLoading(in: self?.navigatableState)
+                            .subscribe(on: DispatchQueue.global(qos: .userInitiated), using: dependencies)
+                            .receive(on: DispatchQueue.main, using: dependencies)
                             .sinkUntilComplete(
                                 receiveCompletion: { [weak self] result in
                                     switch result {
@@ -1423,59 +1424,68 @@ class ThreadSettingsViewModel: SessionTableViewModel, NavigatableStateHolder, Ob
             default: break
         }
         
-        func performChanges(_ viewController: ModalActivityIndicatorViewController, _ displayPictureUpdate: DisplayPictureManager.Update) {
+        Just(displayPictureUpdate)
-            let existingFileName: String? = dependencies[singleton: .storage].read { [threadId] db in
+            .setFailureType(to: Error.self)
+            .flatMap { [dependencies] update -> AnyPublisher<DisplayPictureManager.Update, Error> in
+                switch displayPictureUpdate {
+                    case .none, .currentUserRemove, .currentUserUploadImageData, .currentUserUpdateTo,
+                        .contactRemove, .contactUpdateTo:
+                        return Fail(error: AttachmentError.invalidStartState).eraseToAnyPublisher()
+                        
+                    case .groupRemove, .groupUpdateTo:
+                        return Just(displayPictureUpdate)
+                            .setFailureType(to: Error.self)
+                            .eraseToAnyPublisher()
+                        
+                    case .groupUploadImageData(let data):
+                        return dependencies[singleton: .displayPictureManager]
+                            .prepareAndUploadDisplayPicture(imageData: data)
+                            .map { url, fileName, key -> DisplayPictureManager.Update in
+                                .groupUpdateTo(url: url, key: key, fileName: fileName)
+                            }
+                            .mapError { $0 as Error }
+                            .eraseToAnyPublisher()
+                }
+            }
+            .flatMapStorageReadPublisher(using: dependencies) { [threadId] db, displayPictureUpdate -> (DisplayPictureManager.Update, String?) in
+                (
+                    displayPictureUpdate,
                     try? ClosedGroup
                         .filter(id: threadId)
                         .select(.displayPictureFilename)
                         .asRequest(of: String.self)
                         .fetchOne(db)
+                )
             }
-            
+            .flatMap { [threadId, dependencies] displayPictureUpdate, existingFileName -> AnyPublisher<String?, Error> in
                 MessageSender
                     .updateGroup(
                         groupSessionId: threadId,
                         displayPictureUpdate: displayPictureUpdate,
                         using: dependencies
                     )
-                .sinkUntilComplete(
+                    .map { _ in existingFileName }
-                    receiveCompletion: { [dependencies] result in
+                    .eraseToAnyPublisher()
+            }
+            .handleEvents(
+                receiveOutput: { [dependencies] existingFileName in
                     // Remove any cached avatar image value
                     if let existingFileName: String = existingFileName {
                         dependencies.mutate(cache: .displayPicture) { $0.imageData[existingFileName] = nil }
                     }
-                        
-                        DispatchQueue.main.async {
-                            viewController.dismiss(completion: {
-                                onComplete()
-                            })
-                        }
                 }
             )
-        }
+            .showingBlockingLoading(in: self.navigatableState)
-        
+            .subscribe(on: DispatchQueue.global(qos: .userInitiated), using: dependencies)
-        let viewController = ModalActivityIndicatorViewController(canCancel: false) { [weak self, dependencies] viewController in
-            switch displayPictureUpdate {
-                case .none, .currentUserRemove, .currentUserUploadImageData, .currentUserUpdateTo,
-                    .contactRemove, .contactUpdateTo:
-                    viewController.dismiss(animated: true) // Shouldn't get called
-                
-                case .groupRemove, .groupUpdateTo: performChanges(viewController, displayPictureUpdate)
-                case .groupUploadImageData(let data):
-                    dependencies[singleton: .displayPictureManager]
-                        .prepareAndUploadDisplayPicture(imageData: data)
-                        .subscribe(on: DispatchQueue.global(qos: .background), using: dependencies)
             .receive(on: DispatchQueue.main, using: dependencies)
             .sinkUntilComplete(
-                            receiveCompletion: { result in
+                receiveCompletion: { [weak self] result in
                     switch result {
-                                    case .finished: break
                         case .failure(let error):
-                                        viewController.dismiss {
                             let message: String = {
                                 switch (displayPictureUpdate, error) {
                                     case (.groupRemove, _): return "profileDisplayPictureRemoveError".localized()
-                                                    case (_, .uploadMaxFileSizeExceeded):
+                                    case (_, DisplayPictureError.uploadMaxFileSizeExceeded):
                                         return "profileDisplayPictureSizeError".localized()
                                     
                                     default: return "errorConnection".localized()
@@ -1494,20 +1504,12 @@ class ThreadSettingsViewModel: SessionTableViewModel, NavigatableStateHolder, Ob
                                 ),
                                 transitionType: .present
                             )
+                        
+                        case .finished: onComplete()
                     }
                 }
-                            },
-                            receiveValue: { url, fileName, key in
-                                performChanges(
-                                    viewController,
-                                    .groupUpdateTo(url: url, key: key, fileName: fileName)
-                                )
-                            }
             )
     }
-        }
-        self.transitionToScreen(viewController, transitionType: .present)
-    }
     
     private func updateBlockedState(
         from oldBlockedState: Bool,

SessionMessagingKit/Database/Models/ConfigDump.swift

@@ -120,10 +120,15 @@ public extension ConfigDump.Variant {
     
     /// This value defines the order that the ConfigDump records should be loaded in, we need to load the `groupKeys`
     /// config _after_ the `groupInfo` and `groupMembers` configs as it requires those to be passed as arguments
+    ///
+    /// We also may as well load the user configs first (shouldn't make a difference but makes things easier to debug when
+    /// the user configs are loaded first
     var loadOrder: Int {
         switch self {
-            case .groupKeys: return 1
+            case .invalid: return 3
-            default: return 0
+            case .groupKeys: return 2
+            case .groupInfo, .groupMembers: return 1
+            case .userProfile, .contacts, .convoInfoVolatile, .userGroups: return 0
         }
     }
     

SessionUIKit/Components/ConfirmationModal.swift

@@ -701,7 +701,10 @@ public extension ConfirmationModal.Info {
         func isValid(with info: ConfirmationModal.Info) -> Bool { boolValue }
     }
     
+    /// The `AfterChangeValidator` will also return `false` for the initial validity check and will use the provided
+    /// value for subsequent checks
     class AfterChangeValidator: ButtonValidator {
+        private(set) var hasDoneInitialValidCheck: Bool = false
         let isValid: (ConfirmationModal.Info) -> Bool
         
         required public init(booleanLiteral value: BooleanLiteralType) {
@@ -717,7 +720,14 @@ public extension ConfirmationModal.Info {
             super.init(booleanLiteral: false)
         }
         
-        public override func isValid(with info: ConfirmationModal.Info) -> Bool { return self.isValid(info) }
+        public override func isValid(with info: ConfirmationModal.Info) -> Bool {
+            guard hasDoneInitialValidCheck else {
+                hasDoneInitialValidCheck = true
+                return false
+            }
+            
+            return self.isValid(info)
+        }
     }
         
     // MARK: - ShowCondition

SessionUtilitiesKit/Database/Storage.swift

@@ -48,6 +48,9 @@ open class Storage {
     /// When attempting to do a write the transaction will wait this long to acquite a lock before failing
     private static let writeTransactionStartTimeout: TimeInterval = 5
     
+    /// If a transaction takes longer than this duration then we should fail the transaction rather than keep hanging
+    private static let transactionDeadlockTimeoutSeconds: Int = 5
+    
     private static var sharedDatabaseDirectoryPath: String { "\(SessionFileManager.nonInjectedAppSharedDataDirectoryPath)/database" }
     private static var databasePath: String { "\(Storage.sharedDatabaseDirectoryPath)/\(Storage.dbFileName)" }
     private static var databasePathShm: String { "\(Storage.sharedDatabaseDirectoryPath)/\(Storage.dbFileName)-shm" }
@@ -376,7 +379,7 @@ open class Storage {
         // Note: The non-async migration should only be used for unit tests
         guard async else { return migrationCompleted(Result(catching: { try migrator.migrate(dbWriter) })) }
         
-        migrator.asyncMigrate(dbWriter) { result in
+        migrator.asyncMigrate(dbWriter) { [dependencies] result in
             let finalResult: Result<Void, Error> = {
                 switch result {
                     case .failure(let error): return .failure(error)
@@ -384,9 +387,13 @@ open class Storage {
                 }
             }()
             
+            // Note: We need to dispatch this to the next run toop to prevent blocking if the callback
+            // performs subsequent database operations
+            DispatchQueue.global(qos: .userInitiated).async(using: dependencies) {
                 migrationCompleted(finalResult)
             }
         }
+    }
     
     public func willStartMigration(
         _ db: Database,
@@ -670,7 +677,10 @@ open class Storage {
         
         /// Perform the actual operation
         switch (StorageState(info.storage), info.isWrite) {
-            case (.invalid(let error), _): result = .failure(error)
+            case (.invalid(let error), _):
+                result = .failure(error)
+                semaphore?.signal()
+                
             case (.valid(let dbWriter), true):
                 dbWriter.asyncWrite(
                     { db in result = .success(try Storage.track(db, info, operation)) },
@@ -705,7 +715,13 @@ open class Storage {
         
         /// If this is a synchronous operation then `semaphore` will exist and will block here waiting on the signal from one of the
         /// above closures to be sent
-        semaphore?.wait()
+        let semaphoreResult: DispatchTimeoutResult? = semaphore?.wait(timeout: .now() + .seconds(Storage.transactionDeadlockTimeoutSeconds))
+
+        /// If the transaction timed out then log the error and report a failure
+        guard semaphoreResult != .timedOut else {
+            StorageState.logIfNeeded(StorageError.transactionDeadlockTimeout, isWrite: info.isWrite)
+            return .failure(StorageError.transactionDeadlockTimeout)
+        }
         
         if !info.isAsync { logErrorIfNeeded(result) }
         return result

SessionUtilitiesKit/Database/StorageError.swift

@@ -14,6 +14,7 @@ public enum StorageError: Error {
     case keySpecInaccessible
     case decodingFailed
     case invalidQueryResult
+    case transactionDeadlockTimeout
     
     case failedToSave
     case objectNotFound