From fb8d3f1e44a62cbfad085119e58d05a8279f400e Mon Sep 17 00:00:00 2001 From: Scott Goodson Date: Tue, 19 Sep 2017 21:03:42 -0700 Subject: [PATCH 1/4] [PINDiskCache] Respect small byteLimit settings by checking object size in setObject: This serves as a mechanism for some clients, like users of the Texture framework, to effectively disable the PINDiskCache by setting a byteLimit of 1. Before this change, the cache would transiently exceed the byte limit by writing the file anyway, and then immediately deleting it. This change will not affect most users of PINDiskCache, but is important for this specific use case. --- Source/PINDiskCache.m | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/Source/PINDiskCache.m b/Source/PINDiskCache.m index 9427c489..7e5ea0c4 100644 --- a/Source/PINDiskCache.m +++ b/Source/PINDiskCache.m @@ -936,8 +936,23 @@ - (void)setObject:(id )object forKey:(NSString *)key fileURL:(NSURL ** NSDataWritingOptions writeOptions = NSDataWritingAtomic; #endif - NSURL *fileURL = [self encodedFileURLForKey:key]; - + // Remain unlocked here so that we're not locked while serializing. + NSData *data = _serializer(object, key); + NSURL *fileURL = nil; + + NSUInteger byteLimit = self.byteLimit; + if (data.length <= byteLimit || byteLimit == 0) { + // The cache is large enough to fit this object (although we may need to evict others). + fileURL = [self encodedFileURLForKey:key]; + } else { + // The cache isn't large enough to fit this object (even if all others were evicted). + // We should not write it to disk because it will be deleted immediately after. + if (outFileURL) { + *outFileURL = nil; + } + return; + } + [self lock]; PINCacheObjectBlock willAddObjectBlock = self->_willAddObjectBlock; if (willAddObjectBlock) { @@ -946,13 +961,7 @@ - (void)setObject:(id )object forKey:(NSString *)key fileURL:(NSURL ** [self lock]; } - //We unlock here so that we're not locked while serializing. - [self unlock]; - NSData *data = _serializer(object, key); - [self lock]; - NSError *writeError = nil; - BOOL written = [data writeToURL:fileURL options:writeOptions error:&writeError]; PINDiskCacheError(writeError); From fe6a122a8192cf72aeb625bc71ce3460f32cbde8 Mon Sep 17 00:00:00 2001 From: Scott Goodson Date: Tue, 19 Sep 2017 21:37:10 -0700 Subject: [PATCH 2/4] [PINDiskCache] Fast-path objectForKey: when the cache is empty. --- Source/PINDiskCache.m | 2 +- Tests/PINCacheTests.m | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/PINDiskCache.m b/Source/PINDiskCache.m index 7e5ea0c4..cb57497c 100644 --- a/Source/PINDiskCache.m +++ b/Source/PINDiskCache.m @@ -838,7 +838,7 @@ - (id)objectForKeyedSubscript:(NSString *)key { NSDate *now = [[NSDate alloc] init]; - if (!key) + if (!key || (_dates.count == 0 && _sizes.count == 0)) return nil; id object = nil; diff --git a/Tests/PINCacheTests.m b/Tests/PINCacheTests.m index 2154211c..5ecb2543 100644 --- a/Tests/PINCacheTests.m +++ b/Tests/PINCacheTests.m @@ -877,7 +877,7 @@ - (void)testAsyncDiskInitialization testCache = [[PINDiskCache alloc] initWithName:cacheName]; //This should not return until *after* disk cache directory has been created - [testCache objectForKey:@"some bogus key"]; + [testCache setObject:[NSNumber numberWithInt:1] forKey:@"some bogus key"]; XCTAssertTrue([[NSFileManager defaultManager] fileExistsAtPath:[testCacheURL path]]); } From eb5b9a139744e160c579c302156349926fe94437 Mon Sep 17 00:00:00 2001 From: Scott Goodson Date: Tue, 19 Sep 2017 21:40:15 -0700 Subject: [PATCH 3/4] Fill out changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2356eb8e..f7cca1dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ * Add your own contributions to the next release on the line below this with your name. ## 3.0.1 -- Beta 5 +- [fix] Respect small byteLimit settings by checking object size in setObject: [#198](https://github.com/pinterest/PINCache/pull/198) - [new] Added an ability to set custom encoder/decoder for file names: [#192](https://github.com/pinterest/PINCache/pull/192) ## 2.2.1 -- 2016 Mar 5 From 8fd0de106a41254d64d39b6d0a159e0006b3305f Mon Sep 17 00:00:00 2001 From: Scott Goodson Date: Wed, 20 Sep 2017 19:43:11 -0700 Subject: [PATCH 4/4] Ensure lock is acquired before early return in read path; fix test. --- Source/PINDiskCache.m | 6 +++++- Tests/PINCacheTests.m | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Source/PINDiskCache.m b/Source/PINDiskCache.m index cb57497c..5070ff7f 100644 --- a/Source/PINDiskCache.m +++ b/Source/PINDiskCache.m @@ -838,7 +838,11 @@ - (id)objectForKeyedSubscript:(NSString *)key { NSDate *now = [[NSDate alloc] init]; - if (!key || (_dates.count == 0 && _sizes.count == 0)) + [self lock]; + BOOL isEmpty = (_dates.count == 0 && _sizes.count == 0); + [self unlock]; + + if (!key || isEmpty) return nil; id object = nil; diff --git a/Tests/PINCacheTests.m b/Tests/PINCacheTests.m index 5ecb2543..2154211c 100644 --- a/Tests/PINCacheTests.m +++ b/Tests/PINCacheTests.m @@ -877,7 +877,7 @@ - (void)testAsyncDiskInitialization testCache = [[PINDiskCache alloc] initWithName:cacheName]; //This should not return until *after* disk cache directory has been created - [testCache setObject:[NSNumber numberWithInt:1] forKey:@"some bogus key"]; + [testCache objectForKey:@"some bogus key"]; XCTAssertTrue([[NSFileManager defaultManager] fileExistsAtPath:[testCacheURL path]]); }