Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix undefined behavior of using dispatch_once_t as instance variable #12225

Merged
merged 1 commit into from
Aug 22, 2017
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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