From 40e864e68a73eb1ebfaf9d9aad50f14e5ff444aa Mon Sep 17 00:00:00 2001 From: Roman Gurzhiy <spec4eng@gmail.com> Date: Sat, 21 May 2016 19:25:03 +0800 Subject: [PATCH 1/2] Replace `@synchronized` with NSRecursiveLock --- Bolts/Common/BFTask.m | 190 ++++++++++++++++++++++++------------------ 1 file changed, 109 insertions(+), 81 deletions(-) diff --git a/Bolts/Common/BFTask.m b/Bolts/Common/BFTask.m index 68436ec..d7f1f00 100644 --- a/Bolts/Common/BFTask.m +++ b/Bolts/Common/BFTask.m @@ -38,7 +38,7 @@ NSString *const BFTaskMultipleExceptionsUserInfoKey = @"exceptions"; @property (nonatomic, assign, readwrite, getter=isFaulted) BOOL faulted; @property (nonatomic, assign, readwrite, getter=isCompleted) BOOL completed; -@property (nonatomic, strong) NSObject *lock; +@property (nonatomic, strong) NSRecursiveLock *lock; @property (nonatomic, strong) NSCondition *condition; @property (nonatomic, strong) NSMutableArray *callbacks; @@ -52,7 +52,7 @@ NSString *const BFTaskMultipleExceptionsUserInfoKey = @"exceptions"; self = [super init]; if (!self) return self; - _lock = [[NSObject alloc] init]; + _lock = [[NSRecursiveLock alloc] init]; _condition = [[NSCondition alloc] init]; _callbacks = [NSMutableArray array]; @@ -120,7 +120,7 @@ NSString *const BFTaskMultipleExceptionsUserInfoKey = @"exceptions"; } __block int32_t cancelled = 0; - NSObject *lock = [[NSObject alloc] init]; + NSRecursiveLock *lock = [[NSRecursiveLock alloc] init]; NSMutableArray *errors = [NSMutableArray array]; NSMutableArray *exceptions = [NSMutableArray array]; @@ -128,13 +128,13 @@ NSString *const BFTaskMultipleExceptionsUserInfoKey = @"exceptions"; for (BFTask *task in tasks) { [task continueWithBlock:^id(BFTask *task) { if (task.exception) { - @synchronized (lock) { - [exceptions addObject:task.exception]; - } + [lock lock]; + [exceptions addObject:task.exception]; + [lock unlock]; } else if (task.error) { - @synchronized (lock) { - [errors addObject:task.error]; - } + [lock lock]; + [errors addObject:task.error]; + [lock unlock]; } else if (task.cancelled) { OSAtomicIncrement32Barrier(&cancelled); } @@ -187,7 +187,7 @@ NSString *const BFTaskMultipleExceptionsUserInfoKey = @"exceptions"; __block int completed = 0; __block int32_t cancelled = 0; - NSObject *lock = [NSObject new]; + NSRecursiveLock *lock = [NSRecursiveLock new]; NSMutableArray<NSError *> *errors = [NSMutableArray new]; NSMutableArray<NSException *> *exceptions = [NSMutableArray new]; @@ -195,13 +195,13 @@ NSString *const BFTaskMultipleExceptionsUserInfoKey = @"exceptions"; for (BFTask *task in tasks) { [task continueWithBlock:^id(BFTask *task) { if (task.exception != nil) { - @synchronized(lock) { - [exceptions addObject:task.exception]; - } + [lock lock]; + [exceptions addObject:task.exception]; + [lock unlock]; } else if (task.error != nil) { - @synchronized(lock) { - [errors addObject:task.error]; - } + [lock lock]; + [errors addObject:task.error]; + [lock unlock]; } else if (task.cancelled) { OSAtomicIncrement32Barrier(&cancelled); } else { @@ -278,101 +278,126 @@ NSString *const BFTaskMultipleExceptionsUserInfoKey = @"exceptions"; #pragma mark - Custom Setters/Getters - (nullable id)result { - @synchronized(self.lock) { - return _result; - } + [self.lock lock]; + id returnValue = _result; + + [self.lock unlock]; + return returnValue; } - (BOOL)trySetResult:(nullable id)result { - @synchronized(self.lock) { - if (self.completed) { - return NO; - } + [self.lock lock]; + + BOOL returnValue = NO; + if (!self.completed) { self.completed = YES; _result = result; [self runContinuations]; - return YES; + returnValue = YES; } + + [self.lock unlock]; + return returnValue; } - (nullable NSError *)error { - @synchronized(self.lock) { - return _error; - } + [self.lock lock]; + NSError *returnValue = _error; + + [self.lock unlock]; + return returnValue; } - (BOOL)trySetError:(NSError *)error { - @synchronized(self.lock) { - if (self.completed) { - return NO; - } + [self.lock lock]; + + BOOL returnValue = NO; + if (!self.completed) { self.completed = YES; self.faulted = YES; _error = error; [self runContinuations]; - return YES; + returnValue = YES; } + + [self.lock unlock]; + return returnValue; } - (nullable NSException *)exception { - @synchronized(self.lock) { - return _exception; - } + [self.lock lock]; + NSException *returnValue = _exception; + + [self.lock unlock]; + return returnValue; } - (BOOL)trySetException:(NSException *)exception { - @synchronized(self.lock) { - if (self.completed) { - return NO; - } + [self.lock lock]; + BOOL returnValue = NO; + + if (!self.completed) { self.completed = YES; self.faulted = YES; _exception = exception; [self runContinuations]; - return YES; + returnValue = YES; } + + [self.lock unlock]; + return returnValue; + } - (BOOL)isCancelled { - @synchronized(self.lock) { - return _cancelled; - } + [self.lock lock]; + BOOL returnValue = _cancelled; + + [self.lock unlock]; + return returnValue; } - (BOOL)isFaulted { - @synchronized(self.lock) { - return _faulted; - } + [self.lock lock]; + BOOL returnValue = _faulted; + + [self.lock unlock]; + return returnValue; } - (BOOL)trySetCancelled { - @synchronized(self.lock) { - if (self.completed) { - return NO; - } + [self.lock lock]; + + BOOL returnValue = NO; + if (!self.completed) { self.completed = YES; self.cancelled = YES; [self runContinuations]; - return YES; + returnValue = YES; } + + [self.lock unlock]; + return returnValue; } - (BOOL)isCompleted { - @synchronized(self.lock) { - return _completed; - } + [self.lock lock]; + BOOL returnValue = _completed; + + [self.lock unlock]; + return returnValue; } - (void)runContinuations { - @synchronized(self.lock) { - [self.condition lock]; - [self.condition broadcast]; - [self.condition unlock]; - for (void (^callback)() in self.callbacks) { - callback(); - } - [self.callbacks removeAllObjects]; + [self.lock lock]; + [self.condition lock]; + [self.condition broadcast]; + [self.condition unlock]; + for (void (^callback)() in self.callbacks) { + callback(); } + [self.callbacks removeAllObjects]; + [self.lock unlock]; } #pragma mark - Chaining methods @@ -430,14 +455,15 @@ NSString *const BFTaskMultipleExceptionsUserInfoKey = @"exceptions"; }; BOOL completed; - @synchronized(self.lock) { - completed = self.completed; - if (!completed) { - [self.callbacks addObject:[^{ - [executor execute:executionBlock]; - } copy]]; - } + [self.lock lock]; + completed = self.completed; + if (!completed) { + [self.callbacks addObject:[^{ + [executor execute:executionBlock]; + } copy]]; } + [self.lock unlock]; + if (completed) { [executor execute:executionBlock]; } @@ -492,13 +518,15 @@ NSString *const BFTaskMultipleExceptionsUserInfoKey = @"exceptions"; if ([NSThread isMainThread]) { [self warnOperationOnMainThread]; } - - @synchronized(self.lock) { - if (self.completed) { - return; - } - [self.condition lock]; + + [self.lock lock]; + if (self.completed) { + [self.lock unlock]; + return; } + [self.condition lock]; + [self.lock unlock]; + while (!self.completed) { [self.condition wait]; } @@ -514,12 +542,12 @@ NSString *const BFTaskMultipleExceptionsUserInfoKey = @"exceptions"; BOOL faulted; NSString *resultDescription = nil; - @synchronized(self.lock) { - completed = self.completed; - cancelled = self.cancelled; - faulted = self.faulted; - resultDescription = completed ? [NSString stringWithFormat:@" result = %@", self.result] : @""; - } + [self.lock lock]; + completed = self.completed; + cancelled = self.cancelled; + faulted = self.faulted; + resultDescription = completed ? [NSString stringWithFormat:@" result = %@", self.result] : @""; + [self.lock unlock]; // Description string includes status information and, if available, the // result since in some ways this is what a promise actually "is". -- GitLab From 5de08b895f15c4e8c147f72d49296006d7f001f0 Mon Sep 17 00:00:00 2001 From: Roman Gurzhiy <spec4eng@gmail.com> Date: Mon, 23 May 2016 18:20:34 +0800 Subject: [PATCH 2/2] Replace `@synchronized` with NSRecursiveLock in BFCancellationToken and BFCancellationTokenRegistration --- Bolts/Common/BFCancellationToken.m | 62 +++++++++---------- .../Common/BFCancellationTokenRegistration.m | 25 ++++---- 2 files changed, 43 insertions(+), 44 deletions(-) diff --git a/Bolts/Common/BFCancellationToken.m b/Bolts/Common/BFCancellationToken.m index 4acd589..7d91d8b 100644 --- a/Bolts/Common/BFCancellationToken.m +++ b/Bolts/Common/BFCancellationToken.m @@ -16,7 +16,7 @@ NS_ASSUME_NONNULL_BEGIN @interface BFCancellationToken () @property (nullable, nonatomic, strong) NSMutableArray *registrations; -@property (nonatomic, strong) NSObject *lock; +@property (nonatomic, strong) NSRecursiveLock *lock; @property (nonatomic) BOOL disposed; @end @@ -40,7 +40,7 @@ NS_ASSUME_NONNULL_BEGIN if (!self) return self; _registrations = [NSMutableArray array]; - _lock = [NSObject new]; + _lock = [NSRecursiveLock new]; return self; } @@ -48,23 +48,25 @@ NS_ASSUME_NONNULL_BEGIN #pragma mark - Custom Setters/Getters - (BOOL)isCancellationRequested { - @synchronized(self.lock) { - [self throwIfDisposed]; - return _cancellationRequested; - } + [self.lock lock]; + [self throwIfDisposed]; + BOOL returnValue = _cancellationRequested; + + [self.lock unlock]; + return returnValue; } - (void)cancel { NSArray *registrations; - @synchronized(self.lock) { - [self throwIfDisposed]; - if (_cancellationRequested) { - return; - } + + [self.lock lock]; + [self throwIfDisposed]; + if (!_cancellationRequested) { [NSObject cancelPreviousPerformRequestsWithTarget:self selector:@selector(cancelPrivate) object:nil]; _cancellationRequested = YES; registrations = [self.registrations copy]; } + [self.lock unlock]; [self notifyCancellation:registrations]; } @@ -76,19 +78,18 @@ NS_ASSUME_NONNULL_BEGIN } - (BFCancellationTokenRegistration *)registerCancellationObserverWithBlock:(BFCancellationBlock)block { - @synchronized(self.lock) { - BFCancellationTokenRegistration *registration = [BFCancellationTokenRegistration registrationWithToken:self delegate:[block copy]]; - [self.registrations addObject:registration]; - - return registration; - } + [self.lock lock]; + BFCancellationTokenRegistration *registration = [BFCancellationTokenRegistration registrationWithToken:self delegate:[block copy]]; + [self.registrations addObject:registration]; + [self.lock unlock]; + return registration; } - (void)unregisterRegistration:(BFCancellationTokenRegistration *)registration { - @synchronized(self.lock) { - [self throwIfDisposed]; - [self.registrations removeObject:registration]; - } + [self.lock lock]; + [self throwIfDisposed]; + [self.registrations removeObject:registration]; + [self.lock unlock]; } // Delay on a non-public method to prevent interference with a user calling performSelector or @@ -108,29 +109,26 @@ NS_ASSUME_NONNULL_BEGIN return; } - @synchronized(self.lock) { - [self throwIfDisposed]; - [NSObject cancelPreviousPerformRequestsWithTarget:self selector:@selector(cancelPrivate) object:nil]; - if (self.cancellationRequested) { - return; - } - + [self.lock lock]; + [self throwIfDisposed]; + [NSObject cancelPreviousPerformRequestsWithTarget:self selector:@selector(cancelPrivate) object:nil]; + if (!self.cancellationRequested) { if (millis != -1) { double delay = (double)millis / 1000; [self performSelector:@selector(cancelPrivate) withObject:nil afterDelay:delay]; } } + [self.lock unlock]; } - (void)dispose { - @synchronized(self.lock) { - if (self.disposed) { - return; - } + [self.lock lock]; + if (!self.disposed) { [self.registrations makeObjectsPerformSelector:@selector(dispose)]; self.registrations = nil; self.disposed = YES; } + [self.lock unlock]; } - (void)throwIfDisposed { diff --git a/Bolts/Common/BFCancellationTokenRegistration.m b/Bolts/Common/BFCancellationTokenRegistration.m index f396c1c..3eb7708 100644 --- a/Bolts/Common/BFCancellationTokenRegistration.m +++ b/Bolts/Common/BFCancellationTokenRegistration.m @@ -18,7 +18,7 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, weak) BFCancellationToken *token; @property (nullable, nonatomic, strong) BFCancellationBlock cancellationObserverBlock; -@property (nonatomic, strong) NSObject *lock; +@property (nonatomic, strong) NSRecursiveLock *lock; @property (nonatomic) BOOL disposed; @end @@ -42,19 +42,20 @@ NS_ASSUME_NONNULL_BEGIN self = [super init]; if (!self) return self; - _lock = [NSObject new]; + _lock = [NSRecursiveLock new]; return self; } - (void)dispose { - @synchronized(self.lock) { - if (self.disposed) { - return; - } - self.disposed = YES; + [self.lock lock]; + if (self.disposed) { + [self.lock unlock]; + return; } - + self.disposed = YES; + [self.lock unlock]; + BFCancellationToken *token = self.token; if (token != nil) { [token unregisterRegistration:self]; @@ -64,10 +65,10 @@ NS_ASSUME_NONNULL_BEGIN } - (void)notifyDelegate { - @synchronized(self.lock) { - [self throwIfDisposed]; - self.cancellationObserverBlock(); - } + [self.lock lock]; + [self throwIfDisposed]; + self.cancellationObserverBlock(); + [self.lock unlock]; } - (void)throwIfDisposed { -- GitLab