Skip to content

Commit

Permalink
Fix some bugs with async open task cancellation
Browse files Browse the repository at this point in the history
There were some very specific timings where cancelling the Task for an async
open could either crash or fail to call the completion handler. Waiting for
downloads when the Realm was already open would not cancel the download.
  • Loading branch information
tgoyne committed Mar 22, 2023
1 parent ba45055 commit 7d58375
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 26 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ x.y.z Release notes (yyyy-MM-dd)
* <How to hit and notice issue? what was the impact?> ([#????](https://github.com/realm/realm-swift/issues/????), since v?.?.?)
* Add missing `@Sendable` annotations to several sync and app services related
callbacks ([PR #8169](https://github.com/realm/realm-swift/pull/8169), since v10.34.0).
* Fix some bugs in handling task cancellation for async Realm init. Some very
specific timing windows could cause crashes, and the download would not be
cancelled if the Realm was already open ([PR #8178](https://github.com/realm/realm-swift/pull/8178), since v10.37.0).

<!-- ### Breaking Changes - ONLY INCLUDE FOR NEW MAJOR version -->

Expand Down
85 changes: 76 additions & 9 deletions Realm/RLMAsyncTask.mm
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@
#import "RLMAsyncTask_Private.h"

#import "RLMError_Private.hpp"
#import "RLMRealm_Private.hpp"
#import "RLMRealmConfiguration_Private.hpp"
#import "RLMScheduler.h"
#import "RLMSyncSubscription_Private.h"
#import "RLMUtil.hpp"

#import <realm/exceptions.hpp>
#import <realm/object-store/sync/async_open_task.hpp>
#import <realm/object-store/sync/sync_session.hpp>
#import <realm/object-store/thread_safe_reference.hpp>

static dispatch_queue_t s_async_open_queue = dispatch_queue_create("io.realm.asyncOpenDispatchQueue",
Expand All @@ -49,7 +51,7 @@ @implementation RLMAsyncOpenTask {
RLMRealmConfiguration *_configuration;
RLMScheduler *_scheduler;
bool _waitForDownloadCompletion;
RLMAsyncOpenRealmCallback _completion;
void (^_completion)(NSError *);

RLMRealm *_backgroundRealm;
}
Expand Down Expand Up @@ -82,8 +84,8 @@ - (void)cancel {
_progressBlocks.clear();
if (_task) {
_task->cancel();
[self reportError:s_canceledError];
}
[self reportError:s_canceledError];
}

- (void)setTask:(std::shared_ptr<realm::AsyncOpenTask>)task __attribute__((objc_direct)) {
Expand Down Expand Up @@ -127,6 +129,18 @@ - (instancetype)initWithConfiguration:(RLMRealmConfiguration *)configuration
}

- (void)waitForOpen:(RLMAsyncOpenRealmCallback)completion {
__weak auto weakSelf = self;
[self waitWithCompletion:^(NSError *error) {
RLMRealm *realm;
if (auto self = weakSelf) {
realm = self->_localRealm;
self->_localRealm = nil;
}
completion(realm, error);
}];
}

- (void)waitWithCompletion:(void (^)(NSError *))completion {
{
std::lock_guard lock(_mutex);
_completion = completion;
Expand Down Expand Up @@ -229,19 +243,32 @@ - (void)completeAsyncOpen {

[_scheduler invoke:^{
@autoreleasepool {
if ([self checkCancellation]) {
return;
}
NSError *error;
RLMRealm *localRealm = [RLMRealm realmWithConfiguration:_configuration
confinedTo:_scheduler
error:&error];
auto completion = _completion;
[self releaseResources];
completion(localRealm, error);
void (^completion)(NSError *);
{
std::lock_guard lock(_mutex);
if (!_completion) {
return;
}
completion = _completion;
_localRealm = [RLMRealm realmWithConfiguration:_configuration
confinedTo:_scheduler
error:&error];
[self releaseResources];
}
completion(error);
}
}];
}

- (bool)checkCancellation {
std::lock_guard lock(_mutex);
if (_cancel && _completion) {
[self reportError:s_canceledError];
}
return _cancel;
}

Expand All @@ -263,11 +290,15 @@ - (void)reportException:(std::exception_ptr const&)err {
}

- (void)reportError:(NSError *)error {
if (!_completion || !_scheduler) {
return;
}

auto completion = _completion;
auto scheduler = _scheduler;
[self releaseResources];
[scheduler invoke:^{
completion(nil, error);
completion(error);
}];
}

Expand All @@ -278,3 +309,39 @@ - (void)releaseResources {
_completion = nil;
}
@end

@implementation RLMAsyncDownloadTask {
RLMUnfairMutex _mutex;
std::shared_ptr<realm::SyncSession> _session;
bool _started;
}

- (instancetype)initWithRealm:(RLMRealm *)realm {
if (self = [super init]) {
_session = realm->_realm->sync_session();
}
return self;
}

- (void)waitWithCompletion:(void (^)(NSError *_Nullable))completion {
std::unique_lock lock(_mutex);
if (!_session) {
lock.unlock();
return completion(nil);
}

_started = true;
_session->revive_if_needed();
_session->wait_for_download_completion([=](realm::Status status) {
completion(makeError(status));
});
}

- (void)cancel {
std::unique_lock lock(_mutex);
if (_started) {
_session->force_close();
}
_session = nullptr;
}
@end
12 changes: 10 additions & 2 deletions Realm/RLMAsyncTask_Private.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,16 @@ __attribute__((objc_direct));
- (instancetype)initWithConfiguration:(RLMRealmConfiguration *)configuration
confinedTo:(RLMScheduler *)confinement
download:(bool)waitForDownloadCompletion;
- (void)waitForOpen:(RLMAsyncOpenRealmCallback)completion
__attribute__((swift_attr("@_unsafeInheritExecutor")));

- (void)waitWithCompletion:(void (^)(NSError *_Nullable))completion;
- (void)waitForOpen:(RLMAsyncOpenRealmCallback)completion __attribute__((objc_direct));
@end

RLM_SWIFT_SENDABLE
@interface RLMAsyncDownloadTask : NSObject
- (instancetype)initWithRealm:(RLMRealm *)realm;
- (void)cancel;
- (void)waitWithCompletion:(void (^)(NSError *_Nullable))completion;
@end

RLM_HEADER_AUDIT_END(nullability)
48 changes: 33 additions & 15 deletions RealmSwift/Realm.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1220,9 +1220,10 @@ extension Realm {
if let realm = realm {
// This can't be hit on the first open so .once == .never
if downloadBeforeOpen == .always {
try await realm.waitForDownloadCompletion()
let task = RLMAsyncDownloadTask(realm: realm)
try await task.waitWithCancellationHandler()
}
self = Realm(realm)
rlmRealm = realm
return
}

Expand All @@ -1231,18 +1232,8 @@ extension Realm {
let task = RLMAsyncOpenTask(configuration: rlmConfiguration, confinedTo: scheduler,
download: shouldAsyncOpen(configuration, downloadBeforeOpen))
do {
// Work around https://github.com/apple/swift/issues/61119, which
// makes it impossible to use withTaskCancellationHandler() from
// within an isolated function without getting warnings
nonisolated func workaround() async throws {
try await withTaskCancellationHandler { @Sendable in
task.localRealm = try await task.waitForOpen()
} onCancel: { @Sendable in
task.cancel()
}
}
try await workaround()
self = Realm(task.localRealm!)
try await task.waitWithCancellationHandler()
rlmRealm = task.localRealm!
task.localRealm = nil
} catch {
// Check if the task was cancelled and if so replace the error
Expand All @@ -1252,7 +1243,34 @@ extension Realm {
}
}
}
#endif // swift(>=5.5)

@available(macOS 10.15, tvOS 13.0, iOS 13.0, watchOS 6.0, *)
private protocol TaskWithCancellation: Sendable {
func waitWithCancellationHandler() async throws
func wait() async throws
func cancel()
}

@available(macOS 10.15, tvOS 13.0, iOS 13.0, watchOS 6.0, *)
extension TaskWithCancellation {
func waitWithCancellationHandler() async throws {
do {
try await withTaskCancellationHandler {
try await wait()
} onCancel: {
cancel()
}
} catch {
// Check if the task was cancelled and if so replace the error
// with reporting cancellation
try Task.checkCancellation()
throw error
}
}
}
extension RLMAsyncOpenTask: TaskWithCancellation {}
extension RLMAsyncDownloadTask: TaskWithCancellation {}
#endif // canImport(_Concurrency)

/**
Objects which can be fetched from the Realm - Object or Projection
Expand Down

0 comments on commit 7d58375

Please sign in to comment.