Skip to content

Commit

Permalink
Remove locking on callback (#97)
Browse files Browse the repository at this point in the history
* Restructuring blocks to avoid holding lock in callback.

* Remove locking on all callbacks except for enumeration and fileURL requests. Add warnings about deadlock.

* Updating docs

* indicate which functions should be called with lock held / more removing of locks on callback
  • Loading branch information
garrettmoon authored Jul 22, 2016
1 parent 569df93 commit 58635e4
Show file tree
Hide file tree
Showing 49 changed files with 1,167 additions and 10,570 deletions.
10 changes: 4 additions & 6 deletions PINCache/PINCache.m
Original file line number Diff line number Diff line change
Expand Up @@ -110,16 +110,14 @@ - (void)objectForKey:(NSString *)key block:(PINCacheObjectBlock)block
return;

if (memoryCacheObject) {
[strongSelf->_diskCache fileURLForKey:memoryCacheKey block:^(PINDiskCache *diskCache, NSString *diskCacheKey, id <NSCoding> diskCacheObject, NSURL *fileURL) {
// update the access time on disk
}];
[strongSelf->_diskCache fileURLForKey:memoryCacheKey block:NULL];
dispatch_async(strongSelf->_concurrentQueue, ^{
PINCache *strongSelf = weakSelf;
if (strongSelf)
block(strongSelf, memoryCacheKey, memoryCacheObject);
});
} else {
[strongSelf->_diskCache objectForKey:memoryCacheKey block:^(PINDiskCache *diskCache, NSString *diskCacheKey, id <NSCoding> diskCacheObject, NSURL *fileURL) {
[strongSelf->_diskCache objectForKey:memoryCacheKey block:^(PINDiskCache *diskCache, NSString *diskCacheKey, id <NSCoding> diskCacheObject) {
PINCache *strongSelf = weakSelf;
if (!strongSelf)
return;
Expand Down Expand Up @@ -158,7 +156,7 @@ - (void)setObject:(id <NSCoding>)object forKey:(NSString *)key block:(PINCacheOb
dispatch_group_leave(group);
};

diskBlock = ^(PINDiskCache *diskCache, NSString *diskCacheKey, id <NSCoding> memoryCacheObject, NSURL *memoryCacheFileURL) {
diskBlock = ^(PINDiskCache *diskCache, NSString *diskCacheKey, id <NSCoding> memoryCacheObject) {
dispatch_group_leave(group);
};
}
Expand Down Expand Up @@ -198,7 +196,7 @@ - (void)removeObjectForKey:(NSString *)key block:(PINCacheObjectBlock)block
dispatch_group_leave(group);
};

diskBlock = ^(PINDiskCache *diskCache, NSString *diskCacheKey, id <NSCoding> memoryCacheObject, NSURL *memoryCacheFileURL) {
diskBlock = ^(PINDiskCache *diskCache, NSString *diskCacheKey, id <NSCoding> memoryCacheObject) {
dispatch_group_leave(group);
};
}
Expand Down
33 changes: 27 additions & 6 deletions PINCache/PINDiskCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@ typedef void (^PINDiskCacheBlock)(PINDiskCache *cache);
/**
A callback block which provides the cache, key and object as arguments
*/
typedef void (^PINDiskCacheObjectBlock)(PINDiskCache *cache, NSString *key, id <NSCoding> __nullable object, NSURL * __nullable fileURL);
typedef void (^PINDiskCacheObjectBlock)(PINDiskCache *cache, NSString *key, id <NSCoding> __nullable object);

/**
A callback block which provides the key and fileURL of the object
*/
typedef void (^PINDiskCacheFileURLBlock)(NSString *key, NSURL * __nullable fileURL);

/**
A callback block which provides a BOOL value as argument
Expand Down Expand Up @@ -227,8 +232,6 @@ typedef void (^PINDiskCacheContainsBlock)(BOOL containsObject);
Retrieves the object for the specified key. This method returns immediately and executes the passed
block as soon as the object is available.
@warning The fileURL is only valid for the duration of this block, do not use it after the block ends.
@param key The key associated with the requested object.
@param block A block to be executed serially when the object is available.
*/
Expand All @@ -241,10 +244,15 @@ typedef void (^PINDiskCacheContainsBlock)(BOOL containsObject);
@warning Access is protected for the duration of the block, but to maintain safe disk access do not
access this fileURL after the block has ended.
@warning The PINDiskCache lock is held while block is executed. Any synchronous calls to the diskcache
or a cache which owns the instance of the disk cache are likely to cause a deadlock. This is why the block is
*not* passed the instance of the disk cache. You should also avoid doing extensive work while this
lock is held.
@param key The key associated with the requested object.
@param block A block to be executed serially when the file URL is available.
*/
- (void)fileURLForKey:(nullable NSString *)key block:(nullable PINDiskCacheObjectBlock)block;
- (void)fileURLForKey:(NSString *)key block:(nullable PINDiskCacheFileURLBlock)block;

/**
Stores an object in the cache for the specified key. This method returns immediately and executes the
Expand Down Expand Up @@ -308,8 +316,14 @@ typedef void (^PINDiskCacheContainsBlock)(BOOL containsObject);
@param block A block to be executed for every object in the cache.
@param completionBlock An optional block to be executed after the enumeration is complete.
@warning The PINDiskCache lock is held while block is executed. Any synchronous calls to the diskcache
or a cache which owns the instance of the disk cache are likely to cause a deadlock. This is why the block is
*not* passed the instance of the disk cache. You should also avoid doing extensive work while this
lock is held.
*/
- (void)enumerateObjectsWithBlock:(PINDiskCacheObjectBlock)block completionBlock:(nullable PINDiskCacheBlock)completionBlock;
- (void)enumerateObjectsWithBlock:(PINDiskCacheFileURLBlock)block completionBlock:(nullable PINDiskCacheBlock)completionBlock;

#pragma mark -
/// @name Synchronous Methods
Expand Down Expand Up @@ -404,10 +418,17 @@ typedef void (^PINDiskCacheContainsBlock)(BOOL containsObject);
read from disk, the `object` parameter of the block will be `nil` but the `fileURL` will be available.
This method blocks the calling thread until all objects have been enumerated.
@param block A block to be executed for every object in the cache.
@warning Do not call this method within the event blocks (<didRemoveObjectBlock>, etc.)
Instead use the asynchronous version, <enumerateObjectsWithBlock:completionBlock:>.
@warning The PINDiskCache lock is held while block is executed. Any synchronous calls to the diskcache
or a cache which owns the instance of the disk cache are likely to cause a deadlock. This is why the block is
*not* passed the instance of the disk cache. You should also avoid doing extensive work while this
lock is held.
*/
- (void)enumerateObjectsWithBlock:(nullable PINDiskCacheObjectBlock)block;
- (void)enumerateObjectsWithBlock:(nullable PINDiskCacheFileURLBlock)block;

@end

Expand Down
Loading

0 comments on commit 58635e4

Please sign in to comment.