From 384b2d7c8be97bb52eed789662b4b97f5a58ab17 Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Fri, 11 Nov 2022 14:22:49 +0800 Subject: [PATCH 1/2] Fix the missing lock for callbackTokens which may cause thread-safe issue --- SDWebImage/Core/SDWebImageDownloaderOperation.m | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/SDWebImage/Core/SDWebImageDownloaderOperation.m b/SDWebImage/Core/SDWebImageDownloaderOperation.m index 837883e2..3c0c8abd 100644 --- a/SDWebImage/Core/SDWebImageDownloaderOperation.m +++ b/SDWebImage/Core/SDWebImageDownloaderOperation.m @@ -244,7 +244,10 @@ self.coderQueue.qualityOfService = NSQualityOfServiceDefault; } [self.dataTask resume]; - NSArray *tokens = [self.callbackTokens copy]; + NSArray *tokens; + @synchronized (self) { + tokens = [self.callbackTokens copy]; + } for (SDWebImageDownloaderOperationToken *token in tokens) { if (token.progressBlock) { token.progressBlock(0, NSURLResponseUnknownLength, self.request.URL); From 736f3f41f74c70a5c82546238cc25d377cb51436 Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Fri, 11 Nov 2022 14:29:20 +0800 Subject: [PATCH 2/2] Move block before sending to the main queue This can avoid some life cycle issue and increase performance --- SDWebImage/Core/SDImageCache.m | 13 ++++---- .../Core/SDWebImageDownloaderOperation.m | 30 ++++++++++--------- SDWebImage/Core/SDWebImageManager.m | 8 ++--- SDWebImage/Core/UIView+WebCache.m | 8 ++--- 4 files changed, 31 insertions(+), 28 deletions(-) diff --git a/SDWebImage/Core/SDImageCache.m b/SDWebImage/Core/SDImageCache.m index 5810996c..94e2cd77 100644 --- a/SDWebImage/Core/SDImageCache.m +++ b/SDWebImage/Core/SDImageCache.m @@ -40,12 +40,13 @@ } self.cancelled = YES; - dispatch_main_async_safe(^{ - if (self.doneBlock) { - self.doneBlock(nil, nil, SDImageCacheTypeNone); - self.doneBlock = nil; - } - }); + SDImageCacheQueryCompletionBlock doneBlock = self.doneBlock; + self.doneBlock = nil; + if (doneBlock) { + dispatch_main_async_safe(^{ + doneBlock(nil, nil, SDImageCacheTypeNone); + }); + } } } diff --git a/SDWebImage/Core/SDWebImageDownloaderOperation.m b/SDWebImage/Core/SDWebImageDownloaderOperation.m index 3c0c8abd..82c3772c 100644 --- a/SDWebImage/Core/SDWebImageDownloaderOperation.m +++ b/SDWebImage/Core/SDWebImageDownloaderOperation.m @@ -158,11 +158,11 @@ [self.callbackTokens removeObjectIdenticalTo:token]; } SDWebImageDownloaderCompletedBlock completedBlock = ((SDWebImageDownloaderOperationToken *)token).completedBlock; - dispatch_main_async_safe(^{ - if (completedBlock) { + if (completedBlock) { + dispatch_main_async_safe(^{ completedBlock(nil, nil, [NSError errorWithDomain:SDWebImageErrorDomain code:SDWebImageErrorCancelled userInfo:@{NSLocalizedDescriptionKey : @"Operation cancelled by user during sending the request"}], YES); - } - }); + }); + } } return shouldCancel; } @@ -697,11 +697,12 @@ didReceiveResponse:(NSURLResponse *)response tokens = [self.callbackTokens copy]; } for (SDWebImageDownloaderOperationToken *token in tokens) { - dispatch_main_async_safe(^{ - if (token.completedBlock) { - token.completedBlock(image, imageData, error, finished); - } - }); + SDWebImageDownloaderCompletedBlock completedBlock = token.completedBlock; + if (completedBlock) { + dispatch_main_async_safe(^{ + completedBlock(image, imageData, error, finished); + }); + } } } @@ -710,11 +711,12 @@ didReceiveResponse:(NSURLResponse *)response imageData:(nullable NSData *)imageData error:(nullable NSError *)error finished:(BOOL)finished { - dispatch_main_async_safe(^{ - if (token.completedBlock) { - token.completedBlock(image, imageData, error, finished); - } - }); + SDWebImageDownloaderCompletedBlock completedBlock = token.completedBlock; + if (completedBlock) { + dispatch_main_async_safe(^{ + completedBlock(image, imageData, error, finished); + }); + } } @end diff --git a/SDWebImage/Core/SDWebImageManager.m b/SDWebImage/Core/SDWebImageManager.m index 00b99573..e02a2409 100644 --- a/SDWebImage/Core/SDWebImageManager.m +++ b/SDWebImage/Core/SDWebImageManager.m @@ -672,11 +672,11 @@ static id _defaultImageLoader; cacheType:(SDImageCacheType)cacheType finished:(BOOL)finished url:(nullable NSURL *)url { - dispatch_main_async_safe(^{ - if (completionBlock) { + if (completionBlock) { + dispatch_main_async_safe(^{ completionBlock(image, data, error, cacheType, finished, url); - } - }); + }); + } } - (BOOL)shouldBlockFailedURLWithURL:(nonnull NSURL *)url diff --git a/SDWebImage/Core/UIView+WebCache.m b/SDWebImage/Core/UIView+WebCache.m index 486584f4..569d53f2 100644 --- a/SDWebImage/Core/UIView+WebCache.m +++ b/SDWebImage/Core/UIView+WebCache.m @@ -229,12 +229,12 @@ const int64_t SDWebImageProgressUnitCountUnknown = 1LL; #if SD_UIKIT || SD_MAC [self sd_stopImageIndicator]; #endif - dispatch_main_async_safe(^{ - if (completedBlock) { + if (completedBlock) { + dispatch_main_async_safe(^{ NSError *error = [NSError errorWithDomain:SDWebImageErrorDomain code:SDWebImageErrorInvalidURL userInfo:@{NSLocalizedDescriptionKey : @"Image url is nil"}]; completedBlock(nil, nil, error, SDImageCacheTypeNone, YES, url); - } - }); + }); + } } return operation;