Sfoglia il codice sorgente

Stop using spinlocks

Apple engineers have pointed out that OSSpinLocks are vulnerable to live locking
on iOS in cases of priority inversion:

. http://mjtsai.com/blog/2015/12/16/osspinlock-is-unsafe/
. https://lists.swift.org/pipermail/swift-dev/Week-of-Mon-20151214/000372.html
ibireme 9 anni fa
parent
commit
bfadddd557
4 ha cambiato i file con 50 aggiunte e 65 eliminazioni
  1. 2 2
      YYCache/YYDiskCache.h
  2. 12 13
      YYCache/YYDiskCache.m
  3. 0 13
      YYCache/YYKVStorage.m
  4. 36 37
      YYCache/YYMemoryCache.m

+ 2 - 2
YYCache/YYDiskCache.h

@@ -145,7 +145,7 @@
  
  @return A new cache object, or nil if an error occurs.
  
- @warning If the cache instance for the specified path already exists in memory, 
+ @warning If the cache instance for the specified path already exists in memory,
      this method will return it directly, instead of creating a new instance.
  */
 - (instancetype)initWithPath:(NSString *)path;
@@ -165,7 +165,7 @@
  
  @return A new cache object, or nil if an error occurs.
  
- @warning If the cache instance for the specified path already exists in memory, 
+ @warning If the cache instance for the specified path already exists in memory,
      this method will return it directly, instead of creating a new instance.
  */
 - (instancetype)initWithPath:(NSString *)path

+ 12 - 13
YYCache/YYDiskCache.m

@@ -11,13 +11,12 @@
 
 #import "YYDiskCache.h"
 #import "YYKVStorage.h"
-#import <libkern/OSAtomic.h>
 #import <CommonCrypto/CommonCrypto.h>
 #import <objc/runtime.h>
 #import <time.h>
 
-#define Lock() dispatch_semaphore_wait(_lock, DISPATCH_TIME_FOREVER)
-#define Unlock() dispatch_semaphore_signal(_lock)
+#define Lock() dispatch_semaphore_wait(self->_lock, DISPATCH_TIME_FOREVER)
+#define Unlock() dispatch_semaphore_signal(self->_lock)
 
 static const int extended_data_key;
 
@@ -48,12 +47,12 @@ static NSString *_YYNSStringMD5(NSString *string) {
 
 /// weak reference for all instances
 static NSMapTable *_globalInstances;
-static OSSpinLock _globalInstancesLock;
+static dispatch_semaphore_t _globalInstancesLock;
 
 static void _YYDiskCacheInitGlobal() {
     static dispatch_once_t onceToken;
     dispatch_once(&onceToken, ^{
-        _globalInstancesLock = OS_SPINLOCK_INIT;
+        _globalInstancesLock = dispatch_semaphore_create(1);
         _globalInstances = [[NSMapTable alloc] initWithKeyOptions:NSPointerFunctionsStrongMemory valueOptions:NSPointerFunctionsWeakMemory capacity:0];
     });
 }
@@ -61,18 +60,18 @@ static void _YYDiskCacheInitGlobal() {
 static YYDiskCache *_YYDiskCacheGetGlobal(NSString *path) {
     if (path.length == 0) return nil;
     _YYDiskCacheInitGlobal();
-    OSSpinLockLock(&_globalInstancesLock);
+    dispatch_semaphore_wait(_globalInstancesLock, DISPATCH_TIME_FOREVER);
     id cache = [_globalInstances objectForKey:path];
-    OSSpinLockUnlock(&_globalInstancesLock);
+    dispatch_semaphore_signal(_globalInstancesLock);
     return cache;
 }
 
 static void _YYDiskCacheSetGlobal(YYDiskCache *cache) {
     if (cache.path.length == 0) return;
     _YYDiskCacheInitGlobal();
-    OSSpinLockLock(&_globalInstancesLock);
+    dispatch_semaphore_wait(_globalInstancesLock, DISPATCH_TIME_FOREVER);
     [_globalInstances setObject:cache forKey:cache.path];
-    OSSpinLockUnlock(&_globalInstancesLock);
+    dispatch_semaphore_signal(_globalInstancesLock);
 }
 
 
@@ -98,12 +97,12 @@ static void _YYDiskCacheSetGlobal(YYDiskCache *cache) {
     dispatch_async(_queue, ^{
         __strong typeof(_self) self = _self;
         if (!self) return;
-        dispatch_semaphore_wait(self->_lock, DISPATCH_TIME_FOREVER);
+        Lock();
         [self _trimToCost:self.costLimit];
         [self _trimToCount:self.countLimit];
         [self _trimToAge:self.ageLimit];
         [self _trimToFreeDiskSpace:self.freeDiskSpaceLimit];
-        dispatch_semaphore_signal(self->_lock);
+        Unlock();
     });
 }
 
@@ -330,9 +329,9 @@ static void _YYDiskCacheSetGlobal(YYDiskCache *cache) {
             if (end) end(YES);
             return;
         }
-        dispatch_semaphore_wait(self->_lock, DISPATCH_TIME_FOREVER);
+        Lock();
         [_kv removeAllItemsWithProgressBlock:progress endBlock:end];
-        dispatch_semaphore_signal(self->_lock);
+        Unlock();
     });
 }
 

