From 81c9dc891958521d1ba3364ea20a38858aff8d6f Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Tue, 6 Oct 2020 12:24:06 +1100 Subject: [PATCH 1/2] Fix SSK group leaving race condition --- .../Closed Groups/ClosedGroupsProtocol.swift | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift index b959d17c1..b2aa36302 100644 --- a/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift @@ -118,11 +118,18 @@ public final class ClosedGroupsProtocol : NSObject { print("[Loki] Can't remove self and others simultaneously.") return Promise(error: Error.invalidUpdate) } - // Send the update to the group (don't include new ratchets as everyone should regenerate new ratchets individually) - let closedGroupUpdateMessageKind = ClosedGroupUpdateMessage.Kind.info(groupPublicKey: Data(hex: groupPublicKey), name: name, senderKeys: [], - members: membersAsData, admins: adminsAsData) - let closedGroupUpdateMessage = ClosedGroupUpdateMessage(thread: thread, kind: closedGroupUpdateMessageKind) - SSKEnvironment.shared.messageSender.send(closedGroupUpdateMessage, success: { seal.fulfill(()) }, failure: { seal.reject($0) }) + // Establish sessions if needed + establishSessionsIfNeeded(with: [String](members), using: transaction) + // Send the update to the existing members using established channels (don't include new ratchets as everyone should regenerate new ratchets individually) + let promises: [Promise] = oldMembers.map { member in + let thread = TSContactThread.getOrCreateThread(withContactId: member, transaction: transaction) + thread.save(with: transaction) + let closedGroupUpdateMessageKind = ClosedGroupUpdateMessage.Kind.info(groupPublicKey: Data(hex: groupPublicKey), name: name, senderKeys: [], + members: membersAsData, admins: adminsAsData) + let closedGroupUpdateMessage = ClosedGroupUpdateMessage(thread: thread, kind: closedGroupUpdateMessageKind) + return SSKEnvironment.shared.messageSender.sendPromise(message: closedGroupUpdateMessage) + } + when(resolved: promises).done2 { _ in seal.fulfill(()) }.catch2 { seal.reject($0) } promise.done { try! Storage.writeSync { transaction in // Delete all ratchets (it's important that this happens * after * sending out the update) @@ -134,8 +141,6 @@ public final class ClosedGroupsProtocol : NSObject { // Notify the PN server LokiPushNotificationManager.performOperation(.unsubscribe, for: groupPublicKey, publicKey: userPublicKey) } else { - // Establish sessions if needed - establishSessionsIfNeeded(with: [String](members), using: transaction) // Send closed group update messages to any new members using established channels for member in newMembers { let thread = TSContactThread.getOrCreateThread(withContactId: member, transaction: transaction) @@ -203,13 +208,13 @@ public final class ClosedGroupsProtocol : NSObject { return promise } - /// The returned promise is fulfilled when the message has been sent **to the group**. It doesn't wait for the user's new ratchet to be distributed. + /// The returned promise is fulfilled when the group update message has been sent. It doesn't wait for the user's new ratchet to be distributed. @objc(leaveGroupWithPublicKey:transaction:) public static func objc_leave(_ groupPublicKey: String, using transaction: YapDatabaseReadWriteTransaction) -> AnyPromise { return AnyPromise.from(leave(groupPublicKey, using: transaction)) } - /// The returned promise is fulfilled when the message has been sent **to the group**. It doesn't wait for the user's new ratchet to be distributed. + /// The returned promise is fulfilled when the group update message has been sent. It doesn't wait for the user's new ratchet to be distributed. public static func leave(_ groupPublicKey: String, using transaction: YapDatabaseReadWriteTransaction) -> Promise { let userPublicKey = UserDefaults.standard[.masterHexEncodedPublicKey] ?? getUserHexEncodedPublicKey() let groupID = LKGroupUtilities.getEncodedClosedGroupIDAsData(groupPublicKey) From 3c0e5145657f9456837d62f03e1771da0e9d5765 Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Tue, 6 Oct 2020 14:24:09 +1100 Subject: [PATCH 2/2] Show a loader while the group is updating --- .../View Controllers/EditClosedGroupVC.swift | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/Signal/src/Loki/View Controllers/EditClosedGroupVC.swift b/Signal/src/Loki/View Controllers/EditClosedGroupVC.swift index 02548f496..075a146e4 100644 --- a/Signal/src/Loki/View Controllers/EditClosedGroupVC.swift +++ b/Signal/src/Loki/View Controllers/EditClosedGroupVC.swift @@ -241,13 +241,17 @@ final class EditClosedGroupVC : BaseVC, UITableViewDataSource, UITableViewDelega guard members != Set(thread.groupModel.groupMemberIds) || name != thread.groupModel.groupName else { return popToConversationVC(self) } - try! Storage.writeSync { [weak self] transaction in - ClosedGroupsProtocol.update(groupPublicKey, with: members, name: name, transaction: transaction).done(on: DispatchQueue.main) { - guard let self = self else { return } - popToConversationVC(self) - }.catch(on: DispatchQueue.main) { error in - guard let self = self else { return } - self.showError(title: "Couldn't Update Group", message: "Please check your internet connection and try again.") + ModalActivityIndicatorViewController.present(fromViewController: navigationController!, canCancel: false) { [weak self] _ in + try! Storage.writeSync { [weak self] transaction in + ClosedGroupsProtocol.update(groupPublicKey, with: members, name: name, transaction: transaction).done(on: DispatchQueue.main) { + guard let self = self else { return } + self.dismiss(animated: true, completion: nil) // Dismiss the loader + popToConversationVC(self) + }.catch(on: DispatchQueue.main) { error in + guard let self = self else { return } + self.dismiss(animated: true, completion: nil) // Dismiss the loader + self.showError(title: "Couldn't Update Group", message: "Please check your internet connection and try again.") + } } } }