Merge pull request SDWebImage#3380 from dreampiggy/bugfix_edge_case_cancel_cache_callback_twice

Fix the rare case when cancel an async disk cache query may cause twice callback

Author: dreampiggy

SDWebImage/Core/SDImageCache.m

@@ -662,6 +662,13 @@ - (nullable SDImageCacheToken *)queryCacheOperationForKey:(nullable NSString *)k
             }
             if (doneBlock) {
                 dispatch_async(dispatch_get_main_queue(), ^{
+                    // Dispatch from IO queue to main queue need time, user may call cancel during the dispatch timing
+                    // This check is here to avoid double callback (one is from `SDImageCacheToken` in sync)
+                    @synchronized (operation) {
+                        if (operation.isCancelled) {
+                            return;
+                        }
+                    }
                     doneBlock(diskImage, diskData, SDImageCacheTypeDisk);
                 });
             }

SDWebImage/Core/SDWebImageManager.m

@@ -382,6 +382,11 @@ - (void)callDownloadProcessForOperation:(nonnull SDWebImageCombinedOperation *)o
                               cacheType:(SDImageCacheType)cacheType
                                progress:(nullable SDImageLoaderProgressBlock)progressBlock
                               completed:(nullable SDInternalCompletionBlock)completedBlock {
+    // Mark the cache operation end
+    @synchronized (operation) {
+        operation.cacheOperation = nil;
+    }
+    
     // Grab the image loader to use
     id<SDImageLoader> imageLoader;
     if ([context[SDWebImageContextImageLoader] conformsToProtocol:@protocol(SDImageLoader)]) {

Tests/Tests/SDImageCacheTests.m

@@ -136,14 +136,13 @@ - (void)test09RetrieveImageThroughNSOperation {
     XCTestExpectation *expectation = [self expectationWithDescription:@"queryCacheOperationForKey"];
     UIImage *imageForTesting = [self testJPEGImage];
     [[SDImageCache sharedImageCache] storeImage:imageForTesting forKey:kTestImageKeyJPEG completion:nil];
-    NSOperation *operation = [[SDImageCache sharedImageCache] queryCacheOperationForKey:kTestImageKeyJPEG done:^(UIImage *image, NSData *data, SDImageCacheType cacheType) {
+    id<SDWebImageOperation> operation = [[SDImageCache sharedImageCache] queryCacheOperationForKey:kTestImageKeyJPEG done:^(UIImage *image, NSData *data, SDImageCacheType cacheType) {
         expect(image).to.equal(imageForTesting);
         [[SDImageCache sharedImageCache] removeImageForKey:kTestImageKeyJPEG withCompletion:^{
             [expectation fulfill];
         }];
     }];
     expect(operation).toNot.beNil;
-    [operation start];
     [self waitForExpectationsWithCommonTimeout];
 }
 
@@ -209,6 +208,26 @@ - (void)test14QueryCacheFirstFrameOnlyHitMemoryCache {
     [[SDImageCache sharedImageCache] removeImageFromMemoryForKey:kTestGIFURL];
 }
 
+- (void)test15CancelQueryShouldCallbackOnceInSync {
+    XCTestExpectation *expectation = [self expectationWithDescription:@"Cancel Query Should Callback Once In Sync"];
+    expectation.expectedFulfillmentCount = 1;
+    NSString *key = @"test15CancelQueryShouldCallbackOnceInSync";
+    [SDImageCache.sharedImageCache removeImageFromMemoryForKey:key];
+    [SDImageCache.sharedImageCache removeImageFromDiskForKey:key];
+    __block BOOL callced = NO;
+    SDImageCacheToken *token = [SDImageCache.sharedImageCache queryCacheOperationForKey:key done:^(UIImage * _Nullable image, NSData * _Nullable data, SDImageCacheType cacheType) {
+        callced = true;
+        [expectation fulfill]; // callback once fulfill once
+    }];
+    dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(1 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
+        expect(callced).beFalsy();
+        [token cancel]; // sync
+        expect(callced).beTruthy();
+    });
+    
+    [self waitForExpectationsWithCommonTimeout];
+}
+
 - (void)test20InitialCacheSize{
     expect([[SDImageCache sharedImageCache] totalDiskSize]).to.equal(0);
 }

Tests/Tests/SDWebImageManagerTests.m

@@ -11,6 +11,14 @@
 #import "SDWebImageTestCache.h"
 #import "SDWebImageTestLoader.h"
 
+// Keep strong references for object
+@interface SDObjectContainer<ObjectType> : NSObject
+@property (nonatomic, strong, readwrite) ObjectType object;
+@end
+
+@implementation SDObjectContainer
+@end
+
 @interface SDWebImageManagerTests : SDTestCase
 
 @end
@@ -27,14 +35,24 @@ - (void)test02ThatDownloadInvokesCompletionBlockWithCorrectParamsAsync {
 
     NSURL *originalImageURL = [NSURL URLWithString:kTestJPEGURL];
 
-    [[SDWebImageManager sharedManager] loadImageWithURL:originalImageURL
-                                                options:SDWebImageRefreshCached
-                                               progress:nil
-                                              completed:^(UIImage *image, NSData *data, NSError *error, SDImageCacheType cacheType, BOOL finished, NSURL *imageURL) {
+    [SDImageCache.sharedImageCache removeImageFromMemoryForKey:kTestJPEGURL];
+    [SDImageCache.sharedImageCache removeImageFromDiskForKey:kTestJPEGURL];
+    SDObjectContainer<SDWebImageCombinedOperation *> *container = [SDObjectContainer new];
+    container.object = [[SDWebImageManager sharedManager] loadImageWithURL:originalImageURL
+                                                                   options:0
+                                                                  progress:nil
+                                                                 completed:^(UIImage *image, NSData *data, NSError *error, SDImageCacheType cacheType, BOOL finished, NSURL *imageURL) {
         expect(image).toNot.beNil();
         expect(error).to.beNil();
         expect(originalImageURL).to.equal(imageURL);
-
+        
+        // When download, the cache operation will reset to nil since it's always finished
+        SDWebImageCombinedOperation *operation = container.object;
+        expect(container).notTo.beNil();
+        expect(operation.cacheOperation).beNil();
+        expect(operation.loaderOperation).notTo.beNil();
+        container.object = nil;
+        
         [expectation fulfill];
         expectation = nil;
     }];
@@ -49,7 +67,7 @@ - (void)test03ThatDownloadWithIncorrectURLInvokesCompletionBlockWithAnErrorAsync
     NSURL *originalImageURL = [NSURL URLWithString:@"http://static2.dmcdn.net/static/video/656/177/44771656:jpeg_preview_small.png"];
 
     [[SDWebImageManager sharedManager] loadImageWithURL:originalImageURL
-                                                options:SDWebImageRefreshCached
+                                                options:0
                                                progress:nil
                                               completed:^(UIImage *image, NSData *data, NSError *error, SDImageCacheType cacheType, BOOL finished, NSURL *imageURL) {
         expect(image).to.beNil();