+ 0 - 13
YYCache/YYKVStorage.m

@@ -11,7 +11,6 @@
 
 #import "YYKVStorage.h"
 #import <UIKit/UIKit.h>
-#import <libkern/OSAtomic.h>
 #import <time.h>
 
 #if __has_include(<sqlite3.h>)
@@ -58,7 +57,6 @@ static NSString *const kTrashDirectoryName = @"trash";
     
     BOOL _invalidated; ///< If YES, then the db should not open again, all read/write should be ignored.
     BOOL _dbIsClosing; ///< If YES, then the db is during closing.
-    OSSpinLock _dbStateLock;
 }
 
 
@@ -66,7 +64,6 @@ static NSString *const kTrashDirectoryName = @"trash";
 
 - (BOOL)_dbOpen {
     BOOL shouldOpen = YES;
-    OSSpinLockLock(&_dbStateLock);
     if (_invalidated) {
         shouldOpen = NO;
     } else if (_dbIsClosing) {
@@ -74,7 +71,6 @@ static NSString *const kTrashDirectoryName = @"trash";
     } else if (_db){
         shouldOpen = NO;
     }
-    OSSpinLockUnlock(&_dbStateLock);
     if (!shouldOpen) return YES;
     
     int result = sqlite3_open(_dbPath.UTF8String, &_db);
@@ -91,7 +87,6 @@ static NSString *const kTrashDirectoryName = @"trash";
 
 - (BOOL)_dbClose {
     BOOL needClose = YES;
-    OSSpinLockLock(&_dbStateLock);
     if (!_db) {
         needClose = NO;
     } else if (_invalidated) {
@@ -101,7 +96,6 @@ static NSString *const kTrashDirectoryName = @"trash";
     } else {
         _dbIsClosing = YES;
     }
-    OSSpinLockUnlock(&_dbStateLock);
     if (!needClose) return YES;
     
     int  result = 0;
@@ -128,11 +122,7 @@ static NSString *const kTrashDirectoryName = @"trash";
         }
     } while (retry);
     _db = NULL;
-    
-    OSSpinLockLock(&_dbStateLock);
     _dbIsClosing = NO;
-    OSSpinLockUnlock(&_dbStateLock);
-    
     return YES;
 }
 
@@ -646,9 +636,7 @@ static NSString *const kTrashDirectoryName = @"trash";
 }
 
 - (void)_appWillBeTerminated {
-    OSSpinLockLock(&_dbStateLock);
     _invalidated = YES;
-    OSSpinLockUnlock(&_dbStateLock);
 }
 
 #pragma mark - public
@@ -675,7 +663,6 @@ static NSString *const kTrashDirectoryName = @"trash";
     _trashPath = [path stringByAppendingPathComponent:kTrashDirectoryName];
     _trashQueue = dispatch_queue_create("com.ibireme.cache.disk.trash", DISPATCH_QUEUE_SERIAL);
     _dbPath = [path stringByAppendingPathComponent:kDBFileName];
