mirrors
/
SDWebImage
mirror of https://github.com/SDWebImage/SDWebImage.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Change the lock into @synchronized to ensure the operation callback is always get attached correctly, avoid the callback is not get called

This commit is contained in:
DreamPiggy 2019-07-18 16:20:46 +08:00
parent f2fb26d1c6
commit a2b3a57bb2
2 changed files with 29 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
SDWebImage/SDWebImageDownloader.m
View File

@ -204,6 +204,7 @@ static void * SDWebImageDownloaderContext = &SDWebImageDownloaderContext;
}
SD_LOCK(self.operationsLock);
id downloadOperationCancelToken;
NSOperation<SDWebImageDownloaderOperation> *operation = [self.URLOperations objectForKey:url];
// There is a case that the operation may be marked as finished or cancelled, but not been removed from `self.URLOperations`.
if (!operation || operation.isFinished || operation.isCancelled) {
@ -230,20 +231,25 @@ static void * SDWebImageDownloaderContext = &SDWebImageDownloaderContext;
// Add operation to operation queue only after all configuration done according to Apple's doc.
// `addOperation:` does not synchronously execute the `operation.completionBlock` so this will not cause deadlock.
[self.downloadQueue addOperation:operation];
}
else if (!operation.isExecuting) {
if (options & SDWebImageDownloaderHighPriority) {
operation.queuePriority = NSOperationQueuePriorityHigh;
} else if (options & SDWebImageDownloaderLowPriority) {
operation.queuePriority = NSOperationQueuePriorityLow;
} else {
operation.queuePriority = NSOperationQueuePriorityNormal;
downloadOperationCancelToken = [operation addHandlersForProgress:progressBlock completed:completedBlock];
} else {
// When we reuse the download operation to attach more callbacks, there may be thread safe issue because the getter of callbacks may in another queue (decoding queue or delegate queue)
// So we lock the operation here, and in `SDWebImageDownloaderOperation`, we use `@synchonzied (self)`, to ensure the thread safe between these two classes.
@synchronized (operation) {
downloadOperationCancelToken = [operation addHandlersForProgress:progressBlock completed:completedBlock];
}
if (!operation.isExecuting) {
if (options & SDWebImageDownloaderHighPriority) {
operation.queuePriority = NSOperationQueuePriorityHigh;
} else if (options & SDWebImageDownloaderLowPriority) {
operation.queuePriority = NSOperationQueuePriorityLow;
} else {
operation.queuePriority = NSOperationQueuePriorityNormal;
}
}
}
SD_UNLOCK(self.operationsLock);
id downloadOperationCancelToken = [operation addHandlersForProgress:progressBlock completed:completedBlock];
SDWebImageDownloadToken *token = [[SDWebImageDownloadToken alloc] initWithDownloadOperation:operation];
token.url = url;
token.request = operation.request;

31
SDWebImage/SDWebImageDownloaderOperation.m
View File

@ -47,8 +47,6 @@ typedef NSMutableDictionary<NSString *, id> SDCallbacksDictionary;
@property (strong, nonatomic, readwrite, nullable) NSURLSessionTask *dataTask;
@property (strong, nonatomic, nonnull) dispatch_semaphore_t callbacksLock; // a lock to keep the access to `callbackBlocks` thread-safe
@property (strong, nonatomic, nonnull) dispatch_queue_t coderQueue; // the queue to do image decoding
#if SD_UIKIT
@property (assign, nonatomic) UIBackgroundTaskIdentifier backgroundTaskId;
@ -82,7 +80,6 @@ typedef NSMutableDictionary<NSString *, id> SDCallbacksDictionary;
_finished = NO;
_expectedSize = 0;
_unownedSession = session;
_callbacksLock = dispatch_semaphore_create(1);
_coderQueue = dispatch_queue_create("com.hackemist.SDWebImageDownloaderOperationCoderQueue", DISPATCH_QUEUE_SERIAL);
#if SD_UIKIT
_backgroundTaskId = UIBackgroundTaskInvalid;
@ -96,16 +93,17 @@ typedef NSMutableDictionary<NSString *, id> SDCallbacksDictionary;
SDCallbacksDictionary *callbacks = [NSMutableDictionary new];
if (progressBlock) callbacks[kProgressCallbackKey] = [progressBlock copy];
if (completedBlock) callbacks[kCompletedCallbackKey] = [completedBlock copy];
SD_LOCK(self.callbacksLock);
[self.callbackBlocks addObject:callbacks];
SD_UNLOCK(self.callbacksLock);
@synchronized (self) {
[self.callbackBlocks addObject:callbacks];
}
return callbacks;
}
- (nullable NSArray<id> *)callbacksForKey:(NSString *)key {
SD_LOCK(self.callbacksLock);
NSMutableArray<id> *callbacks = [[self.callbackBlocks valueForKey:key] mutableCopy];
SD_UNLOCK(self.callbacksLock);
NSMutableArray<id> *callbacks;
@synchronized (self) {
callbacks = [[self.callbackBlocks valueForKey:key] mutableCopy];
}
// We need to remove [NSNull null] because there might not always be a progress block for each callback
[callbacks removeObjectIdenticalTo:[NSNull null]];
return [callbacks copy]; // strip mutability here
@ -113,12 +111,12 @@ typedef NSMutableDictionary<NSString *, id> SDCallbacksDictionary;
- (BOOL)cancel:(nullable id)token {
BOOL shouldCancel = NO;
SD_LOCK(self.callbacksLock);
[self.callbackBlocks removeObjectIdenticalTo:token];
if (self.callbackBlocks.count == 0) {
shouldCancel = YES;
@synchronized (self) {
[self.callbackBlocks removeObjectIdenticalTo:token];
if (self.callbackBlocks.count == 0) {
shouldCancel = YES;
}
}
SD_UNLOCK(self.callbacksLock);
if (shouldCancel) {
[self cancel];
}
@ -238,11 +236,8 @@ typedef NSMutableDictionary<NSString *, id> SDCallbacksDictionary;
}
- (void)reset {
SD_LOCK(self.callbacksLock);
[self.callbackBlocks removeAllObjects];
SD_UNLOCK(self.callbacksLock);
@synchronized (self) {
[self.callbackBlocks removeAllObjects];
self.dataTask = nil;
if (self.ownedSession) {