Fix race condition with SDWebImageManager's cancelForDelegate:

The -cancelForDelegate: method was working for downloads but not local cache checks. Add some new machinery to keep track of pending cache requests and only message the delegate if it hasn't since requested cancellation.
This commit is contained in:
Adam Ernst 2011-07-12 20:32:33 -04:00 committed by Olivier Poitrey
parent 47aad5b55f
commit 45cc126d32
2 changed files with 54 additions and 27 deletions

View File

@ -13,8 +13,9 @@
@interface SDWebImageManager : NSObject <SDWebImageDownloaderDelegate, SDImageCacheDelegate> @interface SDWebImageManager : NSObject <SDWebImageDownloaderDelegate, SDImageCacheDelegate>
{ {
NSMutableArray *delegates; NSMutableArray *downloadDelegates;
NSMutableArray *downloaders; NSMutableArray *downloaders;
NSMutableArray *cacheDelegates;
NSMutableDictionary *downloaderForURL; NSMutableDictionary *downloaderForURL;
NSMutableArray *failedURLs; NSMutableArray *failedURLs;
} }

View File

@ -18,8 +18,9 @@ static SDWebImageManager *instance;
{ {
if ((self = [super init])) if ((self = [super init]))
{ {
delegates = [[NSMutableArray alloc] init]; downloadDelegates = [[NSMutableArray alloc] init];
downloaders = [[NSMutableArray alloc] init]; downloaders = [[NSMutableArray alloc] init];
cacheDelegates = [[NSMutableArray alloc] init];
downloaderForURL = [[NSMutableDictionary alloc] init]; downloaderForURL = [[NSMutableDictionary alloc] init];
failedURLs = [[NSMutableArray alloc] init]; failedURLs = [[NSMutableArray alloc] init];
} }
@ -28,8 +29,9 @@ static SDWebImageManager *instance;
- (void)dealloc - (void)dealloc
{ {
[delegates release], delegates = nil; [downloadDelegates release], downloadDelegates = nil;
[downloaders release], downloaders = nil; [downloaders release], downloaders = nil;
[cacheDelegates release], cacheDelegates = nil;
[downloaderForURL release], downloaderForURL = nil; [downloaderForURL release], downloaderForURL = nil;
[failedURLs release], failedURLs = nil; [failedURLs release], failedURLs = nil;
[super dealloc]; [super dealloc];
@ -72,32 +74,34 @@ static SDWebImageManager *instance;
} }
// Check the on-disk cache async so we don't block the main thread // Check the on-disk cache async so we don't block the main thread
[cacheDelegates addObject:delegate];
NSDictionary *info = [NSDictionary dictionaryWithObjectsAndKeys:delegate, @"delegate", url, @"url", [NSNumber numberWithBool:lowPriority], @"low_priority", nil]; NSDictionary *info = [NSDictionary dictionaryWithObjectsAndKeys:delegate, @"delegate", url, @"url", [NSNumber numberWithBool:lowPriority], @"low_priority", nil];
[[SDImageCache sharedImageCache] queryDiskCacheForKey:[url absoluteString] delegate:self userInfo:info]; [[SDImageCache sharedImageCache] queryDiskCacheForKey:[url absoluteString] delegate:self userInfo:info];
} }
- (void)cancelForDelegate:(id<SDWebImageManagerDelegate>)delegate - (void)cancelForDelegate:(id<SDWebImageManagerDelegate>)delegate
{ {
NSUInteger idx = [delegates indexOfObjectIdenticalTo:delegate]; // Remove all instances of delegate from cacheDelegates.
// (removeObjectIdenticalTo: does this, despite its singular name.)
[cacheDelegates removeObjectIdenticalTo:delegate];
if (idx == NSNotFound) NSUInteger idx;
while ((idx = [downloadDelegates indexOfObjectIdenticalTo:delegate]) != NSNotFound)
{ {
return; SDWebImageDownloader *downloader = [[downloaders objectAtIndex:idx] retain];
[downloadDelegates removeObjectAtIndex:idx];
[downloaders removeObjectAtIndex:idx];
if (![downloaders containsObject:downloader])
{
// No more delegate are waiting for this download, cancel it
[downloader cancel];
[downloaderForURL removeObjectForKey:downloader.url];
}
[downloader release];
} }
SDWebImageDownloader *downloader = [[downloaders objectAtIndex:idx] retain];
[delegates removeObjectAtIndex:idx];
[downloaders removeObjectAtIndex:idx];
if (![downloaders containsObject:downloader])
{
// No more delegate are waiting for this download, cancel it
[downloader cancel];
[downloaderForURL removeObjectForKey:downloader.url];
}
[downloader release];
} }
#pragma mark SDImageCacheDelegate #pragma mark SDImageCacheDelegate
@ -105,10 +109,23 @@ static SDWebImageManager *instance;
- (void)imageCache:(SDImageCache *)imageCache didFindImage:(UIImage *)image forKey:(NSString *)key userInfo:(NSDictionary *)info - (void)imageCache:(SDImageCache *)imageCache didFindImage:(UIImage *)image forKey:(NSString *)key userInfo:(NSDictionary *)info
{ {
id<SDWebImageManagerDelegate> delegate = [info objectForKey:@"delegate"]; id<SDWebImageManagerDelegate> delegate = [info objectForKey:@"delegate"];
NSUInteger idx = [cacheDelegates indexOfObjectIdenticalTo:delegate];
if (idx == NSNotFound)
{
// Request has since been canceled
return;
}
if ([delegate respondsToSelector:@selector(webImageManager:didFinishWithImage:)]) if ([delegate respondsToSelector:@selector(webImageManager:didFinishWithImage:)])
{ {
[delegate performSelector:@selector(webImageManager:didFinishWithImage:) withObject:self withObject:image]; [delegate performSelector:@selector(webImageManager:didFinishWithImage:) withObject:self withObject:image];
} }
// Remove one instance of delegate from the array,
// not all of them (as |removeObjectIdenticalTo:| would)
// in case multiple requests are issued.
[cacheDelegates removeObjectAtIndex:idx];
} }
- (void)imageCache:(SDImageCache *)imageCache didNotFindImageForKey:(NSString *)key userInfo:(NSDictionary *)info - (void)imageCache:(SDImageCache *)imageCache didNotFindImageForKey:(NSString *)key userInfo:(NSDictionary *)info
@ -117,6 +134,15 @@ static SDWebImageManager *instance;
id<SDWebImageManagerDelegate> delegate = [info objectForKey:@"delegate"]; id<SDWebImageManagerDelegate> delegate = [info objectForKey:@"delegate"];
BOOL lowPriority = [[info objectForKey:@"low_priority"] boolValue]; BOOL lowPriority = [[info objectForKey:@"low_priority"] boolValue];
NSUInteger idx = [cacheDelegates indexOfObjectIdenticalTo:delegate];
if (idx == NSNotFound)
{
// Request has since been canceled
return;
}
[cacheDelegates removeObjectAtIndex:idx];
// Share the same downloader for identical URLs so we don't download the same URL several times // Share the same downloader for identical URLs so we don't download the same URL several times
SDWebImageDownloader *downloader = [downloaderForURL objectForKey:url]; SDWebImageDownloader *downloader = [downloaderForURL objectForKey:url];
@ -132,7 +158,7 @@ static SDWebImageManager *instance;
downloader.lowPriority = NO; downloader.lowPriority = NO;
} }
[delegates addObject:delegate]; [downloadDelegates addObject:delegate];
[downloaders addObject:downloader]; [downloaders addObject:downloader];
} }
@ -142,14 +168,14 @@ static SDWebImageManager *instance;
{ {
[downloader retain]; [downloader retain];
// Notify all the delegates with this downloader // Notify all the downloadDelegates with this downloader
for (NSInteger idx = (NSInteger)[downloaders count] - 1; idx >= 0; idx--) for (NSInteger idx = (NSInteger)[downloaders count] - 1; idx >= 0; idx--)
{ {
NSUInteger uidx = (NSUInteger)idx; NSUInteger uidx = (NSUInteger)idx;
SDWebImageDownloader *aDownloader = [downloaders objectAtIndex:uidx]; SDWebImageDownloader *aDownloader = [downloaders objectAtIndex:uidx];
if (aDownloader == downloader) if (aDownloader == downloader)
{ {
id<SDWebImageManagerDelegate> delegate = [delegates objectAtIndex:uidx]; id<SDWebImageManagerDelegate> delegate = [downloadDelegates objectAtIndex:uidx];
if (image) if (image)
{ {
@ -167,7 +193,7 @@ static SDWebImageManager *instance;
} }
[downloaders removeObjectAtIndex:uidx]; [downloaders removeObjectAtIndex:uidx];
[delegates removeObjectAtIndex:uidx]; [downloadDelegates removeObjectAtIndex:uidx];
} }
} }
@ -195,14 +221,14 @@ static SDWebImageManager *instance;
{ {
[downloader retain]; [downloader retain];
// Notify all the delegates with this downloader // Notify all the downloadDelegates with this downloader
for (NSInteger idx = (NSInteger)[downloaders count] - 1; idx >= 0; idx--) for (NSInteger idx = (NSInteger)[downloaders count] - 1; idx >= 0; idx--)
{ {
NSUInteger uidx = (NSUInteger)idx; NSUInteger uidx = (NSUInteger)idx;
SDWebImageDownloader *aDownloader = [downloaders objectAtIndex:uidx]; SDWebImageDownloader *aDownloader = [downloaders objectAtIndex:uidx];
if (aDownloader == downloader) if (aDownloader == downloader)
{ {
id<SDWebImageManagerDelegate> delegate = [delegates objectAtIndex:uidx]; id<SDWebImageManagerDelegate> delegate = [downloadDelegates objectAtIndex:uidx];
if ([delegate respondsToSelector:@selector(webImageManager:didFailWithError:)]) if ([delegate respondsToSelector:@selector(webImageManager:didFailWithError:)])
{ {
@ -210,7 +236,7 @@ static SDWebImageManager *instance;
} }
[downloaders removeObjectAtIndex:uidx]; [downloaders removeObjectAtIndex:uidx];
[delegates removeObjectAtIndex:uidx]; [downloadDelegates removeObjectAtIndex:uidx];
} }
} }