Authored by DreamPiggy
Committed by GitHub

Merge pull request #2184 from dreampiggy/fix_downloader_blocking_main_queue

Use a lock instead of barrier queue to avoid dispatch_sync blocking the main queue on race condition
@@ -36,8 +36,6 @@ @@ -36,8 +36,6 @@
36 @property (assign, nonatomic, nullable) Class operationClass; 36 @property (assign, nonatomic, nullable) Class operationClass;
37 @property (strong, nonatomic, nonnull) NSMutableDictionary<NSURL *, SDWebImageDownloaderOperation *> *URLOperations; 37 @property (strong, nonatomic, nonnull) NSMutableDictionary<NSURL *, SDWebImageDownloaderOperation *> *URLOperations;
38 @property (strong, nonatomic, nullable) SDHTTPHeadersMutableDictionary *HTTPHeaders; 38 @property (strong, nonatomic, nullable) SDHTTPHeadersMutableDictionary *HTTPHeaders;
39 -// This queue is used to serialize the handling of the network responses of all the download operation in a single queue  
40 -@property (strong, nonatomic, nullable) dispatch_queue_t barrierQueue;  
41 39
42 // The session in which data tasks will run 40 // The session in which data tasks will run
43 @property (strong, nonatomic) NSURLSession *session; 41 @property (strong, nonatomic) NSURLSession *session;
@@ -96,7 +94,6 @@ @@ -96,7 +94,6 @@
96 #else 94 #else
97 _HTTPHeaders = [@{@"Accept": @"image/*;q=0.8"} mutableCopy]; 95 _HTTPHeaders = [@{@"Accept": @"image/*;q=0.8"} mutableCopy];
98 #endif 96 #endif
99 - _barrierQueue = dispatch_queue_create("com.hackemist.SDWebImageDownloaderBarrierQueue", DISPATCH_QUEUE_CONCURRENT);  
100 _downloadTimeout = 15.0; 97 _downloadTimeout = 15.0;
101 98
102 [self createNewSessionWithConfiguration:sessionConfiguration]; 99 [self createNewSessionWithConfiguration:sessionConfiguration];
@@ -231,13 +228,15 @@ @@ -231,13 +228,15 @@
231 } 228 }
232 229
233 - (void)cancel:(nullable SDWebImageDownloadToken *)token { 230 - (void)cancel:(nullable SDWebImageDownloadToken *)token {
234 - dispatch_barrier_async(self.barrierQueue, ^{  
235 - SDWebImageDownloaderOperation *operation = self.URLOperations[token.url];  
236 - BOOL canceled = [operation cancel:token.downloadOperationCancelToken];  
237 - if (canceled) {  
238 - [self.URLOperations removeObjectForKey:token.url];  
239 - }  
240 - }); 231 + NSURL *url = token.url;
  232 + if (!url) {
  233 + return;
  234 + }
  235 + SDWebImageDownloaderOperation *operation = [self operationForURL:url];
  236 + BOOL canceled = [operation cancel:token.downloadOperationCancelToken];
  237 + if (canceled) {
  238 + [self removeOperationForURL:url];
  239 + }
241 } 240 }
242 241
243 - (nullable SDWebImageDownloadToken *)addProgressCallback:(SDWebImageDownloaderProgressBlock)progressBlock 242 - (nullable SDWebImageDownloadToken *)addProgressCallback:(SDWebImageDownloaderProgressBlock)progressBlock
@@ -251,33 +250,24 @@ @@ -251,33 +250,24 @@
251 } 250 }
252 return nil; 251 return nil;
253 } 252 }
254 -  
255 - __block SDWebImageDownloadToken *token = nil;  
256 -  
257 - dispatch_barrier_sync(self.barrierQueue, ^{  
258 - SDWebImageDownloaderOperation *operation = self.URLOperations[url];  
259 - if (!operation) {  
260 - operation = createCallback();  
261 - self.URLOperations[url] = operation;  
262 -  
263 - __weak SDWebImageDownloaderOperation *woperation = operation;  
264 - operation.completionBlock = ^{  
265 - dispatch_barrier_sync(self.barrierQueue, ^{  
266 - SDWebImageDownloaderOperation *soperation = woperation;  
267 - if (!soperation) return;  
268 - if (self.URLOperations[url] == soperation) {  
269 - [self.URLOperations removeObjectForKey:url];  
270 - };  
271 - });  
272 - };  
273 - }  
274 - id downloadOperationCancelToken = [operation addHandlersForProgress:progressBlock completed:completedBlock];  
275 -  
276 - token = [SDWebImageDownloadToken new];  
277 - token.downloadOperation = operation;  
278 - token.url = url;  
279 - token.downloadOperationCancelToken = downloadOperationCancelToken;  
280 - }); 253 +
  254 + SDWebImageDownloaderOperation *operation = [self operationForURL:url];
  255 + if (!operation) {
  256 + operation = createCallback();
  257 + [self setOperation:operation forURL:url];
  258 +
  259 + __weak typeof(self) wself = self;
  260 + operation.completionBlock = ^{
  261 + __strong typeof(wself) sself = wself;
  262 + [sself removeOperationForURL:url];
  263 + };
  264 + }
  265 + id downloadOperationCancelToken = [operation addHandlersForProgress:progressBlock completed:completedBlock];
  266 +
  267 + SDWebImageDownloadToken *token = [SDWebImageDownloadToken new];
  268 + token.downloadOperation = operation;
  269 + token.url = url;
  270 + token.downloadOperationCancelToken = downloadOperationCancelToken;
281 271
282 return token; 272 return token;
283 } 273 }
@@ -292,6 +282,35 @@ @@ -292,6 +282,35 @@
292 282
293 #pragma mark Helper methods 283 #pragma mark Helper methods
294 284
  285 +- (SDWebImageDownloaderOperation *)operationForURL:(NSURL *)url {
  286 + if (!url) {
  287 + return nil;
  288 + }
  289 + SDWebImageDownloaderOperation *operation;
  290 + @synchronized (self.URLOperations) {
  291 + operation = [self.URLOperations objectForKey:url];
  292 + }
  293 + return operation;
  294 +}
  295 +
  296 +- (void)setOperation:(SDWebImageDownloaderOperation *)operation forURL:(NSURL *)url {
  297 + if (!operation || !url) {
  298 + return;
  299 + }
  300 + @synchronized (self.URLOperations) {
  301 + [self.URLOperations setObject:operation forKey:url];
  302 + }
  303 +}
  304 +
  305 +- (void)removeOperationForURL:(NSURL *)url {
  306 + if (!url) {
  307 + return;
  308 + }
  309 + @synchronized (self.URLOperations) {
  310 + [self.URLOperations removeObjectForKey:url];
  311 + }
  312 +}
  313 +
295 - (SDWebImageDownloaderOperation *)operationWithTask:(NSURLSessionTask *)task { 314 - (SDWebImageDownloaderOperation *)operationWithTask:(NSURLSessionTask *)task {
296 SDWebImageDownloaderOperation *returnOperation = nil; 315 SDWebImageDownloaderOperation *returnOperation = nil;
297 for (SDWebImageDownloaderOperation *operation in self.downloadQueue.operations) { 316 for (SDWebImageDownloaderOperation *operation in self.downloadQueue.operations) {