From 570965f6cff9346a370039266f8a45d8fe15b950 Mon Sep 17 00:00:00 2001 From: Olivier Poitrey Date: Tue, 6 Nov 2012 03:31:03 +0100 Subject: [PATCH] Use dispatch_barrier to handle NSMutableDictionary thread unsafety instead of main thread dispatching --- SDWebImage/SDWebImageDownloader.m | 130 +++++++++++++++++------------- 1 file changed, 73 insertions(+), 57 deletions(-) diff --git a/SDWebImage/SDWebImageDownloader.m b/SDWebImage/SDWebImageDownloader.m index f08cff89..767d8852 100644 --- a/SDWebImage/SDWebImageDownloader.m +++ b/SDWebImage/SDWebImageDownloader.m @@ -20,6 +20,7 @@ NSString *const kCompletedCallbackKey = @"completed"; @property (strong, nonatomic) NSOperationQueue *downloadQueue; @property (strong, nonatomic) NSMutableDictionary *URLCallbacks; +@property (assign, nonatomic) dispatch_queue_t barrierQueue; @end @@ -65,6 +66,7 @@ NSString *const kCompletedCallbackKey = @"completed"; _downloadQueue = NSOperationQueue.new; _downloadQueue.maxConcurrentOperationCount = 10; _URLCallbacks = NSMutableDictionary.new; + _barrierQueue = dispatch_queue_create("com.hackemist.SDWebImageDownloaderBarrierQueue", DISPATCH_QUEUE_CONCURRENT); } return self; } @@ -82,71 +84,85 @@ NSString *const kCompletedCallbackKey = @"completed"; - (NSOperation *)downloadImageWithURL:(NSURL *)url options:(SDWebImageDownloaderOptions)options progress:(void (^)(NSUInteger, long long))progressBlock completed:(void (^)(UIImage *, NSError *, BOOL))completedBlock { __block SDWebImageDownloaderOperation *operation; + __weak SDWebImageDownloader *wself = self; - dispatch_async(dispatch_get_main_queue(), ^ // NSDictionary isn't thread safe + [self addProgressCallback:progressBlock andCompletedBlock:completedBlock forURL:url createCallback:^ { - BOOL performDownload = NO; - - if (!self.URLCallbacks[url]) + // In order to prevent from potential duplicate caching (NSURLCache + SDImageCache) we disable the cache for image requests + NSMutableURLRequest *request = [NSMutableURLRequest.alloc initWithURL:url cachePolicy:NSURLCacheStorageNotAllowed timeoutInterval:15]; + request.HTTPShouldHandleCookies = NO; + request.HTTPShouldUsePipelining = YES; + [request addValue:@"image/*" forHTTPHeaderField:@"Accept"]; + operation = [SDWebImageDownloaderOperation.alloc initWithRequest:request options:options progress:^(NSUInteger receivedSize, long long expectedSize) { - self.URLCallbacks[url] = NSMutableArray.new; - performDownload = YES; - } - - // Handle single download of simultaneous download request for the same URL - { - NSMutableArray *callbacksForURL = self.URLCallbacks[url]; - NSMutableDictionary *callbacks = NSMutableDictionary.new; - if (progressBlock) callbacks[kProgressCallbackKey] = progressBlock; - if (completedBlock) callbacks[kCompletedCallbackKey] = completedBlock; - [callbacksForURL addObject:callbacks]; - self.URLCallbacks[url] = callbacksForURL; - } - - if (performDownload) - { - // In order to prevent from potential duplicate caching (NSURLCache + SDImageCache) we disable the cache for image requests - NSMutableURLRequest *request = [NSMutableURLRequest.alloc initWithURL:url cachePolicy:NSURLCacheStorageNotAllowed timeoutInterval:15]; - request.HTTPShouldHandleCookies = NO; - request.HTTPShouldUsePipelining = YES; - [request addValue:@"image/*" forHTTPHeaderField:@"Accept"]; - operation = [SDWebImageDownloaderOperation.alloc initWithRequest:request options:options progress:^(NSUInteger receivedSize, long long expectedSize) + if (!wself) return; + SDWebImageDownloader *sself = wself; + NSArray *callbacksForURL = [sself removeAndReturnCallbacksForURL:url]; + for (NSDictionary *callbacks in callbacksForURL) { - dispatch_async(dispatch_get_main_queue(), ^ - { - NSMutableArray *callbacksForURL = self.URLCallbacks[url]; - for (NSDictionary *callbacks in callbacksForURL) - { - SDWebImageDownloaderProgressBlock callback = callbacks[kProgressCallbackKey]; - if (callback) callback(receivedSize, expectedSize); - } - }); + SDWebImageDownloaderProgressBlock callback = callbacks[kProgressCallbackKey]; + if (callback) callback(receivedSize, expectedSize); } - completed:^(UIImage *image, NSError *error, BOOL finished) - { - dispatch_async(dispatch_get_main_queue(), ^ - { - NSMutableArray *callbacksForURL = self.URLCallbacks[url]; - [self.URLCallbacks removeObjectForKey:url]; - for (NSDictionary *callbacks in callbacksForURL) - { - SDWebImageDownloaderCompletedBlock callback = callbacks[kCompletedCallbackKey]; - if (callback) callback(image, error, finished); - } - }); - } - cancelled:^ - { - dispatch_async(dispatch_get_main_queue(), ^ - { - [self.URLCallbacks removeObjectForKey:url]; - }); - }]; - [self.downloadQueue addOperation:operation]; } - }); + completed:^(UIImage *image, NSError *error, BOOL finished) + { + if (!wself) return; + SDWebImageDownloader *sself = wself; + NSArray *callbacksForURL = [sself removeAndReturnCallbacksForURL:url]; + for (NSDictionary *callbacks in callbacksForURL) + { + SDWebImageDownloaderCompletedBlock callback = callbacks[kCompletedCallbackKey]; + if (callback) callback(image, error, finished); + } + } + cancelled:^ + { + if (!wself) return; + SDWebImageDownloader *sself = wself; + [sself removeAndReturnCallbacksForURL:url]; + }]; + [self.downloadQueue addOperation:operation]; + }]; + return operation; } +- (void)addProgressCallback:(void (^)(NSUInteger, long long))progressBlock andCompletedBlock:(void (^)(UIImage *, NSError *, BOOL))completedBlock forURL:(NSURL *)url createCallback:(void (^)())createCallback +{ + dispatch_barrier_async(self.barrierQueue, ^ + { + BOOL first = NO; + if (!self.URLCallbacks[url]) + { + self.URLCallbacks[url] = NSMutableArray.new; + first = YES; + } + + // Handle single download of simultaneous download request for the same URL + NSMutableArray *callbacksForURL = self.URLCallbacks[url]; + NSMutableDictionary *callbacks = NSMutableDictionary.new; + if (progressBlock) callbacks[kProgressCallbackKey] = progressBlock; + if (completedBlock) callbacks[kCompletedCallbackKey] = completedBlock; + [callbacksForURL addObject:callbacks]; + self.URLCallbacks[url] = callbacksForURL; + + if (first) + { + createCallback(); + } + }); +} + +- (NSArray *)removeAndReturnCallbacksForURL:(NSURL *)url +{ + __block NSArray *callbacksForURL; + dispatch_barrier_sync(self.barrierQueue, ^ + { + callbacksForURL = self.URLCallbacks[url]; + [self.URLCallbacks removeObjectForKey:url]; + }); + return callbacksForURL; +} + @end