From 482d45dc65fc4ed0fe3757ec7af501ca32e155b3 Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Thu, 26 Dec 2019 17:38:46 +0800 Subject: [PATCH] Fix the issue that "There may be no complete callback when download the picture of the local path" --- SDWebImage/SDWebImageDownloader.m | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/SDWebImage/SDWebImageDownloader.m b/SDWebImage/SDWebImageDownloader.m index 12fb24d3..30a2132a 100644 --- a/SDWebImage/SDWebImageDownloader.m +++ b/SDWebImage/SDWebImageDownloader.m @@ -293,6 +293,7 @@ } LOCK(self.operationsLock); + id downloadOperationCancelToken; NSOperation *operation = [self.URLOperations objectForKey:url]; // There is a case that the operation may be marked as finished or cancelled, but not been removed from `self.URLOperations`. if (!operation || operation.isFinished || operation.isCancelled) { @@ -308,22 +309,28 @@ UNLOCK(sself.operationsLock); }; [self.URLOperations setObject:operation forKey:url]; + // Add the handlers before submitting to operation queue, avoid the race condition that operation finished before setting handlers. + downloadOperationCancelToken = [operation addHandlersForProgress:progressBlock completed:completedBlock]; // Add operation to operation queue only after all configuration done according to Apple's doc. // `addOperation:` does not synchronously execute the `operation.completionBlock` so this will not cause deadlock. [self.downloadQueue addOperation:operation]; - } - else if (!operation.isExecuting) { - if (options & SDWebImageDownloaderHighPriority) { - operation.queuePriority = NSOperationQueuePriorityHigh; - } else if (options & SDWebImageDownloaderLowPriority) { - operation.queuePriority = NSOperationQueuePriorityLow; - } else { - operation.queuePriority = NSOperationQueuePriorityNormal; + } else { + // When we reuse the download operation to attach more callbacks, there may be thread safe issue because the getter of callbacks may in another queue (decoding queue or delegate queue) + // So we lock the operation here, and in `SDWebImageDownloaderOperation`, we use `@synchonzied (self)`, to ensure the thread safe between these two classes. + @synchronized (operation) { + downloadOperationCancelToken = [operation addHandlersForProgress:progressBlock completed:completedBlock]; + } + if (!operation.isExecuting) { + if (options & SDWebImageDownloaderHighPriority) { + operation.queuePriority = NSOperationQueuePriorityHigh; + } else if (options & SDWebImageDownloaderLowPriority) { + operation.queuePriority = NSOperationQueuePriorityLow; + } else { + operation.queuePriority = NSOperationQueuePriorityNormal; + } } } UNLOCK(self.operationsLock); - - id downloadOperationCancelToken = [operation addHandlersForProgress:progressBlock completed:completedBlock]; SDWebImageDownloadToken *token = [SDWebImageDownloadToken new]; token.downloadOperation = operation;