Bugfix/issue 1745 simplify locking

SmartDeviceLink/SDLChoiceSetManager.m

@@ -23,6 +23,7 @@
 #import "SDLGlobals.h"
 #import "SDLHMILevel.h"
 #import "SDLKeyboardProperties.h"
+#import "SDLLockedMutableSet.h"
 #import "SDLLogMacros.h"
 #import "SDLOnHMIStatus.h"
 #import "SDLPerformInteraction.h"
@@ -69,9 +70,9 @@ @interface SDLChoiceSetManager()
 @property (copy, nonatomic, nullable) SDLHMILevel currentHMILevel;
 @property (copy, nonatomic, nullable) SDLWindowCapability *currentWindowCapability;
 
-@property (strong, nonatomic) NSMutableSet<SDLChoiceCell *> *preloadedMutableChoices;
+@property (strong, nonatomic) SDLLockedMutableSet<SDLChoiceCell *> *preloadedMutableChoices;
 @property (strong, nonatomic, readonly) NSSet<SDLChoiceCell *> *pendingPreloadChoices;
-@property (strong, nonatomic) NSMutableSet<SDLChoiceCell *> *pendingMutablePreloadChoices;
+@property (strong, nonatomic) SDLLockedMutableSet<SDLChoiceCell *> *pendingMutablePreloadChoices;
 @property (strong, nonatomic, nullable) SDLChoiceSet *pendingPresentationSet;
 @property (strong, nonatomic, nullable) SDLAsynchronousOperation *pendingPresentOperation;
 
@@ -104,8 +105,8 @@ - (instancetype)initWithConnectionManager:(id<SDLConnectionManagerType>)connecti
         _readWriteQueue = [SDLGlobals sharedGlobals].sdlProcessingQueue;
     }
 
-    _preloadedMutableChoices = [NSMutableSet set];
-    _pendingMutablePreloadChoices = [NSMutableSet set];
+    _preloadedMutableChoices = [[SDLLockedMutableSet alloc] initWithQueue:_readWriteQueue];
+    _pendingMutablePreloadChoices = [[SDLLockedMutableSet alloc] initWithQueue:_readWriteQueue];
 
     _nextChoiceId = ChoiceCellIdMin;
     _nextCancelId = ChoiceCellCancelIdMin;
@@ -181,8 +182,8 @@ - (void)didEnterStateShutdown {
 
     [self.transactionQueue cancelAllOperations];
     self.transactionQueue = [self sdl_newTransactionQueue];
-    _preloadedMutableChoices = [NSMutableSet set];
-    _pendingMutablePreloadChoices = [NSMutableSet set];
+    [_preloadedMutableChoices removeAllObjects];
+    [_pendingMutablePreloadChoices removeAllObjects];
     _pendingPresentationSet = nil;
 
     _vrOptional = YES;
@@ -230,11 +231,8 @@ - (void)preloadChoices:(NSArray<SDLChoiceCell *> *)choices withCompletionHandler
     }
 
     NSMutableSet<SDLChoiceCell *> *choicesToUpload = [[self sdl_choicesToBeUploadedWithArray:choices] mutableCopy];
-
-    [self sdl_runSyncOnQueue:^{
-        [choicesToUpload minusSet:self.preloadedMutableChoices];
-        [choicesToUpload minusSet:self.pendingMutablePreloadChoices];
-    }];
+    [choicesToUpload minusSet:self.preloadedMutableChoices.immutableSet];
+    [choicesToUpload minusSet:self.pendingMutablePreloadChoices.immutableSet];
 
     if (choicesToUpload.count == 0) {
         SDLLogD(@"All choices already preloaded. No need to perform a preload");
@@ -248,9 +246,7 @@ - (void)preloadChoices:(NSArray<SDLChoiceCell *> *)choices withCompletionHandler
     [self sdl_updateIdsOnChoices:choicesToUpload];
 
     // Add the preload cells to the pending preloads
-    [self sdl_runSyncOnQueue:^{
-        [self.pendingMutablePreloadChoices unionSet:choicesToUpload];
-    }];
+    [self.pendingMutablePreloadChoices unionSet:choicesToUpload];
 
     // Upload pending preloads
     // For backward compatibility with Gen38Inch display type head units
@@ -275,11 +271,8 @@ - (void)preloadChoices:(NSArray<SDLChoiceCell *> *)choices withCompletionHandler
             return;
         }
 
-        [strongSelf sdl_runSyncOnQueue:^{
-            __strong typeof(weakSelf) strongSelf = weakSelf;
-            [strongSelf.preloadedMutableChoices unionSet:choicesToUpload];
-            [strongSelf.pendingMutablePreloadChoices minusSet:choicesToUpload];
-        }];
+        [strongSelf.preloadedMutableChoices unionSet:choicesToUpload];
+        [strongSelf.pendingMutablePreloadChoices minusSet:choicesToUpload];
     };
     [self.transactionQueue addOperation:preloadOp];
 }
