Take #2: Attempt to fix unreproducible crasher. Don't rely on auto-zeroing weak references to know when it's no longer OK to hand back a cached value.

Mallory Paine · Mallory Paine · commit 9a44792d1a46 · 2014-01-14T22:01:05.000-08:00
diff --git a/FastImageCache/FICImageTable.m b/FastImageCache/FICImageTable.m
@@ -12,7 +12,6 @@
 #import "FICImageTableChunk.h"
 #import "FICImageTableEntry.h"
 #import "FICUtilities.h"
-#import "FICWeakReference.h"
 
 #import "FICImageCache+FICErrorLogging.h"
 
@@ -51,7 +50,8 @@ @interface FICImageTable () {
     NSInteger _chunkCount;
     
     NSMutableDictionary *_chunkDictionary;
-    NSMutableArray *_recentChunks;
+    NSCountedSet *_chunkSet;
+    
     NSRecursiveLock *_lock;
     
     // Image table metadata
@@ -140,6 +140,7 @@ - (instancetype)initWithFormat:(FICImageFormat *)imageFormat {
         _imageLength = _imageRowLength * (NSInteger)pixelSize.height;
         
         _chunkDictionary = [[NSMutableDictionary alloc] init];
+        _chunkSet = [[NSCountedSet alloc] init];
         
         _indexMap = [[NSMutableDictionary alloc] init];
         _occupiedIndexes = [[NSMutableIndexSet alloc] init];
@@ -149,8 +150,6 @@ - (instancetype)initWithFormat:(FICImageFormat *)imageFormat {
 
         _sourceImageMap = [[NSMutableDictionary alloc] init];
         
-        _recentChunks = [[NSMutableArray alloc] init];
-        
         _filePath = [[self tableFilePath] copy];
         
         [self _loadMetadata];
@@ -206,24 +205,13 @@ - (void)dealloc {
 #pragma mark - Working with Chunks
 
 - (FICImageTableChunk *)_cachedChunkAtIndex:(NSInteger)index {
-    FICImageTableChunk *cachedChunk = nil;
-    NSNumber *indexNumber = @(index);
-    FICWeakReference *chunkReference = [_chunkDictionary objectForKey:indexNumber];
-    
-    if (chunkReference) {
-        cachedChunk = [chunkReference object];
-        if (!cachedChunk) {
-            [_chunkDictionary removeObjectForKey:indexNumber];
-        }
-    }
-    
-    return cachedChunk;
+    return [_chunkDictionary objectForKey:@(index)];
 }
 
 - (void)_setChunk:(FICImageTableChunk *)chunk index:(NSInteger)index {
     NSNumber *indexNumber = @(index);
     if (chunk != nil) {
-        [_chunkDictionary setObject:[[FICWeakReference alloc] initWithObject:chunk] forKey:indexNumber];
+        [_chunkDictionary setObject:chunk forKey:indexNumber];
     } else {
         [_chunkDictionary removeObjectForKey:indexNumber];
     }
@@ -245,15 +233,6 @@ - (FICImageTableChunk *)_chunkAtIndex:(NSInteger)index {
             chunk = [[FICImageTableChunk alloc] initWithFileDescriptor:_fileDescriptor index:index length:chunkLength];
             [self _setChunk:chunk index:index];
         }
-        
-        if (chunk != nil) {
-            static const NSInteger __recentChunksToKeepMapped = 2;
-            [_recentChunks insertObject:chunk atIndex:0];
-            
-            if ([_recentChunks count] > __recentChunksToKeepMapped) {
-                [_recentChunks removeLastObject];
-            }            
-        }
     }
     
     return chunk;
@@ -467,6 +446,13 @@ - (FICImageTableEntry *)_entryDataAtIndex:(NSInteger)index {
             void *mappedChunkAddress = [chunk bytes];
             void *mappedEntryAddress = mappedChunkAddress + entryOffsetInChunk;
             entryData = [[FICImageTableEntry alloc] initWithImageTableChunk:chunk bytes:mappedEntryAddress length:_entryLength];
+            
+            [_chunkSet addObject:chunk];
+            
+            __weak FICImageTable *weakSelf = self;
+            [entryData executeBlockOnDealloc:^{
+                [weakSelf _entryWasDeallocatedFromChunk:chunk];
+            }];
         }
     }
     
@@ -475,6 +461,15 @@ - (FICImageTableEntry *)_entryDataAtIndex:(NSInteger)index {
     return entryData;
 }
 
+- (void)_entryWasDeallocatedFromChunk:(FICImageTableChunk *)chunk {
+    [_lock lock];
+    [_chunkSet removeObject:chunk];
+    if ([_chunkSet countForObject:chunk] == 0) {
+        [self _setChunk:nil index:[chunk index]];
+    }
+    [_lock unlock];
+}
+
 - (NSInteger)_nextEntryIndex {
     NSMutableIndexSet *unoccupiedIndexes = [[NSMutableIndexSet alloc] initWithIndexesInRange:NSMakeRange(0, _entryCount)];
     [unoccupiedIndexes removeIndexes:_occupiedIndexes];
diff --git a/FastImageCache/FICImageTableEntry.h b/FastImageCache/FICImageTableEntry.h
@@ -52,6 +52,8 @@ typedef struct {
  */
 @property (nonatomic, assign) CFUUIDBytes sourceImageUUIDBytes;
 
+@property (nonatomic, readonly) FICImageTableChunk *imageTableChunk;
+
 ///----------------------------------------
 /// @name Initializing an Image Table Entry
 ///----------------------------------------
diff --git a/FastImageCache/FICImageTableEntry.m b/FastImageCache/FICImageTableEntry.m
@@ -21,7 +21,7 @@ @interface FICImageTableEntry () {
     FICImageTableChunk *_imageTableChunk;
     void *_bytes;
     size_t _length;
-    dispatch_block_t _deallocBlock;
+    NSMutableArray *_deallocBlocks;
 }
 
 @end
@@ -32,6 +32,7 @@ @implementation FICImageTableEntry
 
 @synthesize bytes = _bytes;
 @synthesize length = _length;
+@synthesize imageTableChunk = _imageTableChunk;
 
 #pragma mark - Property Accessors
 
@@ -64,18 +65,19 @@ - (id)initWithImageTableChunk:(FICImageTableChunk *)imageTableChunk bytes:(void
         _imageTableChunk = imageTableChunk;
         _bytes = bytes;
         _length = length;
+        _deallocBlocks = [[NSMutableArray alloc] init];
     }
     
     return self;
 }
 
 - (void)executeBlockOnDealloc:(dispatch_block_t)block {
-    _deallocBlock = [block copy];
+    [_deallocBlocks addObject:[block copy]];
 }
 
 - (void)dealloc {
-    if (_deallocBlock) {
-        _deallocBlock();
+    for (dispatch_block_t block in _deallocBlocks) {
+        block();
     }
 }
 
diff --git a/FastImageCache/FICWeakReference.h b/FastImageCache/FICWeakReference.h
diff --git a/FastImageCache/FICWeakReference.m b/FastImageCache/FICWeakReference.m
diff --git a/FastImageCacheDemo/FastImageCacheDemo.xcodeproj/project.pbxproj b/FastImageCacheDemo/FastImageCacheDemo.xcodeproj/project.pbxproj
@@ -20,7 +20,6 @@
 		2E1BC90117C57CDF00836A7E /* FICDViewController.m in Sources */ = {isa = PBXBuildFile; fileRef = 2E1BC8FD17C57CD300836A7E /* FICDViewController.m */; };
 		2E64541017FF24C0001D0531 /* FICDTableView.m in Sources */ = {isa = PBXBuildFile; fileRef = 2E64540F17FF24C0001D0531 /* FICDTableView.m */; };
 		2EA7994417B2A10200684B86 /* main.m in Sources */ = {isa = PBXBuildFile; fileRef = 2EA7993F17B2A10200684B86 /* main.m */; };
-		31F9BA0418822719009630D0 /* FICWeakReference.m in Sources */ = {isa = PBXBuildFile; fileRef = 31F9BA0318822719009630D0 /* FICWeakReference.m */; };
 		EB171332181F089F00EB0E24 /* CoreImage.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = EB171331181F089F00EB0E24 /* CoreImage.framework */; };
 		EB642D8017DCB1750013D644 /* FICDFullscreenPhotoDisplayController.m in Sources */ = {isa = PBXBuildFile; fileRef = EB642D7F17DCB1750013D644 /* FICDFullscreenPhotoDisplayController.m */; };
 		EB6BD53818079DAB00D762CE /* CoreGraphics.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = EB6BD53718079DAB00D762CE /* CoreGraphics.framework */; };
@@ -57,8 +56,6 @@
 		2E64540E17FF24C0001D0531 /* FICDTableView.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = FICDTableView.h; sourceTree = "<group>"; };
 		2E64540F17FF24C0001D0531 /* FICDTableView.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = FICDTableView.m; sourceTree = "<group>"; };
 		2EA7993F17B2A10200684B86 /* main.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = main.m; sourceTree = "<group>"; };
-		31F9BA0218822719009630D0 /* FICWeakReference.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = FICWeakReference.h; sourceTree = "<group>"; };
-		31F9BA0318822719009630D0 /* FICWeakReference.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = FICWeakReference.m; sourceTree = "<group>"; };
 		EB171331181F089F00EB0E24 /* CoreImage.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = CoreImage.framework; path = System/Library/Frameworks/CoreImage.framework; sourceTree = SDKROOT; };
 		EB642D3717DC9C690013D644 /* FastImageCacheDemo-Info.plist */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.xml; path = "FastImageCacheDemo-Info.plist"; sourceTree = "<group>"; };
 		EB642D7E17DCB1750013D644 /* FICDFullscreenPhotoDisplayController.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = FICDFullscreenPhotoDisplayController.h; sourceTree = "<group>"; };
@@ -148,8 +145,6 @@
 				EB6BD53918079E2D00D762CE /* FICImports.h */,
 				2E1BC8ED17C57C4700836A7E /* FICUtilities.h */,
 				2E1BC8EE17C57C4700836A7E /* FICUtilities.m */,
-				31F9BA0218822719009630D0 /* FICWeakReference.h */,
-				31F9BA0318822719009630D0 /* FICWeakReference.m */,
 			);
 			name = FastImageCache;
 			path = ../FastImageCache;
@@ -248,7 +243,6 @@
 			isa = PBXSourcesBuildPhase;
 			buildActionMask = 2147483647;
 			files = (
-				31F9BA0418822719009630D0 /* FICWeakReference.m in Sources */,
 				2EA7994417B2A10200684B86 /* main.m in Sources */,
 				2E1BC8F017C57C4700836A7E /* FICImageCache.m in Sources */,
 				2E1BC8F117C57C4700836A7E /* FICImageFormat.m in Sources */,

Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@ @interface FICImageTableEntry () {`
`21`	`21`	`FICImageTableChunk *_imageTableChunk;`
`22`	`22`	`void *_bytes;`
`23`	`23`	`size_t _length;`
`24`		`- dispatch_block_t _deallocBlock;`
	`24`	`+ NSMutableArray *_deallocBlocks;`
`25`	`25`	`}`
`26`	`26`
`27`	`27`	`@end`
`@@ -32,6 +32,7 @@ @implementation FICImageTableEntry`
`32`	`32`
`33`	`33`	`@synthesize bytes = _bytes;`
`34`	`34`	`@synthesize length = _length;`
	`35`	`+@synthesize imageTableChunk = _imageTableChunk;`
`35`	`36`
`36`	`37`	`#pragma mark - Property Accessors`
`37`	`38`
`@@ -64,18 +65,19 @@ - (id)initWithImageTableChunk:(FICImageTableChunk *)imageTableChunk bytes:(void`
`64`	`65`	`_imageTableChunk = imageTableChunk;`
`65`	`66`	`_bytes = bytes;`
`66`	`67`	`_length = length;`
	`68`	`+ _deallocBlocks = [[NSMutableArray alloc] init];`
`67`	`69`	`}`
`68`	`70`
`69`	`71`	`return self;`
`70`	`72`	`}`
`71`	`73`
`72`	`74`	`- (void)executeBlockOnDealloc:(dispatch_block_t)block {`
`73`		`- _deallocBlock = [block copy];`
	`75`	`+ [_deallocBlocks addObject:[block copy]];`
`74`	`76`	`}`
`75`	`77`
`76`	`78`	`- (void)dealloc {`
`77`		`- if (_deallocBlock) {`
`78`		`- _deallocBlock();`
	`79`	`+ for (dispatch_block_t block in _deallocBlocks) {`
	`80`	`+ block();`
`79`	`81`	`}`
`80`	`82`	`}`
`81`	`83`