-    _dbStateLock = OS_SPINLOCK_INIT;
     _errorLogsEnabled = YES;
     NSError *error = nil;
     if (![[NSFileManager defaultManager] createDirectoryAtPath:path

+ 36 - 37
YYCache/YYMemoryCache.m

@@ -13,7 +13,6 @@
 #import <UIKit/UIKit.h>
 #import <CoreFoundation/CoreFoundation.h>
 #import <QuartzCore/QuartzCore.h>
-#import <libkern/OSAtomic.h>
 #import <pthread.h>
 
 #if __has_include("YYDispatchQueuePool.h")
@@ -183,7 +182,7 @@ static inline dispatch_queue_t YYMemoryCacheGetReleaseQueue() {
 
 
 @implementation YYMemoryCache {
-    OSSpinLock _lock;
+    pthread_mutex_t _lock;
     _YYLinkedMap *_lru;
     dispatch_queue_t _queue;
 }
@@ -208,26 +207,26 @@ static inline dispatch_queue_t YYMemoryCacheGetReleaseQueue() {
 
 - (void)_trimToCost:(NSUInteger)costLimit {
     BOOL finish = NO;
-    OSSpinLockLock(&_lock);
+    pthread_mutex_lock(&_lock);
     if (costLimit == 0) {
         [_lru removeAll];
         finish = YES;
     } else if (_lru->_totalCost <= costLimit) {
         finish = YES;
     }
-    OSSpinLockUnlock(&_lock);
+    pthread_mutex_unlock(&_lock);
     if (finish) return;
     
     NSMutableArray *holder = [NSMutableArray new];
     while (!finish) {
-        if (OSSpinLockTry(&_lock)) {
+        if (pthread_mutex_trylock(&_lock) == 0) {
             if (_lru->_totalCost > costLimit) {
                 _YYLinkedMapNode *node = [_lru removeTailNode];
                 if (node) [holder addObject:node];
             } else {
                 finish = YES;
             }
-            OSSpinLockUnlock(&_lock);
+            pthread_mutex_unlock(&_lock);
         } else {
             usleep(10 * 1000); //10 ms
         }
@@ -242,26 +241,26 @@ static inline dispatch_queue_t YYMemoryCacheGetReleaseQueue() {
 
 - (void)_trimToCount:(NSUInteger)countLimit {
     BOOL finish = NO;
-    OSSpinLockLock(&_lock);
+    pthread_mutex_lock(&_lock);
     if (countLimit == 0) {
         [_lru removeAll];
         finish = YES;
     } else if (_lru->_totalCount <= countLimit) {
         finish = YES;
     }
-    OSSpinLockUnlock(&_lock);
+    pthread_mutex_unlock(&_lock);
     if (finish) return;
     
     NSMutableArray *holder = [NSMutableArray new];
     while (!finish) {
-        if (OSSpinLockTry(&_lock)) {
+        if (pthread_mutex_trylock(&_lock) == 0) {
             if (_lru->_totalCount > countLimit) {
                 _YYLinkedMapNode *node = [_lru removeTailNode];
                 if (node) [holder addObject:node];
             } else {
                 finish = YES;
             }
-            OSSpinLockUnlock(&_lock);
+            pthread_mutex_unlock(&_lock);
         } else {
             usleep(10 * 1000); //10 ms
         }
@@ -277,26 +276,26 @@ static inline dispatch_queue_t YYMemoryCacheGetReleaseQueue() {
 - (void)_trimToAge:(NSTimeInterval)ageLimit {
     BOOL finish = NO;
     NSTimeInterval now = CACurrentMediaTime();
-    OSSpinLockLock(&_lock);
+    pthread_mutex_lock(&_lock);
     if (ageLimit <= 0) {
         [_lru removeAll];
         finish = YES;
     } else if (!_lru->_tail || (now - _lru->_tail->_time) <= ageLimit) {
         finish = YES;
     }
-    OSSpinLockUnlock(&_lock);
+    pthread_mutex_unlock(&_lock);
     if (finish) return;
     
     NSMutableArray *holder = [NSMutableArray new];
     while (!finish) {
-        if (OSSpinLockTry(&_lock)) {
+        if (pthread_mutex_trylock(&_lock) == 0) {
             if (_lru->_tail && (now - _lru->_tail->_time) > ageLimit) {
                 _YYLinkedMapNode *node = [_lru removeTailNode];
                 if (node) [holder addObject:node];
             } else {
                 finish = YES;
             }
-            OSSpinLockUnlock(&_lock);
+            pthread_mutex_unlock(&_lock);
         } else {
             usleep(10 * 1000); //10 ms
         }
@@ -331,7 +330,7 @@ static inline dispatch_queue_t YYMemoryCacheGetReleaseQueue() {
 
 - (instancetype)init {
     self = super.init;
-    _lock = OS_SPINLOCK_INIT;
+    pthread_mutex_init(&_lock, NULL);
     _lru = [_YYLinkedMap new];
     _queue = dispatch_queue_create("com.ibireme.cache.memory", DISPATCH_QUEUE_SERIAL);
     
@@ -356,62 +355,62 @@ static inline dispatch_queue_t YYMemoryCacheGetReleaseQueue() {
 }
 
 - (NSUInteger)totalCount {
-    OSSpinLockLock(&_lock);
+    pthread_mutex_lock(&_lock);
     NSUInteger count = _lru->_totalCount;
-    OSSpinLockUnlock(&_lock);
+    pthread_mutex_unlock(&_lock);
     return count;
 }
 
 - (NSUInteger)totalCost {
-    OSSpinLockLock(&_lock);
+    pthread_mutex_lock(&_lock);
     NSUInteger totalCost = _lru->_totalCost;
-    OSSpinLockUnlock(&_lock);
+    pthread_mutex_unlock(&_lock);
     return totalCost;
 }
 
 - (BOOL)releaseInMainThread {
-    OSSpinLockLock(&_lock);
+    pthread_mutex_lock(&_lock);
     BOOL releaseInMainThread = _lru->_releaseOnMainThread;
-    OSSpinLockUnlock(&_lock);
+    pthread_mutex_unlock(&_lock);
     return releaseInMainThread;
 }
 
 - (void)setReleaseInMainThread:(BOOL)releaseInMainThread {
-    OSSpinLockLock(&_lock);
+    pthread_mutex_lock(&_lock);
     _lru->_releaseOnMainThread = releaseInMainThread;
-    OSSpinLockUnlock(&_lock);
+    pthread_mutex_unlock(&_lock);
 }
 
 - (BOOL)releaseAsynchronously {
-    OSSpinLockLock(&_lock);
+    pthread_mutex_lock(&_lock);
     BOOL releaseAsynchronously = _lru->_releaseAsynchronously;
-    OSSpinLockUnlock(&_lock);
+    pthread_mutex_unlock(&_lock);
     return releaseAsynchronously;
 }
 
 - (void)setReleaseAsynchronously:(BOOL)releaseAsynchronously {
-    OSSpinLockLock(&_lock);
+    pthread_mutex_lock(&_lock);
     _lru->_releaseAsynchronously = releaseAsynchronously;
-    OSSpinLockUnlock(&_lock);
+    pthread_mutex_unlock(&_lock);
 }
 
 - (BOOL)containsObjectForKey:(id)key {
     if (!key) return NO;
-    OSSpinLockLock(&_lock);
+    pthread_mutex_lock(&_lock);
     BOOL contains = CFDictionaryContainsKey(_lru->_dic, (__bridge const void *)(key));
-    OSSpinLockUnlock(&_lock);
+    pthread_mutex_unlock(&_lock);
     return contains;
 }
 
 - (id)objectForKey:(id)key {
     if (!key) return nil;
-    OSSpinLockLock(&_lock);
+    pthread_mutex_lock(&_lock);
     _YYLinkedMapNode *node = CFDictionaryGetValue(_lru->_dic, (__bridge const void *)(key));
     if (node) {
         node->_time = CACurrentMediaTime();
         [_lru bringNodeToHead:node];
     }
-    OSSpinLockUnlock(&_lock);
+    pthread_mutex_unlock(&_lock);
     return node ? node->_value : nil;
 }
 
@@ -425,7 +424,7 @@ static inline dispatch_queue_t YYMemoryCacheGetReleaseQueue() {
         [self removeObjectForKey:key];
         return;
     }
-    OSSpinLockLock(&_lock);
+    pthread_mutex_lock(&_lock);
     _YYLinkedMapNode *node = CFDictionaryGetValue(_lru->_dic, (__bridge const void *)(key));
     NSTimeInterval now = CACurrentMediaTime();
     if (node) {
@@ -461,12 +460,12 @@ static inline dispatch_queue_t YYMemoryCacheGetReleaseQueue() {
             });
         }
     }
-    OSSpinLockUnlock(&_lock);
+    pthread_mutex_unlock(&_lock);
 }
 
 - (void)removeObjectForKey:(id)key {
     if (!key) return;
-    OSSpinLockLock(&_lock);
+    pthread_mutex_lock(&_lock);
     _YYLinkedMapNode *node = CFDictionaryGetValue(_lru->_dic, (__bridge const void *)(key));
     if (node) {
         [_lru removeNode:node];
@@ -481,13 +480,13 @@ static inline dispatch_queue_t YYMemoryCacheGetReleaseQueue() {
             });
         }
     }
-    OSSpinLockUnlock(&_lock);
+    pthread_mutex_unlock(&_lock);
 }
 
 - (void)removeAllObjects {
-    OSSpinLockLock(&_lock);
+    pthread_mutex_lock(&_lock);
     [_lru removeAll];
-    OSSpinLockUnlock(&_lock);
+    pthread_mutex_unlock(&_lock);
 }
 
 - (void)trimToCount:(NSUInteger)count {