From dd682c92b59f556d548031dbaaf3ea0ea5a6f269 Mon Sep 17 00:00:00 2001 From: Matej Bukovinski Date: Mon, 24 Mar 2014 20:14:18 +0100 Subject: [PATCH 1/5] Performing cache callbacks from the ioQueue asynchronously. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - more appropriate than dispatch_main_sync_safe, since we’ll always be on the ioQueue when calling - prevents deadlock situation described in #625 --- SDWebImage/SDImageCache.m | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/SDWebImage/SDImageCache.m b/SDWebImage/SDImageCache.m index 57778cf5..04468472 100644 --- a/SDWebImage/SDImageCache.m +++ b/SDWebImage/SDImageCache.m @@ -296,7 +296,7 @@ BOOL ImageDataHasPNGPreffix(NSData *data) { [self.memCache setObject:diskImage forKey:key cost:cost]; } - dispatch_main_sync_safe(^{ + dispatch_async(dispatch_get_main_queue(), ^{ doneBlock(diskImage, SDImageCacheTypeDisk); }); } @@ -349,7 +349,7 @@ BOOL ImageDataHasPNGPreffix(NSData *data) { error:NULL]; if (completion) { - dispatch_main_sync_safe(^{ + dispatch_async(dispatch_get_main_queue(), ^{ completion(); }); } @@ -486,7 +486,7 @@ BOOL ImageDataHasPNGPreffix(NSData *data) { } if (completionBlock) { - dispatch_main_sync_safe(^{ + dispatch_async(dispatch_get_main_queue(), ^{ completionBlock(fileCount, totalSize); }); } From 1c463ad46b3aac9213ad7de9a9acdd64bf580bce Mon Sep 17 00:00:00 2001 From: Matej Bukovinski Date: Mon, 24 Mar 2014 20:35:52 +0100 Subject: [PATCH 2/5] Using the private fileManager instance when on the ioQueue. --- SDWebImage/SDImageCache.m | 43 +++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/SDWebImage/SDImageCache.m b/SDWebImage/SDImageCache.m index 04468472..7e739248 100644 --- a/SDWebImage/SDImageCache.m +++ b/SDWebImage/SDImageCache.m @@ -181,14 +181,11 @@ BOOL ImageDataHasPNGPreffix(NSData *data) { } if (data) { - // Can't use defaultManager another thread - NSFileManager *fileManager = [NSFileManager new]; - - if (![fileManager fileExistsAtPath:_diskCachePath]) { - [fileManager createDirectoryAtPath:_diskCachePath withIntermediateDirectories:YES attributes:nil error:NULL]; + if (![_fileManager fileExistsAtPath:_diskCachePath]) { + [_fileManager createDirectoryAtPath:_diskCachePath withIntermediateDirectories:YES attributes:nil error:NULL]; } - [fileManager createFileAtPath:[self defaultCachePathForKey:key] contents:data attributes:nil]; + [_fileManager createFileAtPath:[self defaultCachePathForKey:key] contents:data attributes:nil]; } }); } @@ -318,7 +315,7 @@ BOOL ImageDataHasPNGPreffix(NSData *data) { if (fromDisk) { dispatch_async(self.ioQueue, ^{ - [[NSFileManager defaultManager] removeItemAtPath:[self defaultCachePathForKey:key] error:nil]; + [_fileManager removeItemAtPath:[self defaultCachePathForKey:key] error:nil]; }); } } @@ -342,11 +339,11 @@ BOOL ImageDataHasPNGPreffix(NSData *data) { - (void)clearDiskOnCompletion:(void (^)())completion { dispatch_async(self.ioQueue, ^{ - [[NSFileManager defaultManager] removeItemAtPath:self.diskCachePath error:nil]; - [[NSFileManager defaultManager] createDirectoryAtPath:self.diskCachePath - withIntermediateDirectories:YES - attributes:nil - error:NULL]; + [_fileManager removeItemAtPath:self.diskCachePath error:nil]; + [_fileManager createDirectoryAtPath:self.diskCachePath + withIntermediateDirectories:YES + attributes:nil + error:NULL]; if (completion) { dispatch_async(dispatch_get_main_queue(), ^{ @@ -358,15 +355,14 @@ BOOL ImageDataHasPNGPreffix(NSData *data) { - (void)cleanDisk { dispatch_async(self.ioQueue, ^{ - NSFileManager *fileManager = [NSFileManager defaultManager]; NSURL *diskCacheURL = [NSURL fileURLWithPath:self.diskCachePath isDirectory:YES]; NSArray *resourceKeys = @[NSURLIsDirectoryKey, NSURLContentModificationDateKey, NSURLTotalFileAllocatedSizeKey]; // This enumerator prefetches useful properties for our cache files. - NSDirectoryEnumerator *fileEnumerator = [fileManager enumeratorAtURL:diskCacheURL - includingPropertiesForKeys:resourceKeys - options:NSDirectoryEnumerationSkipsHiddenFiles - errorHandler:NULL]; + NSDirectoryEnumerator *fileEnumerator = [_fileManager enumeratorAtURL:diskCacheURL + includingPropertiesForKeys:resourceKeys + options:NSDirectoryEnumerationSkipsHiddenFiles + errorHandler:NULL]; NSDate *expirationDate = [NSDate dateWithTimeIntervalSinceNow:-self.maxCacheAge]; NSMutableDictionary *cacheFiles = [NSMutableDictionary dictionary]; @@ -387,7 +383,7 @@ BOOL ImageDataHasPNGPreffix(NSData *data) { // Remove files that are older than the expiration date; NSDate *modificationDate = resourceValues[NSURLContentModificationDateKey]; if ([[modificationDate laterDate:expirationDate] isEqualToDate:expirationDate]) { - [fileManager removeItemAtURL:fileURL error:nil]; + [_fileManager removeItemAtURL:fileURL error:nil]; continue; } @@ -411,7 +407,7 @@ BOOL ImageDataHasPNGPreffix(NSData *data) { // Delete files until we fall below our desired cache size. for (NSURL *fileURL in sortedFiles) { - if ([fileManager removeItemAtURL:fileURL error:nil]) { + if ([_fileManager removeItemAtURL:fileURL error:nil]) { NSDictionary *resourceValues = cacheFiles[fileURL]; NSNumber *totalAllocatedSize = resourceValues[NSURLTotalFileAllocatedSizeKey]; currentCacheSize -= [totalAllocatedSize unsignedIntegerValue]; @@ -472,11 +468,10 @@ BOOL ImageDataHasPNGPreffix(NSData *data) { NSUInteger fileCount = 0; NSUInteger totalSize = 0; - NSFileManager *fileManager = [NSFileManager defaultManager]; - NSDirectoryEnumerator *fileEnumerator = [fileManager enumeratorAtURL:diskCacheURL - includingPropertiesForKeys:@[NSFileSize] - options:NSDirectoryEnumerationSkipsHiddenFiles - errorHandler:NULL]; + NSDirectoryEnumerator *fileEnumerator = [_fileManager enumeratorAtURL:diskCacheURL + includingPropertiesForKeys:@[NSFileSize] + options:NSDirectoryEnumerationSkipsHiddenFiles + errorHandler:NULL]; for (NSURL *fileURL in fileEnumerator) { NSNumber *fileSize; From 63f5c9706e244f40af1003edbc176ee53562a54d Mon Sep 17 00:00:00 2001 From: Matej Bukovinski Date: Mon, 24 Mar 2014 20:48:06 +0100 Subject: [PATCH 3/5] Performing getSize and getDiskCount on the ioQueue. --- SDWebImage/SDImageCache.m | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/SDWebImage/SDImageCache.m b/SDWebImage/SDImageCache.m index 7e739248..2d4471f8 100644 --- a/SDWebImage/SDImageCache.m +++ b/SDWebImage/SDImageCache.m @@ -441,23 +441,26 @@ BOOL ImageDataHasPNGPreffix(NSData *data) { } - (NSUInteger)getSize { - NSUInteger size = 0; - NSDirectoryEnumerator *fileEnumerator = [[NSFileManager defaultManager] enumeratorAtPath:self.diskCachePath]; - for (NSString *fileName in fileEnumerator) { - NSString *filePath = [self.diskCachePath stringByAppendingPathComponent:fileName]; - NSDictionary *attrs = [[NSFileManager defaultManager] attributesOfItemAtPath:filePath error:nil]; - size += [attrs fileSize]; - } + __block NSUInteger size = 0; + dispatch_sync(self.ioQueue, ^{ + NSDirectoryEnumerator *fileEnumerator = [_fileManager enumeratorAtPath:self.diskCachePath]; + for (NSString *fileName in fileEnumerator) { + NSString *filePath = [self.diskCachePath stringByAppendingPathComponent:fileName]; + NSDictionary *attrs = [[NSFileManager defaultManager] attributesOfItemAtPath:filePath error:nil]; + size += [attrs fileSize]; + } + }); return size; } - (int)getDiskCount { - int count = 0; - NSDirectoryEnumerator *fileEnumerator = [[NSFileManager defaultManager] enumeratorAtPath:self.diskCachePath]; - for (__unused NSString *fileName in fileEnumerator) { - count += 1; - } - + __block int count = 0; + dispatch_sync(self.ioQueue, ^{ + NSDirectoryEnumerator *fileEnumerator = [_fileManager enumeratorAtPath:self.diskCachePath]; + for (__unused NSString *fileName in fileEnumerator) { + count += 1; + } + }); return count; } From 87aed007335603587d0c7d0fb09a9dc9235a67a1 Mon Sep 17 00:00:00 2001 From: Matej Bukovinski Date: Mon, 24 Mar 2014 21:14:21 +0100 Subject: [PATCH 4/5] Fixed the background task handled in backgroundCleanDisk. - the background task had no effect, since cleanDisk returns immediately and thereby cancels the background task - adding cleanDiskWithCompletionBlock: and modifying backgroundCleanDisk to use this method resolves the issue --- SDWebImage/SDImageCache.m | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/SDWebImage/SDImageCache.m b/SDWebImage/SDImageCache.m index 2d4471f8..3f36e5e1 100644 --- a/SDWebImage/SDImageCache.m +++ b/SDWebImage/SDImageCache.m @@ -354,6 +354,10 @@ BOOL ImageDataHasPNGPreffix(NSData *data) { } - (void)cleanDisk { + [self cleanDiskWithCompletionBlock:nil]; +} + +- (void)cleanDiskWithCompletionBlock:(void (^)())completionBlock { dispatch_async(self.ioQueue, ^{ NSURL *diskCacheURL = [NSURL fileURLWithPath:self.diskCachePath isDirectory:YES]; NSArray *resourceKeys = @[NSURLIsDirectoryKey, NSURLContentModificationDateKey, NSURLTotalFileAllocatedSizeKey]; @@ -418,6 +422,11 @@ BOOL ImageDataHasPNGPreffix(NSData *data) { } } } + if (completionBlock) { + dispatch_async(dispatch_get_main_queue(), ^{ + completionBlock(); + }); + } }); } @@ -431,13 +440,10 @@ BOOL ImageDataHasPNGPreffix(NSData *data) { }]; // Start the long-running task and return immediately. - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - // Do the work associated with the task, preferably in chunks. - [self cleanDisk]; - + [self cleanDiskWithCompletionBlock:^{ [application endBackgroundTask:bgTask]; bgTask = UIBackgroundTaskInvalid; - }); + }]; } - (NSUInteger)getSize { From 556665d8a79714d73eaffad864ecfccecbca2680 Mon Sep 17 00:00:00 2001 From: Matej Bukovinski Date: Mon, 24 Mar 2014 21:14:32 +0100 Subject: [PATCH 5/5] Exposed cleanDiskWithCompletionBlock:, added some additional documentation and fixed a typo. --- SDWebImage/SDImageCache.h | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/SDWebImage/SDImageCache.h b/SDWebImage/SDImageCache.h index 8989a184..2e1eed56 100644 --- a/SDWebImage/SDImageCache.h +++ b/SDWebImage/SDImageCache.h @@ -128,7 +128,7 @@ typedef void(^SDWebImageQueryCompletedBlock)(UIImage *image, SDImageCacheType ca - (void)removeImageForKey:(NSString *)key; /** - * Remove the image from memory and optionaly disk cache synchronously + * Remove the image from memory and optionally disk cache synchronously * * @param key The unique image cache key * @param fromDisk Also remove cache entry from disk if YES @@ -141,13 +141,26 @@ typedef void(^SDWebImageQueryCompletedBlock)(UIImage *image, SDImageCacheType ca - (void)clearMemory; /** - * Clear all disk cached images + * Clear all disk cached images. Non-blocking method - returns immediately. + * @param completionBlock An block that should be executed after cache expiration completes (optional) */ -- (void)clearDisk; - (void)clearDiskOnCompletion:(void (^)())completion; +/** + * Clear all disk cached images + * @see clearDiskOnCompletion: + */ +- (void)clearDisk; + +/** + * Remove all expired cached image from disk. Non-blocking method - returns immediately. + * @param completionBlock An block that should be executed after cache expiration completes (optional) + */ +- (void)cleanDiskWithCompletionBlock:(void (^)())completionBlock; + /** * Remove all expired cached image from disk + * @see cleanDiskWithCompletionBlock: */ - (void)cleanDisk;