From 9ac7159a2fb6374ed8557dff5cf832683a7c53c0 Mon Sep 17 00:00:00 2001 From: ryanzhao Date: Mon, 18 May 2020 11:42:16 +1000 Subject: [PATCH 1/5] fix the deadlock issue using the same transaction to write to database in multiple threads --- .../src/Loki/API/LokiFileServerAPI.swift | 14 +++++++++----- .../Multi Device/MultiDeviceProtocol.swift | 2 +- SignalServiceKit/src/Messages/OWSMessageManager.m | 6 ++++-- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/SignalServiceKit/src/Loki/API/LokiFileServerAPI.swift b/SignalServiceKit/src/Loki/API/LokiFileServerAPI.swift index e7b706272..c5cd758df 100644 --- a/SignalServiceKit/src/Loki/API/LokiFileServerAPI.swift +++ b/SignalServiceKit/src/Loki/API/LokiFileServerAPI.swift @@ -24,8 +24,8 @@ public final class LokiFileServerAPI : LokiDotNetAPI { /// Gets the device links associated with the given hex encoded public key from the /// server and stores and returns the valid ones. - public static func getDeviceLinks(associatedWith hexEncodedPublicKey: String) -> Promise> { - return getDeviceLinks(associatedWith: [ hexEncodedPublicKey ]) + public static func getDeviceLinks(associatedWith hexEncodedPublicKey: String, in transaction: YapDatabaseReadWriteTransaction? = nil) -> Promise> { + return getDeviceLinks(associatedWith: [ hexEncodedPublicKey ], in: transaction) } @objc(getDeviceLinksAssociatedWithHexEncodedPublicKeys:) @@ -35,7 +35,7 @@ public final class LokiFileServerAPI : LokiDotNetAPI { /// Gets the device links associated with the given hex encoded public keys from the /// server and stores and returns the valid ones. - public static func getDeviceLinks(associatedWith hexEncodedPublicKeys: Set) -> Promise> { + public static func getDeviceLinks(associatedWith hexEncodedPublicKeys: Set, in transaction: YapDatabaseReadWriteTransaction? = nil) -> Promise> { let hexEncodedPublicKeysDescription = "[ \(hexEncodedPublicKeys.joined(separator: ", ")) ]" print("[Loki] Getting device links for: \(hexEncodedPublicKeysDescription).") return getAuthToken(for: server).then(on: DispatchQueue.global()) { token -> Promise> in @@ -88,8 +88,12 @@ public final class LokiFileServerAPI : LokiDotNetAPI { let (promise, seal) = Promise>.pending() // Dispatch async on the main queue to avoid nested write transactions DispatchQueue.main.async { - storage.dbReadWriteConnection.readWrite { transaction in - storage.setDeviceLinks(deviceLinks, in: transaction) + if let trans = transaction { + storage.setDeviceLinks(deviceLinks, in: trans) + } else { + storage.dbReadWriteConnection.readWrite { transaction in + storage.setDeviceLinks(deviceLinks, in: transaction) + } } // We have to wait for the device links to be stored because a lot of our logic relies // on them being in the database diff --git a/SignalServiceKit/src/Loki/Protocol/Multi Device/MultiDeviceProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Multi Device/MultiDeviceProtocol.swift index 20451221c..6b22f113a 100644 --- a/SignalServiceKit/src/Loki/Protocol/Multi Device/MultiDeviceProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Multi Device/MultiDeviceProtocol.swift @@ -306,7 +306,7 @@ public extension MultiDeviceProtocol { } if timeSinceLastUpdate > deviceLinkUpdateInterval { let masterHexEncodedPublicKey = storage.getMasterHexEncodedPublicKey(for: hexEncodedPublicKey, in: transaction) ?? hexEncodedPublicKey - LokiFileServerAPI.getDeviceLinks(associatedWith: masterHexEncodedPublicKey).done(on: DispatchQueue.global()) { _ in + LokiFileServerAPI.getDeviceLinks(associatedWith: masterHexEncodedPublicKey, in: transaction as! YapDatabaseReadWriteTransaction).done(on: DispatchQueue.global()) { _ in getDestinations() lastDeviceLinkUpdate[hexEncodedPublicKey] = Date() }.catch(on: DispatchQueue.global()) { error in diff --git a/SignalServiceKit/src/Messages/OWSMessageManager.m b/SignalServiceKit/src/Messages/OWSMessageManager.m index 8fb9f0caa..501b1f94f 100644 --- a/SignalServiceKit/src/Messages/OWSMessageManager.m +++ b/SignalServiceKit/src/Messages/OWSMessageManager.m @@ -1266,12 +1266,14 @@ NS_ASSUME_NONNULL_BEGIN // Loki: Update device links in a blocking way // FIXME: This is horrible for performance // FIXME: ======== + // FIX: Using the same transaction for write to avoid deadlock // The envelope source is set during UD decryption if ([ECKeyPair isValidHexEncodedPublicKeyWithCandidate:envelope.source] && dataMessage.publicChatInfo == nil) { // Handled in LokiPublicChatPoller for open group messages dispatch_semaphore_t semaphore = dispatch_semaphore_create(0); - [[LKMultiDeviceProtocol updateDeviceLinksIfNeededForHexEncodedPublicKey:envelope.source in:transaction].ensureOn(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^() { + dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0); + [[LKMultiDeviceProtocol updateDeviceLinksIfNeededForHexEncodedPublicKey:envelope.source in:transaction].ensureOn(queue, ^() { dispatch_semaphore_signal(semaphore); - }).catchOn(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^(NSError *error) { + }).catchOn(queue, ^(NSError *error) { dispatch_semaphore_signal(semaphore); }) retainUntilComplete]; dispatch_semaphore_wait(semaphore, dispatch_time(DISPATCH_TIME_NOW, 10 * NSEC_PER_SEC)); From 6e2189b461b80ae4e9c9a6115b2995a431f284d9 Mon Sep 17 00:00:00 2001 From: ryanzhao Date: Tue, 19 May 2020 13:34:41 +1000 Subject: [PATCH 2/5] fix the issue with cache --- .../src/Loki/API/LokiFileServerAPI.swift | 25 ++++++++++--------- .../Database/OWSPrimaryStorage+Loki.swift | 14 +++++++++++ .../Multi Device/MultiDeviceProtocol.swift | 15 +++++++---- .../src/Messages/OWSMessageManager.m | 5 ++-- .../src/Storage/OWSPrimaryStorage.h | 3 +++ .../src/Storage/OWSPrimaryStorage.m | 2 ++ 6 files changed, 45 insertions(+), 19 deletions(-) diff --git a/SignalServiceKit/src/Loki/API/LokiFileServerAPI.swift b/SignalServiceKit/src/Loki/API/LokiFileServerAPI.swift index c5cd758df..1aed913e2 100644 --- a/SignalServiceKit/src/Loki/API/LokiFileServerAPI.swift +++ b/SignalServiceKit/src/Loki/API/LokiFileServerAPI.swift @@ -24,8 +24,8 @@ public final class LokiFileServerAPI : LokiDotNetAPI { /// Gets the device links associated with the given hex encoded public key from the /// server and stores and returns the valid ones. - public static func getDeviceLinks(associatedWith hexEncodedPublicKey: String, in transaction: YapDatabaseReadWriteTransaction? = nil) -> Promise> { - return getDeviceLinks(associatedWith: [ hexEncodedPublicKey ], in: transaction) + public static func getDeviceLinks(associatedWith hexEncodedPublicKey: String, usingCache: Bool = false) -> Promise> { + return getDeviceLinks(associatedWith: [ hexEncodedPublicKey ], usingCache: usingCache) } @objc(getDeviceLinksAssociatedWithHexEncodedPublicKeys:) @@ -35,7 +35,7 @@ public final class LokiFileServerAPI : LokiDotNetAPI { /// Gets the device links associated with the given hex encoded public keys from the /// server and stores and returns the valid ones. - public static func getDeviceLinks(associatedWith hexEncodedPublicKeys: Set, in transaction: YapDatabaseReadWriteTransaction? = nil) -> Promise> { + public static func getDeviceLinks(associatedWith hexEncodedPublicKeys: Set, usingCache: Bool = false) -> Promise> { let hexEncodedPublicKeysDescription = "[ \(hexEncodedPublicKeys.joined(separator: ", ")) ]" print("[Loki] Getting device links for: \(hexEncodedPublicKeysDescription).") return getAuthToken(for: server).then(on: DispatchQueue.global()) { token -> Promise> in @@ -86,18 +86,19 @@ public final class LokiFileServerAPI : LokiDotNetAPI { }) }.then(on: DispatchQueue.global()) { deviceLinks -> Promise> in let (promise, seal) = Promise>.pending() - // Dispatch async on the main queue to avoid nested write transactions - DispatchQueue.main.async { - if let trans = transaction { - storage.setDeviceLinks(deviceLinks, in: trans) - } else { - storage.dbReadWriteConnection.readWrite { transaction in + if usingCache { + storage.setDeviceLinksInCache(deviceLinks) + seal.fulfill(deviceLinks) + } else { + // Dispatch async on the main queue to avoid nested write transactions + DispatchQueue.main.async { + storage.dbReadWriteConnection.readWrite{ transaction in storage.setDeviceLinks(deviceLinks, in: transaction) } + // We have to wait for the device links to be stored because a lot of our logic relies + // on them being in the database + seal.fulfill(deviceLinks) } - // We have to wait for the device links to be stored because a lot of our logic relies - // on them being in the database - seal.fulfill(deviceLinks) } return promise } diff --git a/SignalServiceKit/src/Loki/Database/OWSPrimaryStorage+Loki.swift b/SignalServiceKit/src/Loki/Database/OWSPrimaryStorage+Loki.swift index 6cc95236b..f63274b5b 100644 --- a/SignalServiceKit/src/Loki/Database/OWSPrimaryStorage+Loki.swift +++ b/SignalServiceKit/src/Loki/Database/OWSPrimaryStorage+Loki.swift @@ -4,6 +4,17 @@ public extension OWSPrimaryStorage { private func getDeviceLinkCollection(for masterHexEncodedPublicKey: String) -> String { return "LokiDeviceLinkCollection-\(masterHexEncodedPublicKey)" } + + public func setDeviceLinksInCache(_ deviceLinks: Set) { + self.deviceLinkCache = deviceLinks + } + + public func syncDeviceLinkCacheToDatabase(in transaction: YapDatabaseReadWriteTransaction) { + if !self.deviceLinkCache.isEmpty { + self.setDeviceLinks(self.deviceLinkCache, in: transaction) + self.deviceLinkCache.removeAll() + } + } public func setDeviceLinks(_ deviceLinks: Set, in transaction: YapDatabaseReadWriteTransaction) { // TODO: Clear collections first? @@ -21,6 +32,9 @@ public extension OWSPrimaryStorage { } public func getDeviceLinks(for masterHexEncodedPublicKey: String, in transaction: YapDatabaseReadTransaction) -> Set { + if !self.deviceLinkCache.isEmpty { + return self.deviceLinkCache + } let collection = getDeviceLinkCollection(for: masterHexEncodedPublicKey) guard !transaction.allKeys(inCollection: collection).isEmpty else { return [] } // Fixes a crash that used to occur on Josh's device var result: Set = [] diff --git a/SignalServiceKit/src/Loki/Protocol/Multi Device/MultiDeviceProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Multi Device/MultiDeviceProtocol.swift index 6b22f113a..39234c2b0 100644 --- a/SignalServiceKit/src/Loki/Protocol/Multi Device/MultiDeviceProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Multi Device/MultiDeviceProtocol.swift @@ -144,11 +144,16 @@ public final class MultiDeviceProtocol : NSObject { } } - @objc(updateDeviceLinksIfNeededForHexEncodedPublicKey:in:) - public static func updateDeviceLinksIfNeeded(for hexEncodedPublicKey: String, in transaction: YapDatabaseReadTransaction) -> AnyPromise { - let promise = getMultiDeviceDestinations(for: hexEncodedPublicKey, in: transaction) + @objc(updateDeviceLinksIfNeededForHexEncodedPublicKey:in:usingCache:) + public static func updateDeviceLinksIfNeeded(for hexEncodedPublicKey: String, in transaction: YapDatabaseReadTransaction, usingCache: Bool = false) -> AnyPromise { + let promise = getMultiDeviceDestinations(for: hexEncodedPublicKey, in: transaction, usingCache: usingCache) return AnyPromise.from(promise) } + + @objc(syncDeviceLinkCacheToDatabaseInTransaction:) + public static func syncDeviceLinkCacheToDatabase(in transaction: YapDatabaseReadWriteTransaction) { + storage.syncDeviceLinkCacheToDatabase(in: transaction) + } /// See [Auto-Generated Friend Requests](https://github.com/loki-project/session-protocol-docs/wiki/Auto-Generated-Friend-Requests) for more information. @objc(getAutoGeneratedMultiDeviceFRMessageForHexEncodedPublicKey:in:) @@ -284,7 +289,7 @@ public final class MultiDeviceProtocol : NSObject { // Here (in a non-@objc extension) because it doesn't interoperate well with Obj-C public extension MultiDeviceProtocol { - fileprivate static func getMultiDeviceDestinations(for hexEncodedPublicKey: String, in transaction: YapDatabaseReadTransaction) -> Promise> { + fileprivate static func getMultiDeviceDestinations(for hexEncodedPublicKey: String, in transaction: YapDatabaseReadTransaction, usingCache: Bool = false) -> Promise> { let (promise, seal) = Promise>.pending() func getDestinations(in transaction: YapDatabaseReadTransaction? = nil) { storage.dbReadConnection.read { transaction in @@ -306,7 +311,7 @@ public extension MultiDeviceProtocol { } if timeSinceLastUpdate > deviceLinkUpdateInterval { let masterHexEncodedPublicKey = storage.getMasterHexEncodedPublicKey(for: hexEncodedPublicKey, in: transaction) ?? hexEncodedPublicKey - LokiFileServerAPI.getDeviceLinks(associatedWith: masterHexEncodedPublicKey, in: transaction as! YapDatabaseReadWriteTransaction).done(on: DispatchQueue.global()) { _ in + LokiFileServerAPI.getDeviceLinks(associatedWith: masterHexEncodedPublicKey, usingCache: usingCache).done(on: DispatchQueue.global()) { _ in getDestinations() lastDeviceLinkUpdate[hexEncodedPublicKey] = Date() }.catch(on: DispatchQueue.global()) { error in diff --git a/SignalServiceKit/src/Messages/OWSMessageManager.m b/SignalServiceKit/src/Messages/OWSMessageManager.m index 501b1f94f..4302b62df 100644 --- a/SignalServiceKit/src/Messages/OWSMessageManager.m +++ b/SignalServiceKit/src/Messages/OWSMessageManager.m @@ -1266,17 +1266,18 @@ NS_ASSUME_NONNULL_BEGIN // Loki: Update device links in a blocking way // FIXME: This is horrible for performance // FIXME: ======== - // FIX: Using the same transaction for write to avoid deadlock + // FIX: Using the cache for write to avoid deadlock // The envelope source is set during UD decryption if ([ECKeyPair isValidHexEncodedPublicKeyWithCandidate:envelope.source] && dataMessage.publicChatInfo == nil) { // Handled in LokiPublicChatPoller for open group messages dispatch_semaphore_t semaphore = dispatch_semaphore_create(0); dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0); - [[LKMultiDeviceProtocol updateDeviceLinksIfNeededForHexEncodedPublicKey:envelope.source in:transaction].ensureOn(queue, ^() { + [[LKMultiDeviceProtocol updateDeviceLinksIfNeededForHexEncodedPublicKey:envelope.source in:transaction usingCache: true].ensureOn(queue, ^() { dispatch_semaphore_signal(semaphore); }).catchOn(queue, ^(NSError *error) { dispatch_semaphore_signal(semaphore); }) retainUntilComplete]; dispatch_semaphore_wait(semaphore, dispatch_time(DISPATCH_TIME_NOW, 10 * NSEC_PER_SEC)); + [LKMultiDeviceProtocol syncDeviceLinkCacheToDatabaseInTransaction:transaction]; } // FIXME: ======== diff --git a/SignalServiceKit/src/Storage/OWSPrimaryStorage.h b/SignalServiceKit/src/Storage/OWSPrimaryStorage.h index dd1cf65b3..96d2bc6bf 100644 --- a/SignalServiceKit/src/Storage/OWSPrimaryStorage.h +++ b/SignalServiceKit/src/Storage/OWSPrimaryStorage.h @@ -12,6 +12,8 @@ extern NSString *const OWSUIDatabaseConnectionWillUpdateExternallyNotification; extern NSString *const OWSUIDatabaseConnectionDidUpdateExternallyNotification; extern NSString *const OWSUIDatabaseConnectionNotificationsKey; +@class LKDeviceLink; + @interface OWSPrimaryStorage : OWSStorage - (instancetype)init NS_UNAVAILABLE; @@ -23,6 +25,7 @@ extern NSString *const OWSUIDatabaseConnectionNotificationsKey; @property (nonatomic, readonly) YapDatabaseConnection *uiDatabaseConnection; @property (nonatomic, readonly) YapDatabaseConnection *dbReadConnection; @property (nonatomic, readonly) YapDatabaseConnection *dbReadWriteConnection; +@property (nonatomic) NSSet *deviceLinkCache; - (void)updateUIDatabaseConnectionToLatest; diff --git a/SignalServiceKit/src/Storage/OWSPrimaryStorage.m b/SignalServiceKit/src/Storage/OWSPrimaryStorage.m index f05fcc8b9..a72507286 100644 --- a/SignalServiceKit/src/Storage/OWSPrimaryStorage.m +++ b/SignalServiceKit/src/Storage/OWSPrimaryStorage.m @@ -75,6 +75,8 @@ void VerifyRegistrationsForPrimaryStorage(OWSStorage *storage) if (self) { [self loadDatabase]; + + _deviceLinkCache = [NSSet set]; _dbReadPool = [[YapDatabaseConnectionPool alloc] initWithDatabase:self.database]; _dbReadWriteConnection = [self newDatabaseConnection]; From d6c24cebb820a60d766177fb8c9bc27f321e086d Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Tue, 19 May 2020 15:28:11 +1000 Subject: [PATCH 3/5] Cache device links in memory rather than on disk --- .../src/Loki/API/LokiFileServerAPI.swift | 30 ++++++++----------- .../Database/OWSPrimaryStorage+Loki.swift | 30 +++++++++++-------- .../Multi Device/MultiDeviceProtocol.swift | 17 ++++------- .../src/Messages/OWSMessageManager.m | 4 +-- .../src/Storage/OWSPrimaryStorage.h | 1 - .../src/Storage/OWSPrimaryStorage.m | 2 -- 6 files changed, 36 insertions(+), 48 deletions(-) diff --git a/SignalServiceKit/src/Loki/API/LokiFileServerAPI.swift b/SignalServiceKit/src/Loki/API/LokiFileServerAPI.swift index 1aed913e2..dea78f83e 100644 --- a/SignalServiceKit/src/Loki/API/LokiFileServerAPI.swift +++ b/SignalServiceKit/src/Loki/API/LokiFileServerAPI.swift @@ -24,8 +24,8 @@ public final class LokiFileServerAPI : LokiDotNetAPI { /// Gets the device links associated with the given hex encoded public key from the /// server and stores and returns the valid ones. - public static func getDeviceLinks(associatedWith hexEncodedPublicKey: String, usingCache: Bool = false) -> Promise> { - return getDeviceLinks(associatedWith: [ hexEncodedPublicKey ], usingCache: usingCache) + public static func getDeviceLinks(associatedWith hexEncodedPublicKey: String) -> Promise> { + return getDeviceLinks(associatedWith: [ hexEncodedPublicKey ]) } @objc(getDeviceLinksAssociatedWithHexEncodedPublicKeys:) @@ -35,7 +35,7 @@ public final class LokiFileServerAPI : LokiDotNetAPI { /// Gets the device links associated with the given hex encoded public keys from the /// server and stores and returns the valid ones. - public static func getDeviceLinks(associatedWith hexEncodedPublicKeys: Set, usingCache: Bool = false) -> Promise> { + public static func getDeviceLinks(associatedWith hexEncodedPublicKeys: Set) -> Promise> { let hexEncodedPublicKeysDescription = "[ \(hexEncodedPublicKeys.joined(separator: ", ")) ]" print("[Loki] Getting device links for: \(hexEncodedPublicKeysDescription).") return getAuthToken(for: server).then(on: DispatchQueue.global()) { token -> Promise> in @@ -84,23 +84,17 @@ public final class LokiFileServerAPI : LokiDotNetAPI { return deviceLink } }) - }.then(on: DispatchQueue.global()) { deviceLinks -> Promise> in - let (promise, seal) = Promise>.pending() - if usingCache { - storage.setDeviceLinksInCache(deviceLinks) - seal.fulfill(deviceLinks) - } else { - // Dispatch async on the main queue to avoid nested write transactions - DispatchQueue.main.async { - storage.dbReadWriteConnection.readWrite{ transaction in - storage.setDeviceLinks(deviceLinks, in: transaction) - } - // We have to wait for the device links to be stored because a lot of our logic relies - // on them being in the database - seal.fulfill(deviceLinks) + }.map(on: DispatchQueue.global()) { deviceLinks in + storage.cacheDeviceLinks(deviceLinks) + /* + // Dispatch async on the main queue to avoid nested write transactions + DispatchQueue.main.async { + storage.dbReadWriteConnection.readWrite { transaction in + storage.setDeviceLinks(deviceLinks, in: transaction) } } - return promise + */ + return deviceLinks } } } diff --git a/SignalServiceKit/src/Loki/Database/OWSPrimaryStorage+Loki.swift b/SignalServiceKit/src/Loki/Database/OWSPrimaryStorage+Loki.swift index f63274b5b..f7a937360 100644 --- a/SignalServiceKit/src/Loki/Database/OWSPrimaryStorage+Loki.swift +++ b/SignalServiceKit/src/Loki/Database/OWSPrimaryStorage+Loki.swift @@ -1,19 +1,14 @@ public extension OWSPrimaryStorage { - + + private static var deviceLinkCache: Set = [] + private func getDeviceLinkCollection(for masterHexEncodedPublicKey: String) -> String { return "LokiDeviceLinkCollection-\(masterHexEncodedPublicKey)" } - public func setDeviceLinksInCache(_ deviceLinks: Set) { - self.deviceLinkCache = deviceLinks - } - - public func syncDeviceLinkCacheToDatabase(in transaction: YapDatabaseReadWriteTransaction) { - if !self.deviceLinkCache.isEmpty { - self.setDeviceLinks(self.deviceLinkCache, in: transaction) - self.deviceLinkCache.removeAll() - } + public func cacheDeviceLinks(_ deviceLinks: Set) { + OWSPrimaryStorage.deviceLinkCache.formUnion(deviceLinks) } public func setDeviceLinks(_ deviceLinks: Set, in transaction: YapDatabaseReadWriteTransaction) { @@ -22,19 +17,24 @@ public extension OWSPrimaryStorage { } public func addDeviceLink(_ deviceLink: DeviceLink, in transaction: YapDatabaseReadWriteTransaction) { + OWSPrimaryStorage.deviceLinkCache.insert(deviceLink) + /* let collection = getDeviceLinkCollection(for: deviceLink.master.hexEncodedPublicKey) transaction.setObject(deviceLink, forKey: deviceLink.slave.hexEncodedPublicKey, inCollection: collection) + */ } public func removeDeviceLink(_ deviceLink: DeviceLink, in transaction: YapDatabaseReadWriteTransaction) { + OWSPrimaryStorage.deviceLinkCache.remove(deviceLink) + /* let collection = getDeviceLinkCollection(for: deviceLink.master.hexEncodedPublicKey) transaction.removeObject(forKey: deviceLink.slave.hexEncodedPublicKey, inCollection: collection) + */ } public func getDeviceLinks(for masterHexEncodedPublicKey: String, in transaction: YapDatabaseReadTransaction) -> Set { - if !self.deviceLinkCache.isEmpty { - return self.deviceLinkCache - } + return OWSPrimaryStorage.deviceLinkCache.filter { $0.master.hexEncodedPublicKey == masterHexEncodedPublicKey } + /* let collection = getDeviceLinkCollection(for: masterHexEncodedPublicKey) guard !transaction.allKeys(inCollection: collection).isEmpty else { return [] } // Fixes a crash that used to occur on Josh's device var result: Set = [] @@ -43,9 +43,12 @@ public extension OWSPrimaryStorage { result.insert(deviceLink) } return result + */ } public func getDeviceLink(for slaveHexEncodedPublicKey: String, in transaction: YapDatabaseReadTransaction) -> DeviceLink? { + return OWSPrimaryStorage.deviceLinkCache.filter { $0.slave.hexEncodedPublicKey == slaveHexEncodedPublicKey }.first + /* let query = YapDatabaseQuery(string: "WHERE \(DeviceLinkIndex.slaveHexEncodedPublicKey) = ?", parameters: [ slaveHexEncodedPublicKey ]) let deviceLinks = DeviceLinkIndex.getDeviceLinks(for: query, in: transaction) guard deviceLinks.count <= 1 else { @@ -53,6 +56,7 @@ public extension OWSPrimaryStorage { return nil } return deviceLinks.first + */ } public func getMasterHexEncodedPublicKey(for slaveHexEncodedPublicKey: String, in transaction: YapDatabaseReadTransaction) -> String? { diff --git a/SignalServiceKit/src/Loki/Protocol/Multi Device/MultiDeviceProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Multi Device/MultiDeviceProtocol.swift index 39234c2b0..b1e740029 100644 --- a/SignalServiceKit/src/Loki/Protocol/Multi Device/MultiDeviceProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Multi Device/MultiDeviceProtocol.swift @@ -24,7 +24,7 @@ public final class MultiDeviceProtocol : NSObject { internal static var storage: OWSPrimaryStorage { OWSPrimaryStorage.shared() } // MARK: - Settings - public static let deviceLinkUpdateInterval: TimeInterval = 20 + public static let deviceLinkUpdateInterval: TimeInterval = 60 // MARK: - Multi Device Destination public struct MultiDeviceDestination : Hashable { @@ -144,16 +144,11 @@ public final class MultiDeviceProtocol : NSObject { } } - @objc(updateDeviceLinksIfNeededForHexEncodedPublicKey:in:usingCache:) - public static func updateDeviceLinksIfNeeded(for hexEncodedPublicKey: String, in transaction: YapDatabaseReadTransaction, usingCache: Bool = false) -> AnyPromise { - let promise = getMultiDeviceDestinations(for: hexEncodedPublicKey, in: transaction, usingCache: usingCache) + @objc(updateDeviceLinksIfNeededForHexEncodedPublicKey:in:) + public static func updateDeviceLinksIfNeeded(for hexEncodedPublicKey: String, in transaction: YapDatabaseReadTransaction) -> AnyPromise { + let promise = getMultiDeviceDestinations(for: hexEncodedPublicKey, in: transaction) return AnyPromise.from(promise) } - - @objc(syncDeviceLinkCacheToDatabaseInTransaction:) - public static func syncDeviceLinkCacheToDatabase(in transaction: YapDatabaseReadWriteTransaction) { - storage.syncDeviceLinkCacheToDatabase(in: transaction) - } /// See [Auto-Generated Friend Requests](https://github.com/loki-project/session-protocol-docs/wiki/Auto-Generated-Friend-Requests) for more information. @objc(getAutoGeneratedMultiDeviceFRMessageForHexEncodedPublicKey:in:) @@ -289,7 +284,7 @@ public final class MultiDeviceProtocol : NSObject { // Here (in a non-@objc extension) because it doesn't interoperate well with Obj-C public extension MultiDeviceProtocol { - fileprivate static func getMultiDeviceDestinations(for hexEncodedPublicKey: String, in transaction: YapDatabaseReadTransaction, usingCache: Bool = false) -> Promise> { + fileprivate static func getMultiDeviceDestinations(for hexEncodedPublicKey: String, in transaction: YapDatabaseReadTransaction) -> Promise> { let (promise, seal) = Promise>.pending() func getDestinations(in transaction: YapDatabaseReadTransaction? = nil) { storage.dbReadConnection.read { transaction in @@ -311,7 +306,7 @@ public extension MultiDeviceProtocol { } if timeSinceLastUpdate > deviceLinkUpdateInterval { let masterHexEncodedPublicKey = storage.getMasterHexEncodedPublicKey(for: hexEncodedPublicKey, in: transaction) ?? hexEncodedPublicKey - LokiFileServerAPI.getDeviceLinks(associatedWith: masterHexEncodedPublicKey, usingCache: usingCache).done(on: DispatchQueue.global()) { _ in + LokiFileServerAPI.getDeviceLinks(associatedWith: masterHexEncodedPublicKey).done(on: DispatchQueue.global()) { _ in getDestinations() lastDeviceLinkUpdate[hexEncodedPublicKey] = Date() }.catch(on: DispatchQueue.global()) { error in diff --git a/SignalServiceKit/src/Messages/OWSMessageManager.m b/SignalServiceKit/src/Messages/OWSMessageManager.m index 4302b62df..43cc7420f 100644 --- a/SignalServiceKit/src/Messages/OWSMessageManager.m +++ b/SignalServiceKit/src/Messages/OWSMessageManager.m @@ -1266,18 +1266,16 @@ NS_ASSUME_NONNULL_BEGIN // Loki: Update device links in a blocking way // FIXME: This is horrible for performance // FIXME: ======== - // FIX: Using the cache for write to avoid deadlock // The envelope source is set during UD decryption if ([ECKeyPair isValidHexEncodedPublicKeyWithCandidate:envelope.source] && dataMessage.publicChatInfo == nil) { // Handled in LokiPublicChatPoller for open group messages dispatch_semaphore_t semaphore = dispatch_semaphore_create(0); dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0); - [[LKMultiDeviceProtocol updateDeviceLinksIfNeededForHexEncodedPublicKey:envelope.source in:transaction usingCache: true].ensureOn(queue, ^() { + [[LKMultiDeviceProtocol updateDeviceLinksIfNeededForHexEncodedPublicKey:envelope.source in:transaction].ensureOn(queue, ^() { dispatch_semaphore_signal(semaphore); }).catchOn(queue, ^(NSError *error) { dispatch_semaphore_signal(semaphore); }) retainUntilComplete]; dispatch_semaphore_wait(semaphore, dispatch_time(DISPATCH_TIME_NOW, 10 * NSEC_PER_SEC)); - [LKMultiDeviceProtocol syncDeviceLinkCacheToDatabaseInTransaction:transaction]; } // FIXME: ======== diff --git a/SignalServiceKit/src/Storage/OWSPrimaryStorage.h b/SignalServiceKit/src/Storage/OWSPrimaryStorage.h index 96d2bc6bf..ea252760a 100644 --- a/SignalServiceKit/src/Storage/OWSPrimaryStorage.h +++ b/SignalServiceKit/src/Storage/OWSPrimaryStorage.h @@ -25,7 +25,6 @@ extern NSString *const OWSUIDatabaseConnectionNotificationsKey; @property (nonatomic, readonly) YapDatabaseConnection *uiDatabaseConnection; @property (nonatomic, readonly) YapDatabaseConnection *dbReadConnection; @property (nonatomic, readonly) YapDatabaseConnection *dbReadWriteConnection; -@property (nonatomic) NSSet *deviceLinkCache; - (void)updateUIDatabaseConnectionToLatest; diff --git a/SignalServiceKit/src/Storage/OWSPrimaryStorage.m b/SignalServiceKit/src/Storage/OWSPrimaryStorage.m index a72507286..06b1ec8cc 100644 --- a/SignalServiceKit/src/Storage/OWSPrimaryStorage.m +++ b/SignalServiceKit/src/Storage/OWSPrimaryStorage.m @@ -76,8 +76,6 @@ void VerifyRegistrationsForPrimaryStorage(OWSStorage *storage) if (self) { [self loadDatabase]; - _deviceLinkCache = [NSSet set]; - _dbReadPool = [[YapDatabaseConnectionPool alloc] initWithDatabase:self.database]; _dbReadWriteConnection = [self newDatabaseConnection]; _uiDatabaseConnection = [self newDatabaseConnection]; From 2cf278f15034a4454b92270408ff580ca07875ad Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Tue, 19 May 2020 15:31:59 +1000 Subject: [PATCH 4/5] Clean --- SignalServiceKit/src/Storage/OWSPrimaryStorage.h | 2 -- SignalServiceKit/src/Storage/OWSPrimaryStorage.m | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/SignalServiceKit/src/Storage/OWSPrimaryStorage.h b/SignalServiceKit/src/Storage/OWSPrimaryStorage.h index ea252760a..dd1cf65b3 100644 --- a/SignalServiceKit/src/Storage/OWSPrimaryStorage.h +++ b/SignalServiceKit/src/Storage/OWSPrimaryStorage.h @@ -12,8 +12,6 @@ extern NSString *const OWSUIDatabaseConnectionWillUpdateExternallyNotification; extern NSString *const OWSUIDatabaseConnectionDidUpdateExternallyNotification; extern NSString *const OWSUIDatabaseConnectionNotificationsKey; -@class LKDeviceLink; - @interface OWSPrimaryStorage : OWSStorage - (instancetype)init NS_UNAVAILABLE; diff --git a/SignalServiceKit/src/Storage/OWSPrimaryStorage.m b/SignalServiceKit/src/Storage/OWSPrimaryStorage.m index 06b1ec8cc..f05fcc8b9 100644 --- a/SignalServiceKit/src/Storage/OWSPrimaryStorage.m +++ b/SignalServiceKit/src/Storage/OWSPrimaryStorage.m @@ -75,7 +75,7 @@ void VerifyRegistrationsForPrimaryStorage(OWSStorage *storage) if (self) { [self loadDatabase]; - + _dbReadPool = [[YapDatabaseConnectionPool alloc] initWithDatabase:self.database]; _dbReadWriteConnection = [self newDatabaseConnection]; _uiDatabaseConnection = [self newDatabaseConnection]; From 19a214f65397470905230bff21c224dbaf9785a4 Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Wed, 20 May 2020 11:12:51 +1000 Subject: [PATCH 5/5] Fix simultaneous session request bug --- .../xcshareddata/xcschemes/Signal.xcscheme | 3 +++ .../Loki/Database/OWSPrimaryStorage+Loki.swift | 15 ++++++++++++++- .../Closed Groups/ClosedGroupsProtocol.swift | 3 ++- .../Loki/Protocol/Meta/SessionMetaProtocol.swift | 1 - .../SessionManagementProtocol.swift | 6 ++++++ 5 files changed, 25 insertions(+), 3 deletions(-) diff --git a/Signal.xcodeproj/xcshareddata/xcschemes/Signal.xcscheme b/Signal.xcodeproj/xcshareddata/xcschemes/Signal.xcscheme index 915c5088b..64a8ba432 100644 --- a/Signal.xcodeproj/xcshareddata/xcschemes/Signal.xcscheme +++ b/Signal.xcodeproj/xcshareddata/xcschemes/Signal.xcscheme @@ -126,6 +126,9 @@ + + diff --git a/SignalServiceKit/src/Loki/Database/OWSPrimaryStorage+Loki.swift b/SignalServiceKit/src/Loki/Database/OWSPrimaryStorage+Loki.swift index f7a937360..1336fcff3 100644 --- a/SignalServiceKit/src/Loki/Database/OWSPrimaryStorage+Loki.swift +++ b/SignalServiceKit/src/Loki/Database/OWSPrimaryStorage+Loki.swift @@ -1,6 +1,18 @@ public extension OWSPrimaryStorage { + // MARK: Session Requests + private static let sessionRequestTimestampCollection = "LokiSessionRequestTimestampCollection" + + public func setSessionRequestTimestamp(for publicKey: String, to timestamp: Date, in transaction: YapDatabaseReadWriteTransaction) { + transaction.setDate(timestamp, forKey: publicKey, inCollection: OWSPrimaryStorage.sessionRequestTimestampCollection) + } + + public func getSessionRequestTimestamp(for publicKey: String, in transaction: YapDatabaseReadTransaction) -> Date? { + transaction.date(forKey: publicKey, inCollection: OWSPrimaryStorage.sessionRequestTimestampCollection) + } + + // MARK: Multi Device private static var deviceLinkCache: Set = [] private func getDeviceLinkCollection(for masterHexEncodedPublicKey: String) -> String { @@ -62,7 +74,8 @@ public extension OWSPrimaryStorage { public func getMasterHexEncodedPublicKey(for slaveHexEncodedPublicKey: String, in transaction: YapDatabaseReadTransaction) -> String? { return getDeviceLink(for: slaveHexEncodedPublicKey, in: transaction)?.master.hexEncodedPublicKey } - + + // MARK: Open Groups public func getUserCount(for publicChat: LokiPublicChat, in transaction: YapDatabaseReadTransaction) -> Int? { return transaction.object(forKey: publicChat.id, inCollection: "LokiPublicChatUserCountCollection") as? Int } diff --git a/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift index 62b915812..ec5a18a7f 100644 --- a/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift @@ -41,9 +41,10 @@ public final class ClosedGroupsProtocol : NSObject { let thread = TSContactThread.getOrCreateThread(withContactId: hexEncodedPublicKey, transaction: transaction) thread.save(with: transaction) let sessionRequestMessage = SessionRequestMessage(thread: thread) + storage.setSessionRequestTimestamp(for: hexEncodedPublicKey, to: Date(), in: transaction) let messageSenderJobQueue = SSKEnvironment.shared.messageSenderJobQueue // This has to happen sync to ensure that session requests get sent before AFRs do (it's - // asssumed that the master device first syncs closed groups first and contacts after that). + // asssumed that the master device syncs closed groups first and contacts after that). messageSenderJobQueue.add(message: sessionRequestMessage, transaction: transaction) } } diff --git a/SignalServiceKit/src/Loki/Protocol/Meta/SessionMetaProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Meta/SessionMetaProtocol.swift index fd374342e..92e880947 100644 --- a/SignalServiceKit/src/Loki/Protocol/Meta/SessionMetaProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Meta/SessionMetaProtocol.swift @@ -55,7 +55,6 @@ public final class SessionMetaProtocol : NSObject { } // MARK: Note to Self - @objc(isThreadNoteToSelf:) public static func isThreadNoteToSelf(_ thread: TSThread) -> Bool { guard let thread = thread as? TSContactThread else { return false } diff --git a/SignalServiceKit/src/Loki/Protocol/Session Management/SessionManagementProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Session Management/SessionManagementProtocol.swift index 82a4a0318..d34af79aa 100644 --- a/SignalServiceKit/src/Loki/Protocol/Session Management/SessionManagementProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Session Management/SessionManagementProtocol.swift @@ -145,6 +145,7 @@ public final class SessionManagementProtocol : NSObject { storage.dbReadWriteConnection.readWrite { transaction in let thread = TSContactThread.getOrCreateThread(withContactId: hexEncodedPublicKey, transaction: transaction) let sessionRequestMessage = SessionRequestMessage(thread: thread) + storage.setSessionRequestTimestamp(for: hexEncodedPublicKey, to: Date(), in: transaction) let messageSenderJobQueue = SSKEnvironment.shared.messageSenderJobQueue messageSenderJobQueue.add(message: sessionRequestMessage, transaction: transaction) } @@ -192,6 +193,11 @@ public final class SessionManagementProtocol : NSObject { public static func handleSessionRequestMessage(_ dataMessage: SSKProtoDataMessage, wrappedIn envelope: SSKProtoEnvelope, using transaction: YapDatabaseReadWriteTransaction) { // The envelope source is set during UD decryption let hexEncodedPublicKey = envelope.source! + if let sentSessionRequestTimestamp = storage.getSessionRequestTimestamp(for: hexEncodedPublicKey, in: transaction), + envelope.timestamp < NSDate.ows_millisecondsSince1970(for: sentSessionRequestTimestamp) { + // We sent a session request after this one was sent + return + } var closedGroupMembers: Set = [] TSGroupThread.enumerateCollectionObjects(with: transaction) { object, _ in guard let group = object as? TSGroupThread, group.groupModel.groupType == .closedGroup,