From 958a349c6c01a5587c94235c9cb43a64464f0aaa Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Sun, 17 Dec 2017 01:26:03 +0800 Subject: [PATCH 1/4] Use a copy-weak maptable for operations stored in UIView(WebCacheOperation) category to avoid retain of operation, and also use lock to keep thread-safe --- SDWebImage/UIView+WebCacheOperation.m | 33 +++++++++++++++++++-------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/SDWebImage/UIView+WebCacheOperation.m b/SDWebImage/UIView+WebCacheOperation.m index a515a74f..f05afec9 100644 --- a/SDWebImage/UIView+WebCacheOperation.m +++ b/SDWebImage/UIView+WebCacheOperation.m @@ -14,18 +14,22 @@ static char loadOperationKey; -typedef NSMutableDictionary SDOperationsDictionary; +// key is copy, value is weak because operation instance is retained by SDWebImageManager's runningOperations property +// we should use lock to keep thread-safe because these method may not be acessed from main queue +typedef NSMapTable SDOperationsDictionary; @implementation UIView (WebCacheOperation) - (SDOperationsDictionary *)operationDictionary { - SDOperationsDictionary *operations = objc_getAssociatedObject(self, &loadOperationKey); - if (operations) { + @synchronized(self) { + SDOperationsDictionary *operations = objc_getAssociatedObject(self, &loadOperationKey); + if (operations) { + return operations; + } + operations = [[NSMapTable alloc] initWithKeyOptions:NSMapTableCopyIn valueOptions:NSMapTableWeakMemory capacity:0]; + objc_setAssociatedObject(self, &loadOperationKey, operations, OBJC_ASSOCIATION_RETAIN_NONATOMIC); return operations; } - operations = [NSMutableDictionary dictionary]; - objc_setAssociatedObject(self, &loadOperationKey, operations, OBJC_ASSOCIATION_RETAIN_NONATOMIC); - return operations; } - (void)sd_setImageLoadOperation:(nullable id)operation forKey:(nullable NSString *)key { @@ -33,7 +37,9 @@ typedef NSMutableDictionary SDOperationsDictionary; [self sd_cancelImageLoadOperationWithKey:key]; if (operation) { SDOperationsDictionary *operationDictionary = [self operationDictionary]; - operationDictionary[key] = operation; + @synchronized (self) { + [operationDictionary setObject:operation forKey:key]; + } } } } @@ -41,7 +47,10 @@ typedef NSMutableDictionary SDOperationsDictionary; - (void)sd_cancelImageLoadOperationWithKey:(nullable NSString *)key { // Cancel in progress downloader from queue SDOperationsDictionary *operationDictionary = [self operationDictionary]; - id operations = operationDictionary[key]; + id operations; + @synchronized (self) { + operations = [operationDictionary objectForKey:key]; + } if (operations) { if ([operations isKindOfClass:[NSArray class]]) { for (id operation in operations) { @@ -52,14 +61,18 @@ typedef NSMutableDictionary SDOperationsDictionary; } else if ([operations conformsToProtocol:@protocol(SDWebImageOperation)]){ [(id) operations cancel]; } - [operationDictionary removeObjectForKey:key]; + @synchronized (self) { + [operationDictionary removeObjectForKey:key]; + } } } - (void)sd_removeImageLoadOperationWithKey:(nullable NSString *)key { if (key) { SDOperationsDictionary *operationDictionary = [self operationDictionary]; - [operationDictionary removeObjectForKey:key]; + @synchronized (self) { + [operationDictionary removeObjectForKey:key]; + } } } From 37f84ce6a65e7598c9be9177295f82915ee236b6 Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Sun, 17 Dec 2017 03:04:59 +0800 Subject: [PATCH 2/4] Use a weak pointerArray to store the operations for sd_setAnimationImagesWithURLs, avoid extra retain of operation instance --- SDWebImage/UIImageView+WebCache.m | 36 ++++++++++++++++++++++++--- SDWebImage/UIView+WebCacheOperation.h | 2 +- SDWebImage/UIView+WebCacheOperation.m | 26 ++++++++----------- 3 files changed, 43 insertions(+), 21 deletions(-) diff --git a/SDWebImage/UIImageView+WebCache.m b/SDWebImage/UIImageView+WebCache.m index 5a7e7988..8832f2e4 100644 --- a/SDWebImage/UIImageView+WebCache.m +++ b/SDWebImage/UIImageView+WebCache.m @@ -73,7 +73,7 @@ [self sd_cancelCurrentAnimationImagesLoad]; __weak __typeof(self)wself = self; - NSMutableArray> *operationsArray = [[NSMutableArray alloc] init]; + NSPointerArray *operationsArray = [self sd_animationOperationArray]; [arrayOfURLs enumerateObjectsUsingBlock:^(NSURL *logoImageURL, NSUInteger idx, BOOL * _Nonnull stop) { id operation = [[SDWebImageManager sharedManager] loadImageWithURL:logoImageURL options:0 progress:nil completed:^(UIImage *image, NSData *data, NSError *error, SDImageCacheType cacheType, BOOL finished, NSURL *imageURL) { @@ -103,14 +103,42 @@ [sself startAnimating]; }); }]; - [operationsArray addObject:operation]; + @synchronized (self) { + [operationsArray addPointer:(__bridge void *)(operation)]; + } }]; +} - [self sd_setImageLoadOperation:[operationsArray copy] forKey:@"UIImageViewAnimationImages"]; +static char animationloadOperationKey; + +// element is weak because operation instance is retained by SDWebImageManager's runningOperations property +// we should use lock to keep thread-safe because these method may not be acessed from main queue +- (NSPointerArray *)sd_animationOperationArray { + @synchronized(self) { + NSPointerArray *operationsArray = objc_getAssociatedObject(self, &animationloadOperationKey); + if (operationsArray) { + return operationsArray; + } + operationsArray = [NSPointerArray weakObjectsPointerArray]; + objc_setAssociatedObject(self, &animationloadOperationKey, operationsArray, OBJC_ASSOCIATION_RETAIN_NONATOMIC); + return operationsArray; + } } - (void)sd_cancelCurrentAnimationImagesLoad { - [self sd_cancelImageLoadOperationWithKey:@"UIImageViewAnimationImages"]; + NSPointerArray *operationsArray = [self sd_animationOperationArray]; + if (operationsArray) { + @synchronized (self) { + for (id operation in operationsArray) { + if ([operation conformsToProtocol:@protocol(SDWebImageOperation)]) { + [operation cancel]; + } + } + for (size_t i = 0; i < operationsArray.count; i++) { + [operationsArray removePointerAtIndex:i]; + } + } + } } #endif diff --git a/SDWebImage/UIView+WebCacheOperation.h b/SDWebImage/UIView+WebCacheOperation.h index f5e95fa6..bef703eb 100644 --- a/SDWebImage/UIView+WebCacheOperation.h +++ b/SDWebImage/UIView+WebCacheOperation.h @@ -20,7 +20,7 @@ * @param operation the operation * @param key key for storing the operation */ -- (void)sd_setImageLoadOperation:(nullable id)operation forKey:(nullable NSString *)key; +- (void)sd_setImageLoadOperation:(nullable id)operation forKey:(nullable NSString *)key; /** * Cancel all operations for the current UIView and key diff --git a/SDWebImage/UIView+WebCacheOperation.m b/SDWebImage/UIView+WebCacheOperation.m index f05afec9..bcdaa3d1 100644 --- a/SDWebImage/UIView+WebCacheOperation.m +++ b/SDWebImage/UIView+WebCacheOperation.m @@ -20,7 +20,7 @@ typedef NSMapTable SDOperationsDictionary; @implementation UIView (WebCacheOperation) -- (SDOperationsDictionary *)operationDictionary { +- (SDOperationsDictionary *)sd_operationDictionary { @synchronized(self) { SDOperationsDictionary *operations = objc_getAssociatedObject(self, &loadOperationKey); if (operations) { @@ -32,11 +32,11 @@ typedef NSMapTable SDOperationsDictionary; } } -- (void)sd_setImageLoadOperation:(nullable id)operation forKey:(nullable NSString *)key { +- (void)sd_setImageLoadOperation:(nullable id)operation forKey:(nullable NSString *)key { if (key) { [self sd_cancelImageLoadOperationWithKey:key]; if (operation) { - SDOperationsDictionary *operationDictionary = [self operationDictionary]; + SDOperationsDictionary *operationDictionary = [self sd_operationDictionary]; @synchronized (self) { [operationDictionary setObject:operation forKey:key]; } @@ -46,20 +46,14 @@ typedef NSMapTable SDOperationsDictionary; - (void)sd_cancelImageLoadOperationWithKey:(nullable NSString *)key { // Cancel in progress downloader from queue - SDOperationsDictionary *operationDictionary = [self operationDictionary]; - id operations; + SDOperationsDictionary *operationDictionary = [self sd_operationDictionary]; + id operation; @synchronized (self) { - operations = [operationDictionary objectForKey:key]; + operation = [operationDictionary objectForKey:key]; } - if (operations) { - if ([operations isKindOfClass:[NSArray class]]) { - for (id operation in operations) { - if (operation) { - [operation cancel]; - } - } - } else if ([operations conformsToProtocol:@protocol(SDWebImageOperation)]){ - [(id) operations cancel]; + if (operation) { + if ([operation conformsToProtocol:@protocol(SDWebImageOperation)]){ + [(id) operation cancel]; } @synchronized (self) { [operationDictionary removeObjectForKey:key]; @@ -69,7 +63,7 @@ typedef NSMapTable SDOperationsDictionary; - (void)sd_removeImageLoadOperationWithKey:(nullable NSString *)key { if (key) { - SDOperationsDictionary *operationDictionary = [self operationDictionary]; + SDOperationsDictionary *operationDictionary = [self sd_operationDictionary]; @synchronized (self) { [operationDictionary removeObjectForKey:key]; } From 91ff8016112927f73ca5c154556a876a2a3b5f46 Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Sun, 17 Dec 2017 03:05:56 +0800 Subject: [PATCH 3/4] Remove some unused code, fix typo, update the comments --- SDWebImage/UIButton+WebCache.m | 8 -------- SDWebImage/UIImageView+WebCache.m | 6 +++--- SDWebImage/UIView+WebCacheOperation.h | 4 +++- SDWebImage/UIView+WebCacheOperation.m | 6 +++--- 4 files changed, 9 insertions(+), 15 deletions(-) diff --git a/SDWebImage/UIButton+WebCache.m b/SDWebImage/UIButton+WebCache.m index 7c861e81..11e34b18 100644 --- a/SDWebImage/UIButton+WebCache.m +++ b/SDWebImage/UIButton+WebCache.m @@ -146,18 +146,10 @@ static inline NSString * backgroundImageURLKeyForState(UIControlState state) { completed:completedBlock]; } -- (void)sd_setImageLoadOperation:(id)operation forState:(UIControlState)state { - [self sd_setImageLoadOperation:operation forKey:[NSString stringWithFormat:@"UIButtonImageOperation%@", @(state)]]; -} - - (void)sd_cancelImageLoadForState:(UIControlState)state { [self sd_cancelImageLoadOperationWithKey:[NSString stringWithFormat:@"UIButtonImageOperation%@", @(state)]]; } -- (void)sd_setBackgroundImageLoadOperation:(id)operation forState:(UIControlState)state { - [self sd_setImageLoadOperation:operation forKey:[NSString stringWithFormat:@"UIButtonBackgroundImageOperation%@", @(state)]]; -} - - (void)sd_cancelBackgroundImageLoadForState:(UIControlState)state { [self sd_cancelImageLoadOperationWithKey:[NSString stringWithFormat:@"UIButtonBackgroundImageOperation%@", @(state)]]; } diff --git a/SDWebImage/UIImageView+WebCache.m b/SDWebImage/UIImageView+WebCache.m index 8832f2e4..be289db8 100644 --- a/SDWebImage/UIImageView+WebCache.m +++ b/SDWebImage/UIImageView+WebCache.m @@ -109,18 +109,18 @@ }]; } -static char animationloadOperationKey; +static char animationLoadOperationKey; // element is weak because operation instance is retained by SDWebImageManager's runningOperations property // we should use lock to keep thread-safe because these method may not be acessed from main queue - (NSPointerArray *)sd_animationOperationArray { @synchronized(self) { - NSPointerArray *operationsArray = objc_getAssociatedObject(self, &animationloadOperationKey); + NSPointerArray *operationsArray = objc_getAssociatedObject(self, &animationLoadOperationKey); if (operationsArray) { return operationsArray; } operationsArray = [NSPointerArray weakObjectsPointerArray]; - objc_setAssociatedObject(self, &animationloadOperationKey, operationsArray, OBJC_ASSOCIATION_RETAIN_NONATOMIC); + objc_setAssociatedObject(self, &animationLoadOperationKey, operationsArray, OBJC_ASSOCIATION_RETAIN_NONATOMIC); return operationsArray; } } diff --git a/SDWebImage/UIView+WebCacheOperation.h b/SDWebImage/UIView+WebCacheOperation.h index bef703eb..5d44691f 100644 --- a/SDWebImage/UIView+WebCacheOperation.h +++ b/SDWebImage/UIView+WebCacheOperation.h @@ -12,10 +12,12 @@ #import "SDWebImageManager.h" +// These methods are used to support canceling for UIView image loading, it's designed to be used internal but not external. +// All the stored operations are weak, so it will be dalloced after image loading finished. If you need to store operations, use your own class to keep a strong reference for them. @interface UIView (WebCacheOperation) /** - * Set the image load operation (storage in a UIView based dictionary) + * Set the image load operation (storage in a UIView based weak map table) * * @param operation the operation * @param key key for storing the operation diff --git a/SDWebImage/UIView+WebCacheOperation.m b/SDWebImage/UIView+WebCacheOperation.m index bcdaa3d1..78d58f41 100644 --- a/SDWebImage/UIView+WebCacheOperation.m +++ b/SDWebImage/UIView+WebCacheOperation.m @@ -16,7 +16,7 @@ static char loadOperationKey; // key is copy, value is weak because operation instance is retained by SDWebImageManager's runningOperations property // we should use lock to keep thread-safe because these method may not be acessed from main queue -typedef NSMapTable SDOperationsDictionary; +typedef NSMapTable> SDOperationsDictionary; @implementation UIView (WebCacheOperation) @@ -47,13 +47,13 @@ typedef NSMapTable SDOperationsDictionary; - (void)sd_cancelImageLoadOperationWithKey:(nullable NSString *)key { // Cancel in progress downloader from queue SDOperationsDictionary *operationDictionary = [self sd_operationDictionary]; - id operation; + id operation; @synchronized (self) { operation = [operationDictionary objectForKey:key]; } if (operation) { if ([operation conformsToProtocol:@protocol(SDWebImageOperation)]){ - [(id) operation cancel]; + [operation cancel]; } @synchronized (self) { [operationDictionary removeObjectForKey:key]; From ac5ec6997cb67e9ec067a9a4fe73a6308a68c6b6 Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Tue, 19 Dec 2017 13:11:51 +0800 Subject: [PATCH 4/4] Fix the way remove all elements from pointer array --- SDWebImage/UIImageView+WebCache.m | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/SDWebImage/UIImageView+WebCache.m b/SDWebImage/UIImageView+WebCache.m index be289db8..b25a234f 100644 --- a/SDWebImage/UIImageView+WebCache.m +++ b/SDWebImage/UIImageView+WebCache.m @@ -134,9 +134,7 @@ static char animationLoadOperationKey; [operation cancel]; } } - for (size_t i = 0; i < operationsArray.count; i++) { - [operationsArray removePointerAtIndex:i]; - } + operationsArray.count = 0; } } }