Skip to content

[iOS] Add lock guard for getter and setter of cancelLoad block when load images #23696

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
zhongwuzw wants to merge 2 commits into from
Closed

[iOS] Add lock guard for getter and setter of cancelLoad block when load images #23696

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0ea5b11
[iOS] Add lock guard for getter and setter of cancelLoad block when l…
zhongwuzw Feb 28, 2019
9543801
Add lock for image load
zhongwuzw Feb 28, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 40 additions & 23 deletions Libraries/Image/RCTImageLoader.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,10 +373,12 @@ - (RCTImageLoaderCancellationBlock)_loadImageOrDataWithURLRequest:(NSURLRequest
[loadHandler shouldCacheLoadedImages] : YES;

__block atomic_bool cancelled = ATOMIC_VAR_INIT(NO);
// TODO: Protect this variable shared between threads.
__block dispatch_block_t cancelLoad = nil;
__block NSLock *cancelLoadLock = [NSLock new];
void (^completionHandler)(NSError *, id, NSURLResponse *) = ^(NSError *error, id imageOrData, NSURLResponse *response) {
[cancelLoadLock lock];
cancelLoad = nil;
[cancelLoadLock unlock];

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

if (loadHandler) {
cancelLoad = [loadHandler loadImageForURL:request.URL
size:size
scale:scale
resizeMode:resizeMode
progressHandler:progressHandler
partialLoadHandler:partialLoadHandler
completionHandler:^(NSError *error, UIImage *image) {
completionHandler(error, image, nil);
}];
dispatch_block_t cancelLoadLocal = [loadHandler loadImageForURL:request.URL
size:size
scale:scale
resizeMode:resizeMode
progressHandler:progressHandler
partialLoadHandler:partialLoadHandler
completionHandler:^(NSError *error, UIImage *image) {
completionHandler(error, image, nil);
}];
[cancelLoadLock lock];
cancelLoad = cancelLoadLocal;
[cancelLoadLock unlock];
} else {
UIImage *image;
if (cacheResult) {
Expand All @@ -442,9 +447,12 @@ - (RCTImageLoaderCancellationBlock)_loadImageOrDataWithURLRequest:(NSURLRequest
completionHandler(nil, image, nil);
} else {
// Use networking module to load image
cancelLoad = [strongSelf _loadURLRequest:request
progressBlock:progressHandler
completionBlock:completionHandler];
dispatch_block_t cancelLoadLocal = [strongSelf _loadURLRequest:request
progressBlock:progressHandler
completionBlock:completionHandler];
[cancelLoadLock lock];
cancelLoad = cancelLoadLocal;
[cancelLoadLock unlock];
}
}
});
Expand All @@ -454,8 +462,10 @@ - (RCTImageLoaderCancellationBlock)_loadImageOrDataWithURLRequest:(NSURLRequest
if (alreadyCancelled) {
return;
}
[cancelLoadLock lock];
dispatch_block_t cancelLoadLocal = cancelLoad;
cancelLoad = nil;
[cancelLoadLock unlock];
if (cancelLoadLocal) {
cancelLoadLocal();
}
Expand Down Expand Up @@ -583,15 +593,17 @@ - (RCTImageLoaderCancellationBlock)loadImageWithURLRequest:(NSURLRequest *)image
completionBlock:(RCTImageLoaderCompletionBlock)completionBlock
{
__block atomic_bool cancelled = ATOMIC_VAR_INIT(NO);
// TODO: Protect this variable shared between threads.
__block dispatch_block_t cancelLoad = nil;
__block NSLock *cancelLoadLock = [NSLock new];
dispatch_block_t cancellationBlock = ^{
BOOL alreadyCancelled = atomic_fetch_or(&cancelled, 1);
if (alreadyCancelled) {
return;
}
[cancelLoadLock lock];
dispatch_block_t cancelLoadLocal = cancelLoad;
cancelLoad = nil;
[cancelLoadLock unlock];
if (cancelLoadLocal) {
cancelLoadLocal();
}
Expand All @@ -605,7 +617,9 @@ - (RCTImageLoaderCancellationBlock)loadImageWithURLRequest:(NSURLRequest *)image
}

if (!imageOrData || [imageOrData isKindOfClass:[UIImage class]]) {
[cancelLoadLock lock];
cancelLoad = nil;
[cancelLoadLock unlock];
completionBlock(error, imageOrData);
return;
}
Expand All @@ -620,19 +634,22 @@ - (RCTImageLoaderCancellationBlock)loadImageWithURLRequest:(NSURLRequest *)image
resizeMode:resizeMode
response:response];
}

[cancelLoadLock lock];
cancelLoad = nil;
[cancelLoadLock unlock];
completionBlock(error_, image);
};

cancelLoad = [strongSelf decodeImageData:imageOrData
size:size
scale:scale
clipped:clipped
resizeMode:resizeMode
completionBlock:decodeCompletionHandler];
dispatch_block_t cancelLoadLocal = [strongSelf decodeImageData:imageOrData
size:size
scale:scale
clipped:clipped
resizeMode:resizeMode
completionBlock:decodeCompletionHandler];
[cancelLoadLock lock];
cancelLoad = cancelLoadLocal;
[cancelLoadLock unlock];
};

cancelLoad = [self _loadImageOrDataWithURLRequest:imageURLRequest
size:size
scale:scale
Expand Down