Try not introduce new public API for operation, attach the isCompleted property using associated object from downloader, ensure compatibility

Author: dreampiggy

SDWebImage/Core/SDWebImageDownloader.m

@@ -13,13 +13,30 @@
 #import "SDWebImageCacheKeyFilter.h"
 #import "SDImageCacheDefine.h"
 #import "SDInternalMacros.h"
+#import "objc/runtime.h"
 
 NSNotificationName const SDWebImageDownloadStartNotification = @"SDWebImageDownloadStartNotification";
 NSNotificationName const SDWebImageDownloadReceiveResponseNotification = @"SDWebImageDownloadReceiveResponseNotification";
 NSNotificationName const SDWebImageDownloadStopNotification = @"SDWebImageDownloadStopNotification";
 NSNotificationName const SDWebImageDownloadFinishNotification = @"SDWebImageDownloadFinishNotification";
 
 static void * SDWebImageDownloaderContext = &SDWebImageDownloaderContext;
+static void * SDWebImageDownloaderOperationKey = &SDWebImageDownloaderOperationKey;
+
+BOOL SDWebImageDownloaderOperationGetCompleted(id<SDWebImageDownloaderOperation> operation) {
+    NSCParameterAssert(operation);
+    NSNumber *value = objc_getAssociatedObject(operation, SDWebImageDownloaderOperationKey);
+    if (value) {
+        return value.boolValue;
+    } else {
+        return NO;
+    }
+}
+
+void SDWebImageDownloaderOperationSetCompleted(id<SDWebImageDownloaderOperation> operation, BOOL isCompleted) {
+    NSCParameterAssert(operation);
+    objc_setAssociatedObject(operation, SDWebImageDownloaderOperationKey, @(isCompleted), OBJC_ASSOCIATION_RETAIN);
+}
 
 @interface SDWebImageDownloadToken ()
 
@@ -219,7 +236,8 @@ - (nullable SDWebImageDownloadToken *)downloadImageWithURL:(nullable NSURL *)url
     SDImageCoderOptions *decodeOptions = SDGetDecodeOptionsFromContext(context, [self.class imageOptionsFromDownloaderOptions:options], cacheKey);
     NSOperation<SDWebImageDownloaderOperation> *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 || operation.isTransferFinished) {
+    BOOL shouldNotReuseOperation = !operation || operation.isFinished || operation.isCancelled || SDWebImageDownloaderOperationGetCompleted(operation);
+    if (shouldNotReuseOperation) {
         operation = [self createDownloaderOperationWithUrl:url options:options context:context];
         if (!operation) {
             SD_UNLOCK(_operationsLock);
@@ -499,6 +517,10 @@ - (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task didComp
 
     // Identify the operation that runs this task and pass it the delegate method
     NSOperation<SDWebImageDownloaderOperation> *dataOperation = [self operationWithTask:task];
+    if (dataOperation) {
+        // Mark the downloader operation `isCompleted = YES`, no longer re-use this operation when new request comes in
+        SDWebImageDownloaderOperationSetCompleted(dataOperation, true);
+    }
     if ([dataOperation respondsToSelector:@selector(URLSession:task:didCompleteWithError:)]) {
         [dataOperation URLSession:session task:task didCompleteWithError:error];
     }

SDWebImage/Core/SDWebImageDownloaderOperation.h

@@ -35,8 +35,6 @@
 
 - (BOOL)cancel:(nullable id)token;
 
-@property (assign, readonly) BOOL isTransferFinished;
-
 @property (strong, nonatomic, readonly, nullable) NSURLRequest *request;
 @property (strong, nonatomic, readonly, nullable) NSURLResponse *response;
 
@@ -58,10 +56,6 @@
  */
 @interface SDWebImageDownloaderOperation : NSOperation <SDWebImageDownloaderOperation>
 
-/// Whether the operation's network data transfer is finished. This is used by downloader to decide whether to call `addHandlersForProgress:`, or create a new operation instead.
-/// @note You must implements this or this will cause downloader attach new handlers for a already finished operation, may cause some callback missing.
-@property (assign, readonly) BOOL isTransferFinished;
-
 /**
  * The request used by the operation's task.
  */

SDWebImage/Core/SDWebImageDownloaderOperation.m

@@ -14,6 +14,8 @@
 #import "SDImageCacheDefine.h"
 #import "SDCallbackQueue.h"
 
+BOOL SDWebImageDownloaderOperationGetCompleted(id<SDWebImageDownloaderOperation> operation); // Private currently, mark open if needed
+
 // A handler to represent individual request
 @interface SDWebImageDownloaderOperationToken : NSObject
 
@@ -74,7 +76,6 @@ @interface SDWebImageDownloaderOperation ()
 @property (strong, nonatomic, readwrite, nullable) NSURLSessionTaskMetrics *metrics API_AVAILABLE(macosx(10.12), ios(10.0), watchos(3.0), tvos(10.0));
 
 @property (strong, nonatomic, nonnull) dispatch_queue_t coderQueue; // the serial operation queue to do image decoding
-@property (assign, readwrite) BOOL isTransferFinished; // Whether current operation's network transfer is finished (actually, `didCompleteWithError` already been called)
 
 @property (strong, nonatomic, nonnull) NSMapTable<SDImageCoderOptions *, UIImage *> *imageMap; // each variant of image is weak-referenced to avoid too many re-decode during downloading
 #if SD_UIKIT
@@ -472,7 +473,7 @@ - (void)URLSession:(NSURLSession *)session dataTask:(NSURLSessionDataTask *)data
                 return;
             }
             // When cancelled or transfer finished (`didCompleteWithError`), cancel the progress callback, only completed block is called and enough
-            if (self.isCancelled || self.isTransferFinished) {
+            if (self.isCancelled || SDWebImageDownloaderOperationGetCompleted(self)) {
                 return;
             }
             UIImage *image = SDImageLoaderDecodeProgressiveImageData(imageData, self.request.URL, NO, self, [[self class] imageOptionsFromDownloaderOptions:self.options], self.context);
@@ -512,7 +513,6 @@ - (void)URLSession:(NSURLSession *)session
 - (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task didCompleteWithError:(NSError *)error {
     // If we already cancel the operation or anything mark the operation finished, don't callback twice
     if (self.isFinished) return;
-    self.isTransferFinished = YES;
 
     NSArray<SDWebImageDownloaderOperationToken *> *tokens;
     @synchronized (self) {
@@ -599,16 +599,13 @@ - (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task didComp
                         });
                     }
                     // call [self done] after all completed block was dispatched
-                    dispatch_barrier_async(self.coderQueue, ^{
+                    dispatch_async(self.coderQueue, ^{
                         @strongify(self);
                         if (!self) {
                             return;
                         }
                         [self done];
                     });
-                    dispatch_async(self.coderQueue, ^{
-                        [self done];
-                    });
                 }
             } else {
                 [self callCompletionBlocksWithError:[NSError errorWithDomain:SDWebImageErrorDomain code:SDWebImageErrorBadImageData userInfo:@{NSLocalizedDescriptionKey : @"Image data is nil"}]];