@@ -307,9 +300,7 @@ - (void)deleteChoices:(NSArray<SDLChoiceCell *> *)choices {
     }
 
     // Remove the cells from pending and delete choices
-    [self sdl_runSyncOnQueue:^{
-        [self.pendingMutablePreloadChoices minusSet:cellsToBeRemovedFromPending];
-    }];
+    [self.pendingMutablePreloadChoices minusSet:cellsToBeRemovedFromPending];
 
     for (SDLAsynchronousOperation *op in self.transactionQueue.operations) {
         if (![op isMemberOfClass:[SDLPreloadChoicesOperation class]]) { continue; }
@@ -341,10 +332,7 @@ - (void)deleteChoices:(NSArray<SDLChoiceCell *> *)choices {
             return;
         }
 
-        [strongSelf sdl_runSyncOnQueue:^{
-            __strong typeof(weakSelf) strongSelf = weakSelf;
-            [strongSelf.preloadedMutableChoices minusSet:cellsToBeDeleted];
-        }];
+        [strongSelf.preloadedMutableChoices minusSet:cellsToBeDeleted];
     };
     [self.transactionQueue addOperation:deleteOp];
 }
@@ -521,21 +509,11 @@ - (SDLKeyboardProperties *)sdl_defaultKeyboardConfiguration {
 #pragma mark - Getters
 
 - (NSSet<SDLChoiceCell *> *)preloadedChoices {
-    __block NSSet<SDLChoiceCell *> *set = nil;
-    [self sdl_runSyncOnQueue:^{
-        set = [self->_preloadedMutableChoices copy];
-    }];
-
-    return set;
+    return self.preloadedMutableChoices.immutableSet;
 }
 
 - (NSSet<SDLChoiceCell *> *)pendingPreloadChoices {
-    __block NSSet<SDLChoiceCell *> *set = nil;
-    [self sdl_runSyncOnQueue:^{
-        set = [self->_pendingMutablePreloadChoices copy];
-    }];
-
-    return set;
+    return self.pendingMutablePreloadChoices.immutableSet;
 }
 
 - (UInt16)nextChoiceId {

SmartDeviceLink/SDLLockedDictionary.m

@@ -0,0 +1,32 @@
+//
+//  SDLLockedDictionary.m
+//  SmartDeviceLink
+//
+//  Created by Joel Fischer on 8/4/20.
+//  Copyright © 2020 smartdevicelink. All rights reserved.
+//
+
+#import "SDLLockedMutableDictionary.h"
+
+@interface SDLLockedMutableDictionary ()
+
+@property (strong, nonatomic) NSDictionary *internalDict;
+
+@end
+
+@implementation SDLLockedMutableDictionary
+
+- (instancetype)init {
+    self = [super init];
+    if (!self) { return nil; }
+
+    _internalDict = [[NSDictionary alloc] init];
+
+    return self;
+}
+
+- (void)setObject:(ObjectType)object forKey:(KeyType <NSCopying>)key {
+
+}
+
+@end

SmartDeviceLink/SDLLockedMapTable.h

@@ -0,0 +1,63 @@
+//
+//  SDLLockedMapTable.h
+//  SmartDeviceLink
+//
+//  Created by Joel Fischer on 8/6/20.
+//  Copyright © 2020 smartdevicelink. All rights reserved.
+//
+
+#import <Foundation/Foundation.h>
+
+NS_ASSUME_NONNULL_BEGIN
+
+@interface SDLLockedMapTable<KeyType, ObjectType> : NSObject
+
+#pragma mark - Initializers
+- (instancetype)init NS_UNAVAILABLE;
+
+/// Create a new locked mutable dictionary with a given dispatch queue. All calls will be reader/writer locked on the queue so that only one operation will occur at a time.
+///
+/// @param keyOptions A bit field that specifies the options for the keys in the map table. For possible values, see NSMapTableOptions.
+/// @param valueOptions A bit field that specifies the options for the values in the map table. For possible values, see NSMapTableOptions.
+/// @param queue The queue to use. It can be either a serial or concurrent queue.
+- (instancetype)initWithKeyOptions:(NSPointerFunctionsOptions)keyOptions valueOptions:(NSPointerFunctionsOptions)valueOptions queue:(dispatch_queue_t)queue;
+
+#pragma mark - Removing
+
+/// Empties the map table of its entries.
+- (void)removeAllObjects;
+
+/// Removes a given key and its associated value from the map table.
+/// @param aKey The key to remove.
+- (void)removeObjectForKey:(nullable KeyType)aKey;
+
+#pragma mark - Getting / Setting
+
+/// Returns a the value associated with a given key.
+/// @param aKey The key for which to return the corresponding value.
+/// @return The value associated with aKey, or nil if no value is associated with aKey.
+- (nullable ObjectType)objectForKey:(nullable KeyType)aKey;
+
+/// Adds a given key-value pair to the map table.
+/// @param anObject The value for aKey
+/// @param aKey The key for anObject
+- (void)setObject:(nullable ObjectType)anObject forKey:(nullable KeyType)aKey;
+
+#pragma mark Retrieving Information
+
+/// The number of objects in the dictionary.
+///
+/// This will occur synchronously and will not return until the operation completes.
+/// @return The number of objects in the dictionary.
+- (NSUInteger)count;
+
+/// Returns a dictionary representation of the map table.
+- (NSDictionary<KeyType, ObjectType> *)dictionaryRepresentation;
+
+#pragma mark Subscripting
+- (void)setObject:(nullable __kindof ObjectType)object forKeyedSubscript:(KeyType<NSCopying>)key;
+- (nullable __kindof ObjectType)objectForKeyedSubscript:(KeyType<NSCopying>)key;
+
+@end
+
+NS_ASSUME_NONNULL_END

SmartDeviceLink/SDLLockedMapTable.m

@@ -0,0 +1,121 @@
+//
+//  SDLLockedMapTable.m
+//  SmartDeviceLink
+//
+//  Created by Joel Fischer on 8/6/20.
+//  Copyright © 2020 smartdevicelink. All rights reserved.
+//
+
+#import "SDLLockedMapTable.h"
+
+@interface SDLLockedMapTable ()
+
+@property (assign, nonatomic) dispatch_queue_t internalQueue;
+@property (assign, nonatomic) const char *internalQueueID;
+
+@property (strong, nonatomic) NSMapTable *internalMapTable;
+
+@end
+
+@implementation SDLLockedMapTable
+
+#pragma mark - Initializers
+
+- (instancetype)initWithKeyOptions:(NSPointerFunctionsOptions)keyOptions valueOptions:(NSPointerFunctionsOptions)valueOptions queue:(dispatch_queue_t)queue {
+    self = [super init];
+    if (!self) { return nil; }
+
+    _internalQueue = queue;
+    _internalQueueID = [[NSUUID alloc] init].UUIDString.UTF8String;
+    dispatch_queue_set_specific(_internalQueue, _internalQueueID, (void *)_internalQueueID, NULL);
+
+    _internalMapTable = [NSMapTable mapTableWithKeyOptions:keyOptions valueOptions:valueOptions];
+
+    return self;
+}
+
+#pragma mark - Removing
+
+- (void)removeAllObjects {
+    __weak typeof(self) weakSelf = self;
+    [self sdl_runAsyncWithBlock:^{
+        [weakSelf.internalMapTable removeAllObjects];
+    }];
+}
+
+- (void)removeObjectForKey:(id<NSCopying>)key {
+    __weak typeof(self) weakSelf = self;
+    [self sdl_runAsyncWithBlock:^{
+        if ([weakSelf objectForKey:key] != nil) {
+            [weakSelf.internalMapTable removeObjectForKey:key];
+        }
+    }];
+}
+
+#pragma mark - Getting / Setting
+
+- (id)objectForKey:(id<NSCopying>)key {
+    __block id retVal = nil;
+    [self sdl_runSyncWithBlock:^{
+        retVal = [self.internalMapTable objectForKey:key];
+    }];
+
+    return retVal;
+}
+
+- (void)setObject:(id)object forKey:(id<NSCopying>)key {
+    __weak typeof(self) weakSelf = self;
+    [self sdl_runAsyncWithBlock:^{
+        [weakSelf.internalMapTable setObject:object forKey:key];
+    }];
+}
+
+#pragma mark Subscripting
+
+- (id)objectForKeyedSubscript:(id<NSCopying>)key {
+    return [self.internalMapTable objectForKey:key];
+}
+
+- (void)setObject:(id)object forKeyedSubscript:(id<NSCopying>)key {
+    [self.internalMapTable setObject:object forKey:key];
+}
+
+#pragma mark Retrieving Information
+
+- (NSUInteger)count {
+    __block NSUInteger retVal = 0;
+    [self sdl_runSyncWithBlock:^{
+        retVal = self.internalMapTable.count;
+    }];
+
+    return retVal;
+}
+
+- (NSDictionary *)dictionaryRepresentation {
+    __block NSDictionary *retVal = nil;
+    [self sdl_runSyncWithBlock:^{
+        retVal = self.internalMapTable.dictionaryRepresentation;
+    }];
+
+    return retVal;
+}
+
+# pragma mark - Utilities
+
+- (void)sdl_runSyncWithBlock:(void (^)(void))block {
+    if (dispatch_get_specific(_internalQueueID) != NULL) {
+        block();
+    } else {
+        dispatch_sync(_internalQueue, block);
+    }
+}
+
+- (void)sdl_runAsyncWithBlock:(void (^)(void))block {
+    if (dispatch_get_specific(_internalQueueID) != NULL) {
+        block();
+    } else {
+        dispatch_barrier_async(_internalQueue, block);
+    }
+}
+
+@end