diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a2a7f678d..07ec878504 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ x.y.z Release notes (yyyy-MM-dd) * ([#????](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). diff --git a/Realm/RLMAsyncTask.mm b/Realm/RLMAsyncTask.mm index fbc0dfbf17..b4b62a98af 100644 --- a/Realm/RLMAsyncTask.mm +++ b/Realm/RLMAsyncTask.mm @@ -19,6 +19,7 @@ #import "RLMAsyncTask_Private.h" #import "RLMError_Private.hpp" +#import "RLMRealm_Private.hpp" #import "RLMRealmConfiguration_Private.hpp" #import "RLMScheduler.h" #import "RLMSyncSubscription_Private.h" @@ -26,6 +27,7 @@ #import #import +#import #import static dispatch_queue_t s_async_open_queue = dispatch_queue_create("io.realm.asyncOpenDispatchQueue", @@ -49,7 +51,7 @@ @implementation RLMAsyncOpenTask { RLMRealmConfiguration *_configuration; RLMScheduler *_scheduler; bool _waitForDownloadCompletion; - RLMAsyncOpenRealmCallback _completion; + void (^_completion)(NSError *); RLMRealm *_backgroundRealm; } @@ -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)task __attribute__((objc_direct)) { @@ -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; @@ -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; } @@ -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); }]; } @@ -278,3 +309,39 @@ - (void)releaseResources { _completion = nil; } @end + +@implementation RLMAsyncDownloadTask { + RLMUnfairMutex _mutex; + std::shared_ptr _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 diff --git a/Realm/RLMAsyncTask_Private.h b/Realm/RLMAsyncTask_Private.h index c7c47bb405..98b1db7c4d 100644 --- a/Realm/RLMAsyncTask_Private.h +++ b/Realm/RLMAsyncTask_Private.h @@ -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) diff --git a/RealmSwift/Realm.swift b/RealmSwift/Realm.swift index 50dd40b57b..51e603d12e 100644 --- a/RealmSwift/Realm.swift +++ b/RealmSwift/Realm.swift @@ -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 } @@ -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 @@ -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