diff options
author | Konstantin Varlamov <var-const@users.noreply.github.com> | 2018-04-11 15:17:49 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-04-11 15:17:49 -0400 |
commit | 9f02fa6b2ee1bfac85b20907693c0afdb1d274b5 (patch) | |
tree | bb4612ef85ff77b87b9779a69cccaa35ddd52f42 /Firestore/Source/Util/FSTDispatchQueue.mm | |
parent | 14bcf8ff741c96c76367c5c0afe92be90c2c452f (diff) |
iOS: fix data race in FSTDispatchQueue. (#1070)
operationInProgress is accessed from both the main thread and from
libdispatch on some other thread. Make it atomic to avoid a data race.
Also reorder assertion checks to only access operationInProgress
after making sure the function is running on the queue.
Tested: ran unit tests using old and new versions under Thread
Sanitizer, verified that TSan reports a data race for the old version,
but finds no issues with the new version.
Diffstat (limited to 'Firestore/Source/Util/FSTDispatchQueue.mm')
-rw-r--r-- | Firestore/Source/Util/FSTDispatchQueue.mm | 27 |
1 files changed, 14 insertions, 13 deletions
diff --git a/Firestore/Source/Util/FSTDispatchQueue.mm b/Firestore/Source/Util/FSTDispatchQueue.mm index 15d6e7b..0974359 100644 --- a/Firestore/Source/Util/FSTDispatchQueue.mm +++ b/Firestore/Source/Util/FSTDispatchQueue.mm @@ -16,6 +16,8 @@ #import <Foundation/Foundation.h> +#include <atomic> + #import "Firestore/Source/Util/FSTAssert.h" #import "Firestore/Source/Util/FSTDispatchQueue.h" @@ -146,24 +148,23 @@ NS_ASSUME_NONNULL_BEGIN #pragma mark - FSTDispatchQueue @interface FSTDispatchQueue () - /** * Callbacks scheduled to be queued in the future. Callbacks are automatically removed after they * are run or canceled. */ @property(nonatomic, strong, readonly) NSMutableArray<FSTDelayedCallback *> *delayedCallbacks; -/** - * Flag set while an FSTDispatchQueue operation is currently executing. Used for assertion - * sanity-checks. - */ -@property(nonatomic, assign) BOOL operationInProgress; - - (instancetype)initWithQueue:(dispatch_queue_t)queue NS_DESIGNATED_INITIALIZER; @end -@implementation FSTDispatchQueue +@implementation FSTDispatchQueue { + /** + * Flag set while an FSTDispatchQueue operation is currently executing. Used for assertion + * sanity-checks. + */ + std::atomic<bool> _operationInProgress; +} + (instancetype)queueWith:(dispatch_queue_t)dispatchQueue { return [[FSTDispatchQueue alloc] initWithQueue:dispatchQueue]; @@ -171,9 +172,9 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)initWithQueue:(dispatch_queue_t)queue { if (self = [super init]) { + _operationInProgress = false; _queue = queue; _delayedCallbacks = [NSMutableArray array]; - _operationInProgress = NO; } return self; } @@ -191,16 +192,16 @@ NS_ASSUME_NONNULL_BEGIN FSTAssert(!_operationInProgress, @"enterCheckedOperation may not be called when an operation is in progress"); @try { - _operationInProgress = YES; + _operationInProgress = true; [self verifyIsCurrentQueue]; block(); } @finally { - _operationInProgress = NO; + _operationInProgress = false; } } - (void)dispatchAsync:(void (^)(void))block { - FSTAssert(!_operationInProgress || ![self onTargetQueue], + FSTAssert(![self onTargetQueue] || !_operationInProgress, @"dispatchAsync called when we are already running on target dispatch queue '%@'", [self targetQueueLabel]); @@ -216,7 +217,7 @@ NS_ASSUME_NONNULL_BEGIN } - (void)dispatchSync:(void (^)(void))block { - FSTAssert(!_operationInProgress || ![self onTargetQueue], + FSTAssert(![self onTargetQueue] || !_operationInProgress, @"dispatchSync called when we are already running on target dispatch queue '%@'", [self targetQueueLabel]); |