Skip to content

Commit

Permalink
Merge pull request #12225 from muxi/fix-already-finished
Browse files Browse the repository at this point in the history
Fix undefined behavior of using dispatch_once_t as instance variable
  • Loading branch information
muxi committed Aug 22, 2017
2 parents e3d6a9a + 4e1b26e commit 72827fb
Showing 1 changed file with 31 additions and 7 deletions.
38 changes: 31 additions & 7 deletions src/objective-c/RxLibrary/GRXConcurrentWriteable.m
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ @interface GRXConcurrentWriteable ()
@implementation GRXConcurrentWriteable {
dispatch_queue_t _writeableQueue;
// This ensures that writesFinishedWithError: is only sent once to the writeable.
dispatch_once_t _alreadyFinished;
BOOL _alreadyFinished;
}

- (instancetype)init {
Expand Down Expand Up @@ -65,19 +65,35 @@ - (void)enqueueValue:(id)value completionHandler:(void (^)())handler {

- (void)enqueueSuccessfulCompletion {
dispatch_async(_writeableQueue, ^{
dispatch_once(&_alreadyFinished, ^{
BOOL finished = NO;
@synchronized (self) {
if (!_alreadyFinished) {
_alreadyFinished = YES;
} else {
finished = YES;
}
}
if (!finished) {
// Cancellation is now impossible. None of the other three blocks can run concurrently with
// this one.
[self.writeable writesFinishedWithError:nil];
// Skip any possible message to the wrapped writeable enqueued after this one.
self.writeable = nil;
});
}
});
}

- (void)cancelWithError:(NSError *)error {
NSAssert(error, @"For a successful completion, use enqueueSuccessfulCompletion.");
dispatch_once(&_alreadyFinished, ^{
BOOL finished = NO;
@synchronized (self) {
if (!_alreadyFinished) {
_alreadyFinished = YES;
} else {
finished = YES;
}
}
if (!finished) {
// Skip any of the still-enqueued messages to the wrapped writeable. We use the atomic setter to
// nillify writeable because we might be running concurrently with the blocks in
// _writeableQueue, and assignment with ARC isn't atomic.
Expand All @@ -87,15 +103,23 @@ - (void)cancelWithError:(NSError *)error {
dispatch_async(_writeableQueue, ^{
[writeable writesFinishedWithError:error];
});
});
}
}

- (void)cancelSilently {
dispatch_once(&_alreadyFinished, ^{
BOOL finished = NO;
@synchronized (self) {
if (!_alreadyFinished) {
_alreadyFinished = YES;
} else {
finished = YES;
}
}
if (!finished) {
// Skip any of the still-enqueued messages to the wrapped writeable. We use the atomic setter to
// nillify writeable because we might be running concurrently with the blocks in
// _writeableQueue, and assignment with ARC isn't atomic.
self.writeable = nil;
});
}
}
@end

0 comments on commit 72827fb

Please sign in to comment.