Introduce a cacheURLs array to prevent race conditions related to checking the on-disk image cache. (Fix #47)

If a cache check is initiated, then canceled with cancelForDelegate: before a new cache check
is started with a different URL but the same delegate, the ongoing disk cache check would still
message the delegate despite being canceled by cancelForDelegate:. This is because it only
checked to see if the delegate was in the cacheDelegates array; it had been removed by
cancelForDelegate:, but added back by the new unrelated cache check. cacheURLs keeps track
of which specific URLs are actually requested by the delegate. If the URL from a completed
disk cache check does not match the delegate in cacheDelegates, the delegate is not messaged
(since we know that request was canceled).
This commit is contained in:
Adam Ernst 2011-12-05 09:07:43 -05:00 committed by Olivier Poitrey
parent 86b60e9c98
commit 8318b295bf
2 changed files with 29 additions and 9 deletions

View File

@ -23,6 +23,7 @@ typedef enum
NSMutableArray *downloadDelegates;
NSMutableArray *downloaders;
NSMutableArray *cacheDelegates;
NSMutableArray *cacheURLs;
NSMutableDictionary *downloaderForURL;
NSMutableArray *failedURLs;
}

View File

@ -21,6 +21,7 @@ static SDWebImageManager *instance;
downloadDelegates = [[NSMutableArray alloc] init];
downloaders = [[NSMutableArray alloc] init];
cacheDelegates = [[NSMutableArray alloc] init];
cacheURLs = [[NSMutableArray alloc] init];
downloaderForURL = [[NSMutableDictionary alloc] init];
failedURLs = [[NSMutableArray alloc] init];
}
@ -32,6 +33,7 @@ static SDWebImageManager *instance;
[downloadDelegates release], downloadDelegates = nil;
[downloaders release], downloaders = nil;
[cacheDelegates release], cacheDelegates = nil;
[cacheURLs release], cacheURLs = nil;
[downloaderForURL release], downloaderForURL = nil;
[failedURLs release], failedURLs = nil;
[super dealloc];
@ -96,17 +98,20 @@ static SDWebImageManager *instance;
// Check the on-disk cache async so we don't block the main thread
[cacheDelegates addObject:delegate];
[cacheURLs addObject:url];
NSDictionary *info = [NSDictionary dictionaryWithObjectsAndKeys:delegate, @"delegate", url, @"url", [NSNumber numberWithInt:options], @"options", nil];
[[SDImageCache sharedImageCache] queryDiskCacheForKey:[url absoluteString] delegate:self userInfo:info];
}
- (void)cancelForDelegate:(id<SDWebImageManagerDelegate>)delegate
{
// Remove all instances of delegate from cacheDelegates.
// (removeObjectIdenticalTo: does this, despite its singular name.)
[cacheDelegates removeObjectIdenticalTo:delegate];
NSUInteger idx;
while ((idx = [cacheDelegates indexOfObjectIdenticalTo:delegate]) != NSNotFound)
{
[cacheDelegates removeObjectAtIndex:idx];
[cacheURLs removeObjectAtIndex:idx];
}
while ((idx = [downloadDelegates indexOfObjectIdenticalTo:delegate]) != NSNotFound)
{
SDWebImageDownloader *downloader = [[downloaders objectAtIndex:idx] retain];
@ -127,11 +132,26 @@ static SDWebImageManager *instance;
#pragma mark SDImageCacheDelegate
- (NSUInteger)indexOfDelegate:(id<SDWebImageManagerDelegate>)delegate waitingForURL:(NSURL *)url
{
// Do a linear search, simple (even if inefficient)
NSUInteger idx;
for (idx = 0; idx < [cacheDelegates count]; idx++)
{
if ([cacheDelegates objectAtIndex:idx] == delegate && [[cacheURLs objectAtIndex:idx] isEqual:url])
{
return idx;
}
}
return NSNotFound;
}
- (void)imageCache:(SDImageCache *)imageCache didFindImage:(UIImage *)image forKey:(NSString *)key userInfo:(NSDictionary *)info
{
NSURL *url = [info objectForKey:@"url"];
id<SDWebImageManagerDelegate> delegate = [info objectForKey:@"delegate"];
NSUInteger idx = [cacheDelegates indexOfObjectIdenticalTo:delegate];
NSUInteger idx = [self indexOfDelegate:delegate waitingForURL:url];
if (idx == NSNotFound)
{
// Request has since been canceled
@ -143,10 +163,8 @@ static SDWebImageManager *instance;
[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];
[cacheURLs removeObjectAtIndex:idx];
}
- (void)imageCache:(SDImageCache *)imageCache didNotFindImageForKey:(NSString *)key userInfo:(NSDictionary *)info
@ -155,7 +173,7 @@ static SDWebImageManager *instance;
id<SDWebImageManagerDelegate> delegate = [info objectForKey:@"delegate"];
SDWebImageOptions options = [[info objectForKey:@"options"] intValue];
NSUInteger idx = [cacheDelegates indexOfObjectIdenticalTo:delegate];
NSUInteger idx = [self indexOfDelegate:delegate waitingForURL:url];
if (idx == NSNotFound)
{
// Request has since been canceled
@ -163,6 +181,7 @@ static SDWebImageManager *instance;
}
[cacheDelegates removeObjectAtIndex:idx];
[cacheURLs removeObjectAtIndex:idx];
// Share the same downloader for identical URLs so we don't download the same URL several times
SDWebImageDownloader *downloader = [downloaderForURL objectForKey:url];