Merge pull request #357 from session-foundation/dev

Release 2.8.6

Session.xcodeproj/project.pbxproj

@@ -7683,7 +7683,7 @@
 				CLANG_WARN__ARC_BRIDGE_CAST_NONARC = YES;
 				CLANG_WARN__DUPLICATE_METHOD_MATCH = YES;
 				CODE_SIGN_IDENTITY = "iPhone Developer";
-				CURRENT_PROJECT_VERSION = 519;
+				CURRENT_PROJECT_VERSION = 526;
 				ENABLE_BITCODE = NO;
 				ENABLE_STRICT_OBJC_MSGSEND = YES;
 				ENABLE_TESTABILITY = YES;
@@ -7720,7 +7720,7 @@
 				GCC_WARN_UNUSED_VARIABLE = YES;
 				HEADER_SEARCH_PATHS = "";
 				IPHONEOS_DEPLOYMENT_TARGET = 13.0;
-				MARKETING_VERSION = 2.8.5;
+				MARKETING_VERSION = 2.8.6;
 				ONLY_ACTIVE_ARCH = YES;
 				OTHER_CFLAGS = (
 					"-fobjc-arc-exceptions",
@@ -7762,7 +7762,7 @@
 				CLANG_WARN__ARC_BRIDGE_CAST_NONARC = YES;
 				CLANG_WARN__DUPLICATE_METHOD_MATCH = YES;
 				CODE_SIGN_IDENTITY = "iPhone Distribution";
-				CURRENT_PROJECT_VERSION = 519;
+				CURRENT_PROJECT_VERSION = 526;
 				ENABLE_BITCODE = NO;
 				ENABLE_STRICT_OBJC_MSGSEND = YES;
 				GCC_NO_COMMON_BLOCKS = YES;
@@ -7794,7 +7794,7 @@
 				GCC_WARN_UNUSED_VARIABLE = YES;
 				HEADER_SEARCH_PATHS = "";
 				IPHONEOS_DEPLOYMENT_TARGET = 13.0;
-				MARKETING_VERSION = 2.8.5;
+				MARKETING_VERSION = 2.8.6;
 				ONLY_ACTIVE_ARCH = NO;
 				OTHER_CFLAGS = (
 					"-DNS_BLOCK_ASSERTIONS=1",

Session/Conversations/ConversationVC.swift

@@ -656,7 +656,7 @@ final class ConversationVC: BaseVC, LibSessionRespondingViewController, Conversa
                     // PagedDatabaseObserver won't have them so we need to force a re-fetch of the current
                     // data to ensure everything is up to date
                     if didReturnFromBackground {
-                        DispatchQueue.global(qos: .background).async {
+                        DispatchQueue.global(qos: .background).asyncAfter(deadline: .now() + 0.01) {
                             self?.viewModel.pagedDataObserver?.reload()
                         }
                     }

Session/Conversations/ConversationViewModel.swift

@@ -198,7 +198,7 @@ public class ConversationViewModel: OWSAudioPlayerDelegate, NavigatableStateHold
         )
 
         // Run the initial query on a background thread so we don't block the push transition
-        DispatchQueue.global(qos: .userInitiated).async { [weak self] in
+        DispatchQueue.global(qos: .userInitiated).asyncAfter(deadline: .now() + 0.01) { [weak self] in
             // If we don't have a `initialFocusedInfo` then default to `.pageBefore` (it'll query
             // from a `0` offset)
             switch (focusedInteractionInfo ?? initialData?.initialUnreadInteractionInfo) {

Session/Home/HomeVC.swift

@@ -46,7 +46,7 @@ final class HomeVC: BaseVC, LibSessionRespondingViewController, UITableViewDataS
 
     // MARK: - UI
 
-    private var tableViewTopConstraint: NSLayoutConstraint!
+    private var tableViewTopConstraint: NSLayoutConstraint?
 
     private lazy var seedReminderView: SeedReminderView = {
         let result = SeedReminderView()
@@ -451,7 +451,7 @@ final class HomeVC: BaseVC, LibSessionRespondingViewController, UITableViewDataS
 
         // Update the 'view seed' UI
         if updatedState.showViewedSeedBanner != self.viewModel.state.showViewedSeedBanner {
-            tableViewTopConstraint.isActive = false
+            tableViewTopConstraint?.isActive = false
             seedReminderView.isHidden = !updatedState.showViewedSeedBanner
 
             if updatedState.showViewedSeedBanner {

Session/Meta/AppDelegate.swift

@@ -564,7 +564,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD
         /// There is a warning which can happen on launch because the Database read can be blocked by another database operation
         /// which could result in this blocking the main thread, as a result we want to check the identity exists on a background thread
         /// and then return to the main thread only when required
-        DispatchQueue.global(qos: .default).async { [weak self] in
+        DispatchQueue.global(qos: .default).asyncAfter(deadline: .now() + 0.01) { [weak self] in
             guard Identity.userExists() else { return }
 
             self?.enableBackgroundRefreshIfNecessary()
@@ -679,7 +679,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD
 
                     /// We want to start observing the changes for the 'HomeVC' and want to wait until we actually get data back before we
                     /// continue as we don't want to show a blank home screen
-                    DispatchQueue.global(qos: .userInitiated).async {
+                    DispatchQueue.global(qos: .userInitiated).asyncAfter(deadline: .now() + 0.01) {
                         viewController.startObservingChanges() {
                             populateHomeScreenTimer.invalidate()
 
@@ -719,7 +719,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD
             /// On application startup the `Storage.read` can be slightly slow while GRDB spins up it's database
             /// read pools (up to a few seconds), since this read is blocking we want to dispatch it to run async to ensure
             /// we don't block user interaction while it's running
-            DispatchQueue.global(qos: .default).async {
+            DispatchQueue.global(qos: .default).asyncAfter(deadline: .now() + 0.01) {
                 let unreadCount: Int = Storage.shared
                     .read { db in try Interaction.fetchUnreadCount(db) }
                     .defaulting(to: 0)
@@ -814,7 +814,10 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD
 
         /// Start the pollers on a background thread so that any database queries they need to run don't
         /// block the main thread
-        DispatchQueue.global(qos: .background).async { [weak self] in
+        ///
+        /// **Note:** We add a delay of `0.01` to prevent potential database re-entrancy if this is triggered
+        /// within the completion block of a database transaction, this gives it the time to complete the transaction
+        DispatchQueue.global(qos: .background).asyncAfter(deadline: .now() + 0.01) { [weak self] in
             self?.poller.start()
 
             guard shouldStartGroupPollers else { return }

Session/Meta/SessionApp.swift

@@ -80,8 +80,9 @@ public struct SessionApp {
             )
         }
 
-        /// The thread should generally exist at the time of calling this method, but on the off chance it doesn't then we need to `fetchOrCreate` it and
-        /// should do it on a background thread just in case something is keeping the DBWrite thread busy as in the past this could cause the app to hang
+        /// The thread should generally exist at the time of calling this method, but on the off chance it doesn't then we need to
+        /// `fetchOrCreate` it and should do it on a background thread just in case something is keeping the DBWrite thread
+        /// busy as in the past this could cause the app to hang
         guard threadInfo?.threadExists == true else {
             DispatchQueue.global(qos: .userInitiated).async {
                 Storage.shared.write { db in

Session/Meta/Translations/Localizable.xcstrings

@@ -304068,7 +304068,7 @@
         "km" : {
           "stringUnit" : {
             "state" : "translated",
-            "value" : "ការបង្កើតគណនីគឺមានភ្លាមៗ, \noofសាន្ត និងមិនបង្ហាញអត្តសញ្ញាណ {emoji}"
+            "value" : "ការបង្កើតគណនីគឺភ្លាមៗ ឥតគិតថ្លៃ និងអនាមិក {emoji}"
           }
         },
         "kn" : {

SessionMessagingKit/Jobs/Types/GarbageCollectionJob.swift

@@ -344,8 +344,11 @@ public enum GarbageCollectionJob: JobExecutor {
                 }
             },
             completion: { _, _ in
-                // Dispatch async so we can swap from the write queue to a read one (we are done writing)
-                queue.async {
+                // Dispatch async so we can swap from the write queue to a read one (we are done
+                // writing), this has to be done after a slight delay to ensure the transaction
+                // provided by the completion block completes first (ie. so we don't hit
+                // re-entrancy issues)
+                queue.asyncAfter(deadline: .now() + 0.01) {
                     // Retrieve a list of all valid attachmnet and avatar file paths
                     struct FileInfo {
                         let attachmentLocalRelativePaths: Set<String>

SessionMessagingKit/Jobs/Types/UpdateProfilePictureJob.swift

@@ -50,10 +50,10 @@ public enum UpdateProfilePictureJob: JobExecutor {
             queue: queue,
             displayPictureUpdate: (profilePictureData.map { .currentUserUploadImageData($0) } ?? .none),
             success: { db in
-                // Need to call the 'success' closure asynchronously on the queue to prevent a reentrancy
-                // issue as it will write to the database and this closure is already called within
-                // another database write
-                queue.async {
+                // Need to call the 'success' closure asynchronously on the queue after a slight
+                // delay to prevent a reentrancy issue as it will write to the database and this
+                // closure is already called within another database write
+                queue.asyncAfter(deadline: .now() + 0.01) {
                     SNLog("[UpdateProfilePictureJob] Profile successfully updated")
                     success(job, false, dependencies)
                 }

SessionMessagingKit/Sending & Receiving/MessageSender.swift

@@ -1054,9 +1054,10 @@ public final class MessageSender {
 
         guard !rowIds.isEmpty else { return error }
 
-        // Need to dispatch to a different thread to prevent a potential db re-entrancy
-        // issue from occuring in some cases
-        DispatchQueue.global(qos: .background).async {
+        // Note: We need to dispatch this after a small 0.01 delay to prevent any potential
+        // re-entrancy issues since the 'asyncMigrate' returns a result containing a DB instance
+        // within a transaction
+        DispatchQueue.global(qos: .background).asyncAfter(deadline: .now() + 0.01, using: dependencies) {
             dependencies.storage.write { db in
                 switch destination {
                     case .syncMessage:

SessionSnodeKit/LibSession/LibSession+Networking.swift

@@ -574,9 +574,12 @@ private extension LibSession {
                 return Log.error("[LibSession] CallbackWrapper called with null context.")
             }
 
-            // Dispatch async so we don't block libSession's internals with Swift logic (which can block other requests)
+            /// Dispatch async so we don't block libSession's internals with Swift logic (which can block other requests), we
+            /// add the `0.01` delay to ensure the closure isn't executed immediately
             let wrapper: CallbackWrapper<Output> = Unmanaged<CallbackWrapper<Output>>.fromOpaque(ctx).takeRetainedValue()
-            DispatchQueue.global(qos: .default).async { [wrapper] in wrapper.resultPublisher.send(output) }
+            DispatchQueue.global(qos: .default).asyncAfter(deadline: .now() + 0.01) { [wrapper] in
+                wrapper.resultPublisher.send(output)
+            }
         }
 
         public func unsafePointer() -> UnsafeMutableRawPointer { Unmanaged.passRetained(self).toOpaque() }

SessionUtilitiesKit/Database/Storage.swift

@@ -329,9 +329,10 @@ open class Storage {
                 }
             }()
 
-            // Note: We need to dispatch this to the next run toop to prevent any potential re-entrancy
-            // issues since the 'asyncMigrate' returns a result containing a DB instance
-            DispatchQueue.global(qos: .userInitiated).async {
+            // Note: We need to dispatch this after a small 0.01 delay to prevent any potential
+            // re-entrancy issues since the 'asyncMigrate' returns a result containing a DB instance
+            // within a transaction
+            DispatchQueue.global(qos: .userInitiated).asyncAfter(deadline: .now() + 0.01, using: dependencies) {
                 migrationCompleted(finalResult)
             }
         }
@@ -524,8 +525,8 @@ open class Storage {
         case valid(DatabaseWriter)
         case invalid(Error)
 
-        init(_ storage: Storage) {
-            switch (storage.isSuspended, storage.isValid, storage.dbWriter) {
+        init(_ storage: Storage?) {
+            switch (storage?.isSuspended, storage?.isValid, storage?.dbWriter) {
                 case (true, _, _): self = .invalid(StorageError.databaseSuspended)
                 case (false, true, .some(let dbWriter)): self = .valid(dbWriter)
                 default: self = .invalid(StorageError.databaseInvalid)
@@ -632,7 +633,7 @@ open class Storage {
     ) -> AnyPublisher<T, Error> {
         switch StorageState(self) {
             case .invalid(let error): return StorageState.logIfNeeded(error, isWrite: true)
-            case .valid(let dbWriter):
+            case .valid:
                 /// **Note:** GRDB does have a `writePublisher` method but it appears to asynchronously trigger
                 /// both the `output` and `complete` closures at the same time which causes a lot of unexpected
                 /// behaviours (this behaviour is apparently expected but still causes a number of odd behaviours in our code
@@ -642,11 +643,19 @@ open class Storage {
                 /// which behaves in a much more expected way than the GRDB `writePublisher` does
                 let info: CallInfo = CallInfo(fileName, functionName, lineNumber, true, self)
                 return Deferred {
-                    Future { resolver in
-                        do { resolver(Result.success(try dbWriter.write(Storage.perform(info: info, updates: updates)))) }
-                        catch {
-                            StorageState.logIfNeeded(error, isWrite: true)
-                            resolver(Result.failure(error))
+                    Future { [weak self] resolver in
+                        /// The `StorageState` may have changed between the creation of the publisher and it actually
+                        /// being executed so we need to check again
+                        switch StorageState(self) {
+                            case .invalid(let error): return StorageState.logIfNeeded(error, isWrite: true)
+                            case .valid(let dbWriter):
+                                do {
+                                    resolver(Result.success(try dbWriter.write(Storage.perform(info: info, updates: updates))))
+                                }
+                                catch {
+                                    StorageState.logIfNeeded(error, isWrite: true)
+                                    resolver(Result.failure(error))
+                                }
                         }
                     }
                 }.eraseToAnyPublisher()
@@ -678,7 +687,7 @@ open class Storage {
     ) -> AnyPublisher<T, Error> {
         switch StorageState(self) {
             case .invalid(let error): return StorageState.logIfNeeded(error, isWrite: false)
-            case .valid(let dbWriter):
+            case .valid:
                 /// **Note:** GRDB does have a `readPublisher` method but it appears to asynchronously trigger
                 /// both the `output` and `complete` closures at the same time which causes a lot of unexpected
                 /// behaviours (this behaviour is apparently expected but still causes a number of odd behaviours in our code
@@ -688,11 +697,19 @@ open class Storage {
                 /// which behaves in a much more expected way than the GRDB `readPublisher` does
                 let info: CallInfo = CallInfo(fileName, functionName, lineNumber, false, self)
                 return Deferred {
-                    Future { resolver in
-                        do { resolver(Result.success(try dbWriter.read(Storage.perform(info: info, updates: value)))) }
-                        catch {
-                            StorageState.logIfNeeded(error, isWrite: false)
-                            resolver(Result.failure(error))
+                    Future { [weak self] resolver in
+                        /// The `StorageState` may have changed between the creation of the publisher and it actually
+                        /// being executed so we need to check again
+                        switch StorageState(self) {
+                            case .invalid(let error): return StorageState.logIfNeeded(error, isWrite: false)
+                            case .valid(let dbWriter):
+                                do {
+                                    resolver(Result.success(try dbWriter.read(Storage.perform(info: info, updates: value))))
+                                }
+                                catch {
+                                    StorageState.logIfNeeded(error, isWrite: false)
+                                    resolver(Result.failure(error))
+                                }
                         }
                     }
                 }.eraseToAnyPublisher()

SessionUtilitiesKit/Database/Types/PagedDatabaseObserver.swift

@@ -158,27 +158,34 @@ public class PagedDatabaseObserver<ObservedTable, T>: TransactionObserver where
         _changesInCommit.performUpdate { $0.inserting(trackedChange) }
     }
 
-    /// We will process all updates which come through this method even if 'onChange' is null because if the UI stops observing and then starts
-    /// again later we don't want to have missed any changes which happened while the UI wasn't subscribed (and doing a full re-query seems painful...)
+    /// We will process all updates which come through this method even if 'onChange' is null because if the UI stops observing and
+    /// then starts again later we don't want to have missed any changes which happened while the UI wasn't subscribed (and doing
+    /// a full re-query seems painful...)
     ///
-    /// **Note:** This function is generally called within the DBWrite thread but we don't actually need write access to process the commit, in order
-    /// to avoid blocking the DBWrite thread we dispatch to a serial `commitProcessingQueue` to process the incoming changes (in the past not doing
-    /// so was resulting in hanging when there was a lot of activity happening)
+    /// **Note:** This function is generally called within the DBWrite thread but we don't actually need write access to process the
+    /// commit, in order to avoid blocking the DBWrite thread we dispatch to a serial `commitProcessingQueue` to process the
+    /// incoming changes (in the past not doing so was resulting in hanging when there was a lot of activity happening)
     public func databaseDidCommit(_ db: Database) {
         // If there were no pending changes in the commit then do nothing
         guard !self.changesInCommit.isEmpty else { return }
 
-        // Since we can't be sure the behaviours of 'databaseDidChange' and 'databaseDidCommit' won't change in
-        // the future we extract and clear the values in 'changesInCommit' since it's '@ThreadSafe' so will different
-        // threads modifying the data resulting in us missing a change
+        // Since we can't be sure the behaviours of 'databaseDidChange' and 'databaseDidCommit'
+        // won't change in the future we extract and clear the values in 'changesInCommit' since
+        // it's '@ThreadSafe' so will different threads modifying the data resulting in us
+        // missing a change
         var committedChanges: Set<PagedData.TrackedChange> = []
 
         self._changesInCommit.performUpdate { cachedChanges in
             committedChanges = cachedChanges
             return []
         }
 
-        commitProcessingQueue.async { [weak self] in
+        // This looks odd but if we just use `commitProcessingQueue.async` then the code can
+        // get executed immediately wihch can result in a new transaction being started whilst
+        // we are still within the transaction wrapping `databaseDidCommit` (which we don't
+        // want), by adding this tiny 0.01 delay we should be giving it enough time to finish
+        // processing the current transaction
+        commitProcessingQueue.asyncAfter(deadline: .now() + 0.01) { [weak self] in
             self?.processDatabaseCommit(committedChanges: committedChanges)
         }
     }