From 948cd3068acba7dac266cfb3933f2480428bef12 Mon Sep 17 00:00:00 2001 From: ronaldheft Date: Mon, 8 Aug 2022 13:47:37 -0400 Subject: [PATCH] Fix more race conditions with progress reporting --- ios/App/App/plugins/AbsDownloader.swift | 80 +++++++++++++++++------- ios/App/Shared/models/DownloadItem.swift | 2 +- 2 files changed, 59 insertions(+), 23 deletions(-) diff --git a/ios/App/App/plugins/AbsDownloader.swift b/ios/App/App/plugins/AbsDownloader.swift index d39be724..22118889 100644 --- a/ios/App/App/plugins/AbsDownloader.swift +++ b/ios/App/App/plugins/AbsDownloader.swift @@ -19,11 +19,9 @@ public class AbsDownloader: CAPPlugin, URLSessionDownloadDelegate { queue.maxConcurrentOperationCount = 5 return URLSession(configuration: .default, delegate: self, delegateQueue: queue) }() - private let progressStatusQueue: OperationQueue = { - let queue = OperationQueue() - queue.maxConcurrentOperationCount = 1 - return queue - }() + private let progressStatusQueue = DispatchQueue(label: "progress-status-queue", attributes: .concurrent) + private var progressStatusWorkItem: DispatchWorkItem? + private var downloadItemProgress = [String: DownloadItem]() public func urlSession(_ session: URLSession, downloadTask: URLSessionDownloadTask, didFinishDownloadingTo location: URL) { handleDownloadTaskUpdate(downloadTask: downloadTask) { downloadItem, downloadItemPart in @@ -59,7 +57,10 @@ public class AbsDownloader: CAPPlugin, URLSessionDownloadDelegate { handleDownloadTaskUpdate(downloadTask: downloadTask) { downloadItem, downloadItemPart in // Calculate the download percentage let percentDownloaded = (Double(totalBytesWritten) / Double(totalBytesExpectedToWrite)) * 100 - downloadItemPart.progress = percentDownloaded + // Only update the progress if we received accurate progress data + if percentDownloaded >= 0.0 && percentDownloaded <= 1.0 { + downloadItemPart.progress = percentDownloaded + } } } @@ -86,22 +87,9 @@ public class AbsDownloader: CAPPlugin, URLSessionDownloadDelegate { // Update the progress Database.shared.updateDownloadItemPart(downloadItem.downloadItemParts[partIndex]) - - // Notify the UI - try? self.notifyListeners("onItemDownloadUpdate", data: downloadItem.asDictionary()) - - // Handle a completed download - if ( downloadItem.isDoneDownloading() ) { - // Prevent race condition when multiple parts finish downloading at the same time by using a queue - self.progressStatusQueue.addOperation { - // Remove the download item after the operation completes - defer { Database.shared.removeDownloadItem(downloadItem) } - // Fetch the latest download item, so we know if it was removed in another thread - let downloadItem = Database.shared.getDownloadItem(downloadItemId: downloadItem.id!) - // We already processed this download item on another thread, skip it - guard let downloadItem = downloadItem else { return } - self.handleDownloadTaskCompleteFromDownloadItem(downloadItem) - } + self.progressStatusQueue.async(flags: .barrier) { + self.downloadItemProgress.updateValue(downloadItem, forKey: downloadItem.id!) + self.notifyDownloadProgress() } } catch { NSLog("DownloadItemError") @@ -109,6 +97,54 @@ public class AbsDownloader: CAPPlugin, URLSessionDownloadDelegate { } } + // We want to handle updating the UI in the background and throttled so we don't overload the UI with progress updates + private func notifyDownloadProgress() { + // Return if the loop is running + guard self.progressStatusWorkItem == nil else { return } + + // Create a background thread to send the download status + self.progressStatusWorkItem = DispatchWorkItem { [weak self] in + // Clean up the work item when done + defer { self?.progressStatusWorkItem = nil } + var downloadItemProgress = [String: DownloadItem]() + + // Get the latest progress + self?.progressStatusQueue.sync { + if let progressItems = self?.downloadItemProgress { + downloadItemProgress = progressItems + } + } + + while !downloadItemProgress.isEmpty { + for item in downloadItemProgress.values { + try? self?.notifyListeners("onItemDownloadUpdate", data: item.asDictionary()) + + // Clean up a completed download + if item.isDoneDownloading() { + self?.progressStatusQueue.async(flags: .barrier) { + self?.downloadItemProgress.removeValue(forKey: item.id!) + } + defer { Database.shared.removeDownloadItem(item) } + self?.handleDownloadTaskCompleteFromDownloadItem(item) + } + } + + // Wait 200ms before reporting status again + Thread.sleep(forTimeInterval: TimeInterval(0.2)) + + // Get the latest progress + self?.progressStatusQueue.sync { + if let progressItems = self?.downloadItemProgress { + downloadItemProgress = progressItems + } + } + } + } + + // Start the thread + DispatchQueue.global(qos: .userInteractive).async(execute: self.progressStatusWorkItem!) + } + private func handleDownloadTaskCompleteFromDownloadItem(_ downloadItem: DownloadItem) { if ( downloadItem.didDownloadSuccessfully() ) { ApiClient.getLibraryItemWithProgress(libraryItemId: downloadItem.libraryItemId!, episodeId: downloadItem.episodeId) { libraryItem in diff --git a/ios/App/Shared/models/DownloadItem.swift b/ios/App/Shared/models/DownloadItem.swift index b7977edc..5fa8e33f 100644 --- a/ios/App/Shared/models/DownloadItem.swift +++ b/ios/App/Shared/models/DownloadItem.swift @@ -30,7 +30,7 @@ struct DownloadItem: Realmable, Codable { } private enum CodingKeys : String, CodingKey { - case id, libraryItemId, episodeId, userMediaProgress, serverConnectionConfigId, serverAddress, serverUserId, mediaType, itemTitle, downloadItemParts + case id, libraryItemId, episodeId, serverConnectionConfigId, serverAddress, serverUserId, mediaType, itemTitle, downloadItemParts } }