From b815a9f348fea35b9a43e5f66b10efa3696bfb6c Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Thu, 24 Mar 2022 15:08:24 +1100 Subject: [PATCH] Fixed up a potential threading issue with the sync configuration logic Moved all the sync configuration calls to be within the existing 'write' blocks instead of waiting until the completion --- .../ConversationVC+Interaction.swift | 29 +++---- .../Views & Modals/BlockedModal.swift | 24 +++--- Session/Home/HomeVC.swift | 16 ++-- .../MessageRequestsViewController.swift | 86 +++++++++---------- 4 files changed, 73 insertions(+), 82 deletions(-) diff --git a/Session/Conversations/ConversationVC+Interaction.swift b/Session/Conversations/ConversationVC+Interaction.swift index ecd73f624..661584fab 100644 --- a/Session/Conversations/ConversationVC+Interaction.swift +++ b/Session/Conversations/ConversationVC+Interaction.swift @@ -48,15 +48,14 @@ extension ConversationVC : InputViewDelegate, MessageCellDelegate, ContextMenuAc self.blockedBanner.alpha = 0 }, completion: { _ in if let contact: Contact = Storage.shared.getContact(with: publicKey) { - Storage.shared.write( - with: { transaction in - contact.isBlocked = false - Storage.shared.setContact(contact, using: transaction) - }, - completion: { - MessageSender.syncConfiguration(forceSyncNow: true).retainUntilComplete() - } - ) + Storage.shared.write { transaction in + guard let transaction = transaction as? YapDatabaseReadWriteTransaction else { return } + + contact.isBlocked = false + Storage.shared.setContact(contact, using: transaction) + + MessageSender.syncConfiguration(forceSyncNow: true, with: transaction).retainUntilComplete() + } } }) } @@ -1149,6 +1148,9 @@ extension ConversationVC { contact.didApproveMe = (contact.didApproveMe || !isNewThread) Storage.shared.setContact(contact, using: transaction) + // Send a sync message with the details of the contact + MessageSender.syncConfiguration(forceSyncNow: true).retainUntilComplete() + // Hide the 'messageRequestView' since the request has been approved and force a config // sync to propagate the contact approval state (both must run on the main thread) DispatchQueue.main.async { [weak self] in @@ -1182,9 +1184,6 @@ extension ConversationVC { newViewControllers.remove(at: messageRequestsIndex) self?.navigationController?.setViewControllers(newViewControllers, animated: false) } - - // Send a sync message with the details of the contact - MessageSender.syncConfiguration(forceSyncNow: true).retainUntilComplete() } } } @@ -1245,11 +1244,11 @@ extension ConversationVC { // Delete all thread content self?.thread.removeAllThreadInteractions(with: transaction) self?.thread.remove(with: transaction) + + // Force a config sync and pop to the previous screen (both must run on the main thread) + MessageSender.syncConfiguration(forceSyncNow: true, with: transaction).retainUntilComplete() }, completion: { [weak self] in - // Force a config sync and pop to the previous screen (both must run on the main thread) - MessageSender.syncConfiguration(forceSyncNow: true).retainUntilComplete() - DispatchQueue.main.async { self?.navigationController?.popViewController(animated: true) } diff --git a/Session/Conversations/Views & Modals/BlockedModal.swift b/Session/Conversations/Views & Modals/BlockedModal.swift index e1d532700..11135e6f6 100644 --- a/Session/Conversations/Views & Modals/BlockedModal.swift +++ b/Session/Conversations/Views & Modals/BlockedModal.swift @@ -1,5 +1,6 @@ +import SessionMessagingKit -final class BlockedModal : Modal { +final class BlockedModal: Modal { private let publicKey: String // MARK: Lifecycle @@ -65,19 +66,16 @@ final class BlockedModal : Modal { @objc private func unblock() { let publicKey: String = self.publicKey - Storage.shared.write( - with: { transaction in - guard let contact: Contact = Storage.shared.getContact(with: publicKey, using: transaction) else { - return - } - - contact.isBlocked = false - Storage.shared.setContact(contact, using: transaction) - }, - completion: { - MessageSender.syncConfiguration(forceSyncNow: true).retainUntilComplete() + Storage.shared.write { transaction in + guard let transaction = transaction as? YapDatabaseReadWriteTransaction, let contact: Contact = Storage.shared.getContact(with: publicKey, using: transaction) else { + return } - ) + + contact.isBlocked = false + Storage.shared.setContact(contact, using: transaction as Any) + + MessageSender.syncConfiguration(forceSyncNow: true, with: transaction).retainUntilComplete() + } presentingViewController?.dismiss(animated: true, completion: nil) } diff --git a/Session/Home/HomeVC.swift b/Session/Home/HomeVC.swift index d1ad5aad5..f401d0bbe 100644 --- a/Session/Home/HomeVC.swift +++ b/Session/Home/HomeVC.swift @@ -511,19 +511,19 @@ final class HomeVC : BaseVC, UITableViewDataSource, UITableViewDelegate, NewConv let block = UITableViewRowAction(style: .normal, title: NSLocalizedString("BLOCK_LIST_BLOCK_BUTTON", comment: "")) { _, _ in Storage.shared.write( with: { transaction in - guard let contact: Contact = Storage.shared.getContact(with: publicKey, using: transaction) else { + guard let transaction = transaction as? YapDatabaseReadWriteTransaction, let contact: Contact = Storage.shared.getContact(with: publicKey, using: transaction) else { return } contact.isBlocked = true - Storage.shared.setContact(contact, using: transaction) + Storage.shared.setContact(contact, using: transaction as Any) + + MessageSender.syncConfiguration(forceSyncNow: true, with: transaction).retainUntilComplete() }, completion: { DispatchQueue.main.async { tableView.reloadRows(at: [ indexPath ], with: UITableView.RowAnimation.fade) } - - MessageSender.syncConfiguration(forceSyncNow: true).retainUntilComplete() } ) } @@ -531,19 +531,19 @@ final class HomeVC : BaseVC, UITableViewDataSource, UITableViewDelegate, NewConv let unblock = UITableViewRowAction(style: .normal, title: NSLocalizedString("BLOCK_LIST_UNBLOCK_BUTTON", comment: "")) { _, _ in Storage.shared.write( with: { transaction in - guard let contact: Contact = Storage.shared.getContact(with: publicKey, using: transaction) else { + guard let transaction = transaction as? YapDatabaseReadWriteTransaction, let contact: Contact = Storage.shared.getContact(with: publicKey, using: transaction) else { return } contact.isBlocked = false - Storage.shared.setContact(contact, using: transaction) + Storage.shared.setContact(contact, using: transaction as Any) + + MessageSender.syncConfiguration(forceSyncNow: true, with: transaction).retainUntilComplete() }, completion: { DispatchQueue.main.async { tableView.reloadRows(at: [ indexPath ], with: UITableView.RowAnimation.fade) } - - MessageSender.syncConfiguration(forceSyncNow: true).retainUntilComplete() } ) } diff --git a/Session/Home/Message Requests/MessageRequestsViewController.swift b/Session/Home/Message Requests/MessageRequestsViewController.swift index 0d3e61a13..5d4a21387 100644 --- a/Session/Home/Message Requests/MessageRequestsViewController.swift +++ b/Session/Home/Message Requests/MessageRequestsViewController.swift @@ -336,38 +336,35 @@ class MessageRequestsViewController: BaseVC, UITableViewDelegate, UITableViewDat let alertVC: UIAlertController = UIAlertController(title: NSLocalizedString("MESSAGE_REQUESTS_CLEAR_ALL_CONFIRMATION_TITLE", comment: ""), message: nil, preferredStyle: .actionSheet) alertVC.addAction(UIAlertAction(title: NSLocalizedString("MESSAGE_REQUESTS_CLEAR_ALL_CONFIRMATION_ACTON", comment: ""), style: .destructive) { _ in // Clear the requests - Storage.write( - with: { [weak self] transaction in - threads.forEach { thread in - if let uniqueId: String = thread.uniqueId { - Storage.shared.cancelPendingMessageSendJobs(for: uniqueId, using: transaction) - } - - self?.updateContactAndThread(thread: thread, with: transaction) { threadNeedsSync in - if threadNeedsSync { - needsSync = true - } - } - - // Block the contact - if - let sessionId: String = (thread as? TSContactThread)?.contactSessionID(), - !thread.isBlocked(), - let contact: Contact = Storage.shared.getContact(with: sessionId, using: transaction) - { - contact.isBlocked = true - Storage.shared.setContact(contact, using: transaction) + Storage.write { [weak self] transaction in + threads.forEach { thread in + if let uniqueId: String = thread.uniqueId { + Storage.shared.cancelPendingMessageSendJobs(for: uniqueId, using: transaction) + } + + self?.updateContactAndThread(thread: thread, with: transaction) { threadNeedsSync in + if threadNeedsSync { needsSync = true } } - }, - completion: { - // Force a config sync - if needsSync { - MessageSender.syncConfiguration(forceSyncNow: true).retainUntilComplete() + + // Block the contact + if + let sessionId: String = (thread as? TSContactThread)?.contactSessionID(), + !thread.isBlocked(), + let contact: Contact = Storage.shared.getContact(with: sessionId, using: transaction) + { + contact.isBlocked = true + Storage.shared.setContact(contact, using: transaction) + needsSync = true } } - ) + + // Force a config sync + if needsSync { + MessageSender.syncConfiguration(forceSyncNow: true, with: transaction).retainUntilComplete() + } + } }) alertVC.addAction(UIAlertAction(title: NSLocalizedString("TXT_CANCEL_TITLE", comment: ""), style: .cancel, handler: nil)) self.present(alertVC, animated: true, completion: nil) @@ -378,26 +375,23 @@ class MessageRequestsViewController: BaseVC, UITableViewDelegate, UITableViewDat let alertVC: UIAlertController = UIAlertController(title: NSLocalizedString("MESSAGE_REQUESTS_DELETE_CONFIRMATION_ACTON", comment: ""), message: nil, preferredStyle: .actionSheet) alertVC.addAction(UIAlertAction(title: NSLocalizedString("TXT_DELETE_TITLE", comment: ""), style: .destructive) { _ in - Storage.write( - with: { [weak self] transaction in - Storage.shared.cancelPendingMessageSendJobs(for: uniqueId, using: transaction) - self?.updateContactAndThread(thread: thread, with: transaction) - - // Block the contact - if - let sessionId: String = (thread as? TSContactThread)?.contactSessionID(), - !thread.isBlocked(), - let contact: Contact = Storage.shared.getContact(with: sessionId, using: transaction) - { - contact.isBlocked = true - Storage.shared.setContact(contact, using: transaction) - } - }, - completion: { - // Force a config sync - MessageSender.syncConfiguration(forceSyncNow: true).retainUntilComplete() + Storage.write { [weak self] transaction in + Storage.shared.cancelPendingMessageSendJobs(for: uniqueId, using: transaction) + self?.updateContactAndThread(thread: thread, with: transaction) + + // Block the contact + if + let sessionId: String = (thread as? TSContactThread)?.contactSessionID(), + !thread.isBlocked(), + let contact: Contact = Storage.shared.getContact(with: sessionId, using: transaction) + { + contact.isBlocked = true + Storage.shared.setContact(contact, using: transaction) } - ) + + // Force a config sync + MessageSender.syncConfiguration(forceSyncNow: true, with: transaction).retainUntilComplete() + } }) alertVC.addAction(UIAlertAction(title: NSLocalizedString("TXT_CANCEL_TITLE", comment: ""), style: .cancel, handler: nil)) self.present(alertVC, animated: true, completion: nil)