From 3ff9fdabdbb8d514d1e1f66f5ec306c329bc865a Mon Sep 17 00:00:00 2001 From: Shadowfacts Date: Sat, 17 Oct 2020 13:10:10 -0400 Subject: [PATCH] Use MultiThreadedDictionary for ImageCache request groups Prevents a crash due a race condition if multiple requets complete simultaneously and attempt to modify the dictionary --- Tusker.xcodeproj/project.pbxproj | 8 ++-- Tusker/CachedDictionary.swift | 35 --------------- Tusker/Caching/ImageCache.swift | 4 +- Tusker/MultiThreadDictionary.swift | 50 ++++++++++++++++++++++ Tusker/Views/AccountDisplayNameLabel.swift | 2 +- Tusker/Views/ContentTextView.swift | 2 +- Tusker/Views/EmojiLabel.swift | 2 +- 7 files changed, 59 insertions(+), 44 deletions(-) delete mode 100644 Tusker/CachedDictionary.swift create mode 100644 Tusker/MultiThreadDictionary.swift diff --git a/Tusker.xcodeproj/project.pbxproj b/Tusker.xcodeproj/project.pbxproj index d2fb8065..31d6ec6f 100644 --- a/Tusker.xcodeproj/project.pbxproj +++ b/Tusker.xcodeproj/project.pbxproj @@ -140,7 +140,7 @@ D64BC19223C271D9000D0238 /* MastodonActivity.swift in Sources */ = {isa = PBXBuildFile; fileRef = D64BC19123C271D9000D0238 /* MastodonActivity.swift */; }; D64D0AAD2128D88B005A6F37 /* LocalData.swift in Sources */ = {isa = PBXBuildFile; fileRef = D64D0AAC2128D88B005A6F37 /* LocalData.swift */; }; D64D0AB12128D9AE005A6F37 /* OnboardingViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = D64D0AB02128D9AE005A6F37 /* OnboardingViewController.swift */; }; - D64D8CA92463B494006B0BAA /* CachedDictionary.swift in Sources */ = {isa = PBXBuildFile; fileRef = D64D8CA82463B494006B0BAA /* CachedDictionary.swift */; }; + D64D8CA92463B494006B0BAA /* MultiThreadDictionary.swift in Sources */ = {isa = PBXBuildFile; fileRef = D64D8CA82463B494006B0BAA /* MultiThreadDictionary.swift */; }; D64F80E2215875CC00BEF393 /* XCBActionType.swift in Sources */ = {isa = PBXBuildFile; fileRef = D64F80E1215875CC00BEF393 /* XCBActionType.swift */; }; D6531DEE246B81C9000F9538 /* GifvAttachmentView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6531DED246B81C9000F9538 /* GifvAttachmentView.swift */; }; D6531DF0246B867E000F9538 /* GifvAttachmentViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6531DEF246B867E000F9538 /* GifvAttachmentViewController.swift */; }; @@ -471,7 +471,7 @@ D64BC19123C271D9000D0238 /* MastodonActivity.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MastodonActivity.swift; sourceTree = ""; }; D64D0AAC2128D88B005A6F37 /* LocalData.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LocalData.swift; sourceTree = ""; }; D64D0AB02128D9AE005A6F37 /* OnboardingViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OnboardingViewController.swift; sourceTree = ""; }; - D64D8CA82463B494006B0BAA /* CachedDictionary.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CachedDictionary.swift; sourceTree = ""; }; + D64D8CA82463B494006B0BAA /* MultiThreadDictionary.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MultiThreadDictionary.swift; sourceTree = ""; }; D64F80E1215875CC00BEF393 /* XCBActionType.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = XCBActionType.swift; sourceTree = ""; }; D6531DED246B81C9000F9538 /* GifvAttachmentView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GifvAttachmentView.swift; sourceTree = ""; }; D6531DEF246B867E000F9538 /* GifvAttachmentViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GifvAttachmentViewController.swift; sourceTree = ""; }; @@ -1360,7 +1360,7 @@ D6945C2E23AC47C3005C403C /* SavedDataManager.swift */, D6C693EE216192C2007D6A6D /* TuskerNavigationDelegate.swift */, D6DFC69F242C4CCC00ACC392 /* WeakArray.swift */, - D64D8CA82463B494006B0BAA /* CachedDictionary.swift */, + D64D8CA82463B494006B0BAA /* MultiThreadDictionary.swift */, D60E2F2B24423EAD005F8713 /* LazilyDecoding.swift */, D6CA6A91249FAD8900AD45C1 /* AudioSessionHelper.swift */, D6E4269C2532A3E100C02E1C /* FuzzyMatcher.swift */, @@ -1928,7 +1928,7 @@ D641C773213CAA25004B4513 /* NotificationsTableViewController.swift in Sources */, D6412B0524B0227D00F5412E /* ProfileViewController.swift in Sources */, D64BC18A23C16487000D0238 /* UnpinStatusActivity.swift in Sources */, - D64D8CA92463B494006B0BAA /* CachedDictionary.swift in Sources */, + D64D8CA92463B494006B0BAA /* MultiThreadDictionary.swift in Sources */, D6757A7C2157E01900721E32 /* XCBManager.swift in Sources */, D6F1F84D2193B56E00F5FE67 /* Cache.swift in Sources */, D6F0B12B24A3071C001E48C3 /* MainSplitViewController.swift in Sources */, diff --git a/Tusker/CachedDictionary.swift b/Tusker/CachedDictionary.swift deleted file mode 100644 index 3760faaa..00000000 --- a/Tusker/CachedDictionary.swift +++ /dev/null @@ -1,35 +0,0 @@ -// -// CachedDictionary.swift -// Tusker -// -// Created by Shadowfacts on 5/6/20. -// Copyright © 2020 Shadowfacts. All rights reserved. -// - -import Foundation - -class CachedDictionary { - private let name: String - private var dict = [String: Value]() - private let queue: DispatchQueue - - init(name: String) { - self.name = name - self.queue = DispatchQueue(label: "CachedDictionary (\(name)) Coordinator", attributes: .concurrent) - } - - subscript(key: String) -> Value? { - get { - var result: Value? = nil - queue.sync { - result = dict[key] - } - return result - } - set(value) { - queue.async(flags: .barrier) { - self.dict[key] = value - } - } - } -} diff --git a/Tusker/Caching/ImageCache.swift b/Tusker/Caching/ImageCache.swift index 6c8de3a6..2145f920 100644 --- a/Tusker/Caching/ImageCache.swift +++ b/Tusker/Caching/ImageCache.swift @@ -24,7 +24,7 @@ class ImageCache { private let cache: Cache - private var groups = [URL: RequestGroup]() + private var groups = MultiThreadDictionary(name: "ImageCache request groups") init(name: String, memoryExpiry expiry: Expiry) { let storage = MemoryStorage(config: MemoryConfig(expiry: expiry)) @@ -56,7 +56,7 @@ class ImageCache { if let data = data { try? self.cache.setObject(data, forKey: key) } - self.groups.removeValue(forKey: url) + self.groups.removeValueWithoutReturning(forKey: url) } groups[url] = group let request = group.addCallback(completion) diff --git a/Tusker/MultiThreadDictionary.swift b/Tusker/MultiThreadDictionary.swift new file mode 100644 index 00000000..a7622aad --- /dev/null +++ b/Tusker/MultiThreadDictionary.swift @@ -0,0 +1,50 @@ +// +// MultiThreadDictionary.swift +// Tusker +// +// Created by Shadowfacts on 5/6/20. +// Copyright © 2020 Shadowfacts. All rights reserved. +// + +import Foundation + +class MultiThreadDictionary { + private let name: String + private var dict = [Key: Value]() + private let queue: DispatchQueue + + init(name: String) { + self.name = name + self.queue = DispatchQueue(label: "MultiThreadDictionary (\(name)) Coordinator", attributes: .concurrent) + } + + subscript(key: Key) -> Value? { + get { + var result: Value? = nil + queue.sync { + result = dict[key] + } + return result + } + set(value) { + queue.async(flags: .barrier) { + self.dict[key] = value + } + } + } + + func removeValueWithoutReturning(forKey key: Key) { + queue.async(flags: .barrier) { + self.dict.removeValue(forKey: key) + } + } + + /// If the result of this function is unused, it is preferable to use `removeValueWithoutReturning` as it executes asynchronously and doesn't block the calling thread. + func removeValue(forKey key: Key) -> Value? { + var value: Value? = nil + queue.sync(flags: .barrier) { + value = dict.removeValue(forKey: key) + } + return value + } +} diff --git a/Tusker/Views/AccountDisplayNameLabel.swift b/Tusker/Views/AccountDisplayNameLabel.swift index c7c2c156..06076a99 100644 --- a/Tusker/Views/AccountDisplayNameLabel.swift +++ b/Tusker/Views/AccountDisplayNameLabel.swift @@ -41,7 +41,7 @@ struct AccountDisplayNameLabel: View { let matches = emojiRegex.matches(in: account.displayName, options: [], range: fullRange) guard !matches.isEmpty else { return } - let emojiImages = CachedDictionary(name: "AcccountDisplayNameLabel Emoji Images") + let emojiImages = MultiThreadDictionary(name: "AcccountDisplayNameLabel Emoji Images") let group = DispatchGroup() diff --git a/Tusker/Views/ContentTextView.swift b/Tusker/Views/ContentTextView.swift index 2e041bd0..d5ce9ec2 100644 --- a/Tusker/Views/ContentTextView.swift +++ b/Tusker/Views/ContentTextView.swift @@ -51,7 +51,7 @@ class ContentTextView: LinkTextView { func setEmojis(_ emojis: [Emoji]) { guard !emojis.isEmpty else { return } - let emojiImages = CachedDictionary(name: "ContentTextView Emoji Images") + let emojiImages = MultiThreadDictionary(name: "ContentTextView Emoji Images") let group = DispatchGroup() diff --git a/Tusker/Views/EmojiLabel.swift b/Tusker/Views/EmojiLabel.swift index 56fb34bb..cc771614 100644 --- a/Tusker/Views/EmojiLabel.swift +++ b/Tusker/Views/EmojiLabel.swift @@ -26,7 +26,7 @@ class EmojiLabel: UILabel { let matches = emojiRegex.matches(in: attributedText.string, options: [], range: attributedText.fullRange) guard !matches.isEmpty else { return } - let emojiImages = CachedDictionary(name: "EmojiLabel Emoji Images") + let emojiImages = MultiThreadDictionary(name: "EmojiLabel Emoji Images") let group = DispatchGroup()