Skip to content

Commit e64aff8

Browse files
zhongwuzwfacebook-github-bot
authored andcommitted
Add lock guard for getter and setter of cancelLoad block when load images (facebook#23696)
Summary: `cancelLoad` block has race condition, let's add a lock to protect it. [iOS] [Fixed] - Add lock guard for getter and setter of cancelLoad block when load images Pull Request resolved: facebook#23696 Differential Revision: D14275444 Pulled By: cpojer fbshipit-source-id: aea6c05f5d5863fd9c31fda5a94f2045d97e0ff7
1 parent 7ad2f43 commit e64aff8

File tree

1 file changed

+40
-23
lines changed

1 file changed

+40
-23
lines changed

Libraries/Image/RCTImageLoader.m

Lines changed: 40 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -373,10 +373,12 @@ - (RCTImageLoaderCancellationBlock)_loadImageOrDataWithURLRequest:(NSURLRequest
373373
[loadHandler shouldCacheLoadedImages] : YES;
374374

375375
__block atomic_bool cancelled = ATOMIC_VAR_INIT(NO);
376-
// TODO: Protect this variable shared between threads.
377376
__block dispatch_block_t cancelLoad = nil;
377+
__block NSLock *cancelLoadLock = [NSLock new];
378378
void (^completionHandler)(NSError *, id, NSURLResponse *) = ^(NSError *error, id imageOrData, NSURLResponse *response) {
379+
[cancelLoadLock lock];
379380
cancelLoad = nil;
381+
[cancelLoadLock unlock];
380382

381383
// If we've received an image, we should try to set it synchronously,
382384
// if it's data, do decoding on a background thread.
@@ -420,15 +422,18 @@ - (RCTImageLoaderCancellationBlock)_loadImageOrDataWithURLRequest:(NSURLRequest
420422
}
421423

422424
if (loadHandler) {
423-
cancelLoad = [loadHandler loadImageForURL:request.URL
424-
size:size
425-
scale:scale
426-
resizeMode:resizeMode
427-
progressHandler:progressHandler
428-
partialLoadHandler:partialLoadHandler
429-
completionHandler:^(NSError *error, UIImage *image) {
430-
completionHandler(error, image, nil);
431-
}];
425+
dispatch_block_t cancelLoadLocal = [loadHandler loadImageForURL:request.URL
426+
size:size
427+
scale:scale
428+
resizeMode:resizeMode
429+
progressHandler:progressHandler
430+
partialLoadHandler:partialLoadHandler
431+
completionHandler:^(NSError *error, UIImage *image) {
432+
completionHandler(error, image, nil);
433+
}];
434+
[cancelLoadLock lock];
435+
cancelLoad = cancelLoadLocal;
436+
[cancelLoadLock unlock];
432437
} else {
433438
UIImage *image;
434439
if (cacheResult) {
@@ -442,9 +447,12 @@ - (RCTImageLoaderCancellationBlock)_loadImageOrDataWithURLRequest:(NSURLRequest
442447
completionHandler(nil, image, nil);
443448
} else {
444449
// Use networking module to load image
445-
cancelLoad = [strongSelf _loadURLRequest:request
446-
progressBlock:progressHandler
447-
completionBlock:completionHandler];
450+
dispatch_block_t cancelLoadLocal = [strongSelf _loadURLRequest:request
451+
progressBlock:progressHandler
452+
completionBlock:completionHandler];
453+
[cancelLoadLock lock];
454+
cancelLoad = cancelLoadLocal;
455+
[cancelLoadLock unlock];
448456
}
449457
}
450458
});
@@ -454,8 +462,10 @@ - (RCTImageLoaderCancellationBlock)_loadImageOrDataWithURLRequest:(NSURLRequest
454462
if (alreadyCancelled) {
455463
return;
456464
}
465+
[cancelLoadLock lock];
457466
dispatch_block_t cancelLoadLocal = cancelLoad;
458467
cancelLoad = nil;
468+
[cancelLoadLock unlock];
459469
if (cancelLoadLocal) {
460470
cancelLoadLocal();
461471
}
@@ -583,15 +593,17 @@ - (RCTImageLoaderCancellationBlock)loadImageWithURLRequest:(NSURLRequest *)image
583593
completionBlock:(RCTImageLoaderCompletionBlock)completionBlock
584594
{
585595
__block atomic_bool cancelled = ATOMIC_VAR_INIT(NO);
586-
// TODO: Protect this variable shared between threads.
587596
__block dispatch_block_t cancelLoad = nil;
597+
__block NSLock *cancelLoadLock = [NSLock new];
588598
dispatch_block_t cancellationBlock = ^{
589599
BOOL alreadyCancelled = atomic_fetch_or(&cancelled, 1);
590600
if (alreadyCancelled) {
591601
return;
592602
}
603+
[cancelLoadLock lock];
593604
dispatch_block_t cancelLoadLocal = cancelLoad;
594605
cancelLoad = nil;
606+
[cancelLoadLock unlock];
595607
if (cancelLoadLocal) {
596608
cancelLoadLocal();
597609
}
@@ -605,7 +617,9 @@ - (RCTImageLoaderCancellationBlock)loadImageWithURLRequest:(NSURLRequest *)image
605617
}
606618

607619
if (!imageOrData || [imageOrData isKindOfClass:[UIImage class]]) {
620+
[cancelLoadLock lock];
608621
cancelLoad = nil;
622+
[cancelLoadLock unlock];
609623
completionBlock(error, imageOrData);
610624
return;
611625
}
@@ -620,19 +634,22 @@ - (RCTImageLoaderCancellationBlock)loadImageWithURLRequest:(NSURLRequest *)image
620634
resizeMode:resizeMode
621635
response:response];
622636
}
623-
637+
[cancelLoadLock lock];
624638
cancelLoad = nil;
639+
[cancelLoadLock unlock];
625640
completionBlock(error_, image);
626641
};
627-
628-
cancelLoad = [strongSelf decodeImageData:imageOrData
629-
size:size
630-
scale:scale
631-
clipped:clipped
632-
resizeMode:resizeMode
633-
completionBlock:decodeCompletionHandler];
642+
dispatch_block_t cancelLoadLocal = [strongSelf decodeImageData:imageOrData
643+
size:size
644+
scale:scale
645+
clipped:clipped
646+
resizeMode:resizeMode
647+
completionBlock:decodeCompletionHandler];
648+
[cancelLoadLock lock];
649+
cancelLoad = cancelLoadLocal;
650+
[cancelLoadLock unlock];
634651
};
635-
652+
636653
cancelLoad = [self _loadImageOrDataWithURLRequest:imageURLRequest
637654
size:size
638655
scale:scale

0 commit comments

Comments
 (0)