Authored by Olivier Poitrey

Use dispatch_barrier to handle NSMutableDictionary thread unsafety instead of ma…

…in thread dispatching
@@ -20,6 +20,7 @@ NSString *const kCompletedCallbackKey = @"completed"; @@ -20,6 +20,7 @@ NSString *const kCompletedCallbackKey = @"completed";
20 20
21 @property (strong, nonatomic) NSOperationQueue *downloadQueue; 21 @property (strong, nonatomic) NSOperationQueue *downloadQueue;
22 @property (strong, nonatomic) NSMutableDictionary *URLCallbacks; 22 @property (strong, nonatomic) NSMutableDictionary *URLCallbacks;
  23 +@property (assign, nonatomic) dispatch_queue_t barrierQueue;
23 24
24 @end 25 @end
25 26
@@ -65,6 +66,7 @@ NSString *const kCompletedCallbackKey = @"completed"; @@ -65,6 +66,7 @@ NSString *const kCompletedCallbackKey = @"completed";
65 _downloadQueue = NSOperationQueue.new; 66 _downloadQueue = NSOperationQueue.new;
66 _downloadQueue.maxConcurrentOperationCount = 10; 67 _downloadQueue.maxConcurrentOperationCount = 10;
67 _URLCallbacks = NSMutableDictionary.new; 68 _URLCallbacks = NSMutableDictionary.new;
  69 + _barrierQueue = dispatch_queue_create("com.hackemist.SDWebImageDownloaderBarrierQueue", DISPATCH_QUEUE_CONCURRENT);
