Authored by Bogdan Poplauschi
Committed by GitHub

Merge pull request #2048 from dreampiggy/fix_cancelBlock_multi_thread_issue

Fix #1941 by placing a lock to avoid multi-thread issue for SDWebImageCombinedOperation cancelBlock
@@ -228,11 +228,14 @@ @@ -228,11 +228,14 @@
228 [self safelyRemoveOperationFromRunning:strongOperation]; 228 [self safelyRemoveOperationFromRunning:strongOperation];
229 } 229 }
230 }]; 230 }];
231 - operation.cancelBlock = ^{  
232 - [self.imageDownloader cancel:subOperationToken];  
233 - __strong __typeof(weakOperation) strongOperation = weakOperation;  
234 - [self safelyRemoveOperationFromRunning:strongOperation];  
235 - }; 231 + @synchronized(operation) {
  232 + // Need same lock to ensure cancelBlock called because cancel method can be called in different queue
  233 + operation.cancelBlock = ^{
  234 + [self.imageDownloader cancel:subOperationToken];
  235 + __strong __typeof(weakOperation) strongOperation = weakOperation;
  236 + [self safelyRemoveOperationFromRunning:strongOperation];
  237 + };
  238 + }
236 } else if (cachedImage) { 239 } else if (cachedImage) {
237 __strong __typeof(weakOperation) strongOperation = weakOperation; 240 __strong __typeof(weakOperation) strongOperation = weakOperation;
238 [self callCompletionBlockForOperation:strongOperation completion:completedBlock image:cachedImage data:cachedData error:nil cacheType:cacheType finished:YES url:url]; 241 [self callCompletionBlockForOperation:strongOperation completion:completedBlock image:cachedImage data:cachedData error:nil cacheType:cacheType finished:YES url:url];
@@ -319,18 +322,16 @@ @@ -319,18 +322,16 @@
319 } 322 }
320 323
321 - (void)cancel { 324 - (void)cancel {
322 - self.cancelled = YES;  
323 - if (self.cacheOperation) {  
324 - [self.cacheOperation cancel];  
325 - self.cacheOperation = nil;  
326 - }  
327 - if (self.cancelBlock) {  
328 - self.cancelBlock();  
329 -  
330 - // TODO: this is a temporary fix to #809.  
331 - // Until we can figure the exact cause of the crash, going with the ivar instead of the setter  
332 -// self.cancelBlock = nil;  
333 - _cancelBlock = nil; 325 + @synchronized(self) {
  326 + self.cancelled = YES;
  327 + if (self.cacheOperation) {
  328 + [self.cacheOperation cancel];
  329 + self.cacheOperation = nil;
  330 + }
  331 + if (self.cancelBlock) {
  332 + self.cancelBlock();
  333 + self.cancelBlock = nil;
  334 + }
334 } 335 }
335 } 336 }
336 337