Merge remote-tracking branch 'upstream/dev' into feature/groups-rebuild

# Conflicts:
#	Session.xcodeproj/project.pbxproj
#	Session/Home/HomeVC.swift
#	Session/Meta/AppDelegate.swift
#	Session/Meta/SessionApp.swift
#	SessionMessagingKit/Jobs/Types/UpdateProfilePictureJob.swift
#	SessionMessagingKit/Sending & Receiving/MessageSender.swift
#	SessionUtilitiesKit/JobRunner/JobRunner.swift

Session/Home/HomeVC.swift

@@ -46,8 +46,8 @@ public final class HomeVC: BaseVC, LibSessionRespondingViewController, UITableVi
     
     // MARK: - UI
     
-    private var tableViewTopConstraint: NSLayoutConstraint!
+    private var tableViewTopConstraint: NSLayoutConstraint?
-    private var loadingConversationsLabelTopConstraint: NSLayoutConstraint!
+    private var loadingConversationsLabelTopConstraint: NSLayoutConstraint?
     private var navBarProfileView: ProfilePictureView?
     
     private lazy var seedReminderView: SeedReminderView = {
@@ -452,8 +452,8 @@ public final class HomeVC: BaseVC, LibSessionRespondingViewController, UITableVi
         
         // Update the 'view seed' UI
         if updatedState.showViewedSeedBanner != self.viewModel.state.showViewedSeedBanner {
-            tableViewTopConstraint.isActive = false
+            tableViewTopConstraint?.isActive = false
-            loadingConversationsLabelTopConstraint.isActive = false
+            loadingConversationsLabelTopConstraint?.isActive = false
             seedReminderView.isHidden = !updatedState.showViewedSeedBanner
 
             if updatedState.showViewedSeedBanner {

Session/Meta/AppDelegate.swift

@@ -597,7 +597,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, dependencies] in
+        DispatchQueue.global(qos: .default).asyncAfter(deadline: .now() + 0.01) { [weak self, dependencies] in
             guard dependencies[cache: .onboarding].state == .completed else { return }
             
             self?.enableBackgroundRefreshIfNecessary()
@@ -734,7 +734,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() {
                             longRunningStartupTimoutCancellable.cancel()
                             
@@ -774,7 +774,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(using: dependencies) {
+            DispatchQueue.global(qos: .default).asyncAfter(deadline: .now() + 0.01) {
                 let unreadCount: Int = dependencies[singleton: .storage]
                     .read { db in try Interaction.fetchUnreadCount(db, using: dependencies) }
                     .defaulting(to: 0)
@@ -870,7 +870,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 { [dependencies] 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) { [dependencies] in
             dependencies[singleton: .currentUserPoller].startIfNeeded()
             
             guard shouldStartGroupPollers else { return }

Session/Meta/SessionApp.swift

@@ -96,8 +96,9 @@ public class SessionApp: SessionAppType {
             return (SessionThread.filter(id: threadId).isNotEmpty(db), isMessageRequest)
         }
         
-        /// 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
+        /// The thread should generally exist at the time of calling this method, but on the off chance it doesn't then we need to
-        /// 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
+        /// `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
         creatingThreadIfNeededThenRunOnMain(
             threadId: threadId,
             variant: variant,

Session/Meta/Translations/Localizable.xcstrings

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

SessionMessagingKit/Jobs/GarbageCollectionJob.swift

@@ -354,8 +354,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)
+                // Dispatch async so we can swap from the write queue to a read one (we are done
-                queue.async {
+                // 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/ProcessPendingGroupMemberRemovalsJob.swift

@@ -273,7 +273,7 @@ public enum ProcessPendingGroupMemberRemovalsJob: JobExecutor {
                                         }
                                     },
                                     completion: { db, result in
-                                        queue.async(using: dependencies) {
+                                        queue.asyncAfter(deadline: .now() + 0.01, using: dependencies) {
                                             switch result {
                                                 case .success: success(job, false)
                                                 case .failure(let error): failure(job, error, false)

SessionMessagingKit/Sending & Receiving/MessageSender.swift

@@ -775,9 +775,10 @@ public final class MessageSender {
         
         guard !rowIds.isEmpty else { return error }
         
-        // Need to dispatch to a different thread as this function is most commonly called within a read
+        // Note: We need to dispatch this after a small 0.01 delay to prevent any potential
-        // thread and we want to write to the db and don't want to run into a re-entrancy error
+        // re-entrancy issues since the 'asyncMigrate' returns a result containing a DB instance
-        DispatchQueue.global(qos: .background).async(using: dependencies) {
+        // within a transaction
+        DispatchQueue.global(qos: .background).asyncAfter(deadline: .now() + 0.01, using: dependencies) {
             dependencies[singleton: .storage].write { db in
                 switch destination {
                     case .syncMessage:

SessionSnodeKit/LibSession/LibSession+Networking.swift

@@ -414,9 +414,12 @@ private extension LibSessionNetwork {
                 return Log.error(.network, "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

@@ -386,9 +386,10 @@ open class Storage {
                 }
             }()
             
-            // Note: We need to dispatch this to the next run toop to prevent any potential re-entrancy
+            // Note: We need to dispatch this after a small 0.01 delay to prevent any potential
-            // issues since the 'asyncMigrate' returns a result containing a DB instance
+            // re-entrancy issues since the 'asyncMigrate' returns a result containing a DB instance
-            DispatchQueue.global(qos: .userInitiated).async {
+            // within a transaction
+            DispatchQueue.global(qos: .userInitiated).asyncAfter(deadline: .now() + 0.01, using: dependencies) {
                 migrationCompleted(finalResult)
             }
         }
@@ -592,8 +593,8 @@ open class Storage {
         case valid(DatabaseWriter)
         case invalid(Error)
         
-        init(_ storage: Storage) {
+        init(_ storage: Storage?) {
-            switch (storage.isSuspended, storage.isValid, storage.dbWriter) {
+            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)
@@ -697,7 +698,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
@@ -707,11 +708,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
+                    Future { [weak self] resolver in
-                        do { resolver(Result.success(try dbWriter.write(Storage.perform(info: info, updates: updates)))) }
+                        /// The `StorageState` may have changed between the creation of the publisher and it actually
-                        catch {
+                        /// being executed so we need to check again
-                            StorageState.logIfNeeded(error, isWrite: true)
+                        switch StorageState(self) {
-                            resolver(Result.failure(error))
+                            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()
@@ -741,7 +750,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
@@ -751,11 +760,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
+                    Future { [weak self] resolver in
-                        do { resolver(Result.success(try dbWriter.read(Storage.perform(info: info, updates: value)))) }
+                        /// The `StorageState` may have changed between the creation of the publisher and it actually
-                        catch {
+                        /// being executed so we need to check again
-                            StorageState.logIfNeeded(error, isWrite: false)
+                        switch StorageState(self) {
-                            resolver(Result.failure(error))
+                            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

@@ -164,19 +164,21 @@ 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
+    /// We will process all updates which come through this method even if 'onChange' is null because if the UI stops observing and
-    /// 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...)
+    /// 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
+    /// **Note:** This function is generally called within the DBWrite thread but we don't actually need write access to process the
-    /// to avoid blocking the DBWrite thread we dispatch to a serial `commitProcessingQueue` to process the incoming changes (in the past not doing
+    /// commit, in order to avoid blocking the DBWrite thread we dispatch to a serial `commitProcessingQueue` to process the
-    /// so was resulting in hanging when there was a lot of activity happening)
+    /// 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
+        // Since we can't be sure the behaviours of 'databaseDidChange' and 'databaseDidCommit'
-        // the future we extract and clear the values in 'changesInCommit' since it's '@ThreadSafe' so will different
+        // won't change in the future we extract and clear the values in 'changesInCommit' since
-        // threads modifying the data resulting in us missing a change
+        // 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
@@ -184,7 +186,12 @@ public class PagedDatabaseObserver<ObservedTable, T>: TransactionObserver where
             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)
         }
     }

SessionUtilitiesKit/JobRunner/JobRunner.swift

@@ -734,7 +734,9 @@ public final class JobRunner: JobRunnerType {
         
         // Don't add to the queue if the JobRunner isn't ready (it's been saved to the db so it'll be loaded
         // once the queue actually get started later)
-        guard canAddToQueue(updatedJob) || jobQueue?.isRunningInBackgroundTask == true else { return updatedJob }
+        guard canAddToQueue(updatedJob) || jobQueue?.isRunningInBackgroundTask == true else {
+            return updatedJob
+        }
         
         // The queue is ready or running in a background task so we can add the job
         jobQueue?.add(db, job: updatedJob, canStartJob: canStartJob)
@@ -1337,10 +1339,14 @@ public final class JobQueue: Hashable {
         guard canStart?(self) == true || isRunningInBackgroundTask else { return }
         guard forceWhenAlreadyRunning || !isRunning || isRunningInBackgroundTask else { return }
         
-        // The JobRunner runs synchronously we need to ensure this doesn't start
+        // The JobRunner runs synchronously so we need to ensure this doesn't start on the main
-        // on the main thread (if it is on the main thread then swap to a different thread)
+        // thread and do so by creating a number of background queues to run the jobs on, if this
+        // function was called on the wrong queue then we need to dispatch to the correct one
         guard DispatchQueue.with(key: queueKey, matches: queueContext, using: dependencies) else {
-            internalQueue.async(using: dependencies) { [weak self] in
+            // Note: We need to dispatch this after a small 0.01 delay to prevent any potential
+            // re-entrancy issues since the `start` function can be called within an existing
+            // database transaction (eg. via `db.afterNextTransactionNestedOnce`)
+            internalQueue.asyncAfter(deadline: .now() + 0.01, using: dependencies) { [weak self] in
                 self?.start(forceWhenAlreadyRunning: forceWhenAlreadyRunning)
             }
             return

SessionUtilitiesKit/Utilities/ThreadSafe.swift

@@ -18,7 +18,7 @@ import Foundation
 /// as `ThreadSafeType` (reference types or structs which have `mutating` functions **should not** use this mechanism
 /// as it cannot ensure thread safety for those types)
 @propertyWrapper
-public struct ThreadSafe<Value: ThreadSafeType> {
+public class ThreadSafe<Value: ThreadSafeType> {
     private var value: Value
     private let lock: ReadWriteLock = ReadWriteLock()
 
@@ -49,17 +49,17 @@ public struct ThreadSafe<Value: ThreadSafeType> {
     
     // MARK: - Functions
 
-    public mutating func performUpdateAndMap<T>(_ closure: (Value) -> (Value, T)) -> T {
+    public func performUpdateAndMap<T>(_ closure: (Value) -> (Value, T)) -> T {
         return try! performInternal { closure($0) }
     }
     
-    public mutating func performUpdateAndMap<T>(_ closure: (Value) throws -> (Value, T)) throws -> T {
+    public func performUpdateAndMap<T>(_ closure: (Value) throws -> (Value, T)) throws -> T {
         return try performInternal { try closure($0) }
     }
     
     // MARK: - Internal Functions
     
-    @discardableResult private mutating func performInternal<T>(_ mutation: (Value) throws -> (Value, T)) throws -> T {
+    @discardableResult private func performInternal<T>(_ mutation: (Value) throws -> (Value, T)) throws -> T {
         lock.writeLock()
         defer { lock.unlock() }