68 } 70 }
69 return self; 71 return self;
70 } 72 }
@@ -82,28 +84,9 @@ NSString *const kCompletedCallbackKey = @"completed"; @@ -82,28 +84,9 @@ NSString *const kCompletedCallbackKey = @"completed";
82 - (NSOperation *)downloadImageWithURL:(NSURL *)url options:(SDWebImageDownloaderOptions)options progress:(void (^)(NSUInteger, long long))progressBlock completed:(void (^)(UIImage *, NSError *, BOOL))completedBlock 84 - (NSOperation *)downloadImageWithURL:(NSURL *)url options:(SDWebImageDownloaderOptions)options progress:(void (^)(NSUInteger, long long))progressBlock completed:(void (^)(UIImage *, NSError *, BOOL))completedBlock
83 { 85 {
84 __block SDWebImageDownloaderOperation *operation; 86 __block SDWebImageDownloaderOperation *operation;
  87 + __weak SDWebImageDownloader *wself = self;
85 88
86 - dispatch_async(dispatch_get_main_queue(), ^ // NSDictionary isn't thread safe  
87 - {  
88 - BOOL performDownload = NO;  
89 -  
90 - if (!self.URLCallbacks[url])  
91 - {  
92 - self.URLCallbacks[url] = NSMutableArray.new;  
93 - performDownload = YES;  
94 - }  
95 -  
96 - // Handle single download of simultaneous download request for the same URL  
97 - {  
98 - NSMutableArray *callbacksForURL = self.URLCallbacks[url];  
99 - NSMutableDictionary *callbacks = NSMutableDictionary.new;  
100 - if (progressBlock) callbacks[kProgressCallbackKey] = progressBlock;  
101 - if (completedBlock) callbacks[kCompletedCallbackKey] = completedBlock;  
102 - [callbacksForURL addObject:callbacks];  
103 - self.URLCallbacks[url] = callbacksForURL;  
104 - }  
105 -  
106 - if (performDownload) 89 + [self addProgressCallback:progressBlock andCompletedBlock:completedBlock forURL:url createCallback:^
107 { 90 {
108 // In order to prevent from potential duplicate caching (NSURLCache + SDImageCache) we disable the cache for image requests 91 // In order to prevent from potential duplicate caching (NSURLCache + SDImageCache) we disable the cache for image requests
109 NSMutableURLRequest *request = [NSMutableURLRequest.alloc initWithURL:url cachePolicy:NSURLCacheStorageNotAllowed timeoutInterval:15]; 92 NSMutableURLRequest *request = [NSMutableURLRequest.alloc initWithURL:url cachePolicy:NSURLCacheStorageNotAllowed timeoutInterval:15];
@@ -112,41 +95,74 @@ NSString *const kCompletedCallbackKey = @"completed"; @@ -112,41 +95,74 @@ NSString *const kCompletedCallbackKey = @"completed";
112 [request addValue:@"image/*" forHTTPHeaderField:@"Accept"]; 95 [request addValue:@"image/*" forHTTPHeaderField:@"Accept"];
113 operation = [SDWebImageDownloaderOperation.alloc initWithRequest:request options:options progress:^(NSUInteger receivedSize, long long expectedSize) 96 operation = [SDWebImageDownloaderOperation.alloc initWithRequest:request options:options progress:^(NSUInteger receivedSize, long long expectedSize)
114 { 97 {
115 - dispatch_async(dispatch_get_main_queue(), ^  
116 - {  
117 - NSMutableArray *callbacksForURL = self.URLCallbacks[url]; 98 + if (!wself) return;
  99 + SDWebImageDownloader *sself = wself;
  100 + NSArray *callbacksForURL = [sself removeAndReturnCallbacksForURL:url];
118 for (NSDictionary *callbacks in callbacksForURL) 101 for (NSDictionary *callbacks in callbacksForURL)
119 { 102 {
120 SDWebImageDownloaderProgressBlock callback = callbacks[kProgressCallbackKey]; 103 SDWebImageDownloaderProgressBlock callback = callbacks[kProgressCallbackKey];
121 if (callback) callback(receivedSize, expectedSize); 104 if (callback) callback(receivedSize, expectedSize);
122 } 105 }
123 - });  
124 } 106 }
125 completed:^(UIImage *image, NSError *error, BOOL finished) 107 completed:^(UIImage *image, NSError *error, BOOL finished)
126 { 108 {
127 - dispatch_async(dispatch_get_main_queue(), ^  
128 - {  
129 - NSMutableArray *callbacksForURL = self.URLCallbacks[url];  
130 - [self.URLCallbacks removeObjectForKey:url]; 109 + if (!wself) return;
  110 + SDWebImageDownloader *sself = wself;
  111 + NSArray *callbacksForURL = [sself removeAndReturnCallbacksForURL:url];
131 for (NSDictionary *callbacks in callbacksForURL) 112 for (NSDictionary *callbacks in callbacksForURL)
132 { 113 {
133 SDWebImageDownloaderCompletedBlock callback = callbacks[kCompletedCallbackKey]; 114 SDWebImageDownloaderCompletedBlock callback = callbacks[kCompletedCallbackKey];
134 if (callback) callback(image, error, finished); 115 if (callback) callback(image, error, finished);
135 } 116 }
136 - });  
137 } 117 }
138 cancelled:^ 118 cancelled:^
139 { 119 {
140 - dispatch_async(dispatch_get_main_queue(), ^  
141 - {  
142 - [self.URLCallbacks removeObjectForKey:url];  
143 - }); 120 + if (!wself) return;
  121 + SDWebImageDownloader *sself = wself;
  122 + [sself removeAndReturnCallbacksForURL:url];
144 }]; 123 }];
145 [self.downloadQueue addOperation:operation]; 124 [self.downloadQueue addOperation:operation];
  125 + }];
  126 +
  127 +
  128 + return operation;
  129 +}
  130 +
  131 +- (void)addProgressCallback:(void (^)(NSUInteger, long long))progressBlock andCompletedBlock:(void (^)(UIImage *, NSError *, BOOL))completedBlock forURL:(NSURL *)url createCallback:(void (^)())createCallback
  132 +{
  133 + dispatch_barrier_async(self.barrierQueue, ^
  134 + {
  135 + BOOL first = NO;
  136 + if (!self.URLCallbacks[url])
  137 + {
  138 + self.URLCallbacks[url] = NSMutableArray.new;
  139 + first = YES;
  140 + }
  141 +
  142 + // Handle single download of simultaneous download request for the same URL
  143 + NSMutableArray *callbacksForURL = self.URLCallbacks[url];
  144 + NSMutableDictionary *callbacks = NSMutableDictionary.new;
  145 + if (progressBlock) callbacks[kProgressCallbackKey] = progressBlock;
  146 + if (completedBlock) callbacks[kCompletedCallbackKey] = completedBlock;
  147 + [callbacksForURL addObject:callbacks];
  148 + self.URLCallbacks[url] = callbacksForURL;
  149 +
  150 + if (first)
  151 + {
  152 + createCallback();
146 } 153 }
147 }); 154 });
  155 +}
148 156
149 - return operation; 157 +- (NSArray *)removeAndReturnCallbacksForURL:(NSURL *)url
  158 +{
  159 + __block NSArray *callbacksForURL;
  160 + dispatch_barrier_sync(self.barrierQueue, ^
  161 + {
  162 + callbacksForURL = self.URLCallbacks[url];
  163 + [self.URLCallbacks removeObjectForKey:url];
  164 + });
  165 + return callbacksForURL;
150 } 166 }
151 167
152 @end 168 @end