From fe730cc7e414d30b4a34d99e2fc41b46585d9f5c Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Wed, 6 May 2020 16:13:32 +0800 Subject: [PATCH] Fix the deadlock by using the two different semaphore, the `@synchronized` can not represent two pointer array lock and may be entered twice --- SDWebImage/Core/SDWebImagePrefetcher.m | 67 ++++++++++++++++---------- 1 file changed, 41 insertions(+), 26 deletions(-) diff --git a/SDWebImage/Core/SDWebImagePrefetcher.m b/SDWebImage/Core/SDWebImagePrefetcher.m index 30b65ace..5c0f37f7 100644 --- a/SDWebImage/Core/SDWebImagePrefetcher.m +++ b/SDWebImage/Core/SDWebImagePrefetcher.m @@ -20,6 +20,10 @@ atomic_flag _isAllFinished; unsigned long _totalCount; + + // Used to ensure NSPointerArray thread safe + dispatch_semaphore_t _prefetchOperationsLock; + dispatch_semaphore_t _loadOperationsLock; } @property (nonatomic, copy, readwrite) NSArray *urls; @@ -106,7 +110,6 @@ } - (void)startPrefetchWithToken:(SDWebImagePrefetchToken * _Nonnull)token { - NSPointerArray *operations = token.loadOperations; for (NSURL *url in token.urls) { @autoreleasepool { @weakify(self); @@ -142,13 +145,13 @@ [asyncOperation complete]; }]; NSAssert(operation != nil, @"Operation should not be nil, [SDWebImageManager loadImageWithURL:options:context:progress:completed:] break prefetch logic"); - @synchronized (token) { - [operations addPointer:(__bridge void *)operation]; - } + SD_LOCK(token->_loadOperationsLock); + [token.loadOperations addPointer:(__bridge void *)operation]; + SD_UNLOCK(token->_loadOperationsLock); }]; - @synchronized (token) { - [token.prefetchOperations addPointer:(__bridge void *)prefetchOperation]; - } + SD_LOCK(token->_prefetchOperationsLock); + [token.prefetchOperations addPointer:(__bridge void *)prefetchOperation]; + SD_UNLOCK(token->_prefetchOperationsLock); [self.prefetchQueue addOperation:prefetchOperation]; } } @@ -262,26 +265,38 @@ @implementation SDWebImagePrefetchToken -- (void)cancel { - @synchronized (self) { - [self.prefetchOperations compact]; - for (id operation in self.prefetchOperations) { - id strongOperation = operation; - if (strongOperation) { - [strongOperation cancel]; - } - } - self.prefetchOperations.count = 0; - - [self.loadOperations compact]; - for (id operation in self.loadOperations) { - id strongOperation = operation; - if (strongOperation) { - [strongOperation cancel]; - } - } - self.loadOperations.count = 0; +- (instancetype)init { + self = [super init]; + if (self) { + _prefetchOperationsLock = dispatch_semaphore_create(1); + _loadOperationsLock = dispatch_semaphore_create(1); } + return self; +} + +- (void)cancel { + SD_LOCK(_prefetchOperationsLock); + [self.prefetchOperations compact]; + for (id operation in self.prefetchOperations) { + id strongOperation = operation; + if (strongOperation) { + [strongOperation cancel]; + } + } + self.prefetchOperations.count = 0; + SD_UNLOCK(_prefetchOperationsLock); + + SD_LOCK(_loadOperationsLock); + [self.loadOperations compact]; + for (id operation in self.loadOperations) { + id strongOperation = operation; + if (strongOperation) { + [strongOperation cancel]; + } + } + self.loadOperations.count = 0; + SD_UNLOCK(_loadOperationsLock); + self.completionBlock = nil; self.progressBlock = nil; [self.prefetcher removeRunningToken:self];