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

Wrap write transactions in background tasks on supported platforms #4827

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ x.x.x Release notes (yyyy-MM-dd)

* Add a `{RLM}SyncUser.isAdmin` property indicating whether a user is a Realm
Object Server administrator.
* Write transactions are automatically marked as background tasks on platforms
which support them to avoid having an app suspended while it holds the write
lock.

### Bugfixes

Expand Down
107 changes: 99 additions & 8 deletions Realm/RLMRealm.mm
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@
#include <realm/util/scope_exit.hpp>
#include <realm/version.hpp>

#if REALM_IOS
#import <UIKit/UIKit.h>
#endif

using namespace realm;
using util::File;

Expand Down Expand Up @@ -120,23 +124,98 @@ static bool shouldForciblyDisableEncryption() {
@implementation RLMRealm {
NSHashTable<RLMFastEnumerator *> *_collectionEnumerators;
bool _sendingNotifications;

// Needs to be atomic because the timeout callback is called on the main
// thread and not the RLMRealm's thread, so we could try to end the task
// from multiple threads at once
std::atomic<NSUInteger> _backgroundTaskIdentifier;
bool _addedBackgroundNotification;
}

+ (BOOL)isCoreDebug {
return realm::Version::has_feature(realm::feature_Debug);
}

id g_sharedApplication;
+ (void)initialize {
static bool initialized;
if (initialized) {
return;
}
initialized = true;

#if REALM_IOS
// Using NSClassFromString rather than directly referencing UIApplication
// avoids the need to link UIKit.framework, and we can't actually write
// `[UIApplication sharedApplication]` due to compiling as extension-safe
g_sharedApplication = [NSClassFromString(@"UIApplication") sharedApplication];

// Background tasks aren't supported if we're running inside an extension
if (![g_sharedApplication respondsToSelector:@selector(beginBackgroundTaskWithExpirationHandler:)]) {
g_sharedApplication = nil;
}
#endif

RLMCheckForUpdates();
RLMSendAnalytics();
}

#if REALM_IOS
// On iOS we want to wrap our write transactions in background tasks so that the
// OS doesn't suspend the app while we hold the write lock, as that will block
// any other apps in the same app group from using the Realm. Doing this
// unconditionally is extremely slow, so instead we only do it if we're already
// in the background, and just register for the background state transition
// notification otherwise
- (void)beginBackgroundTask {
if (!g_sharedApplication) {
return;
}

if ([g_sharedApplication applicationState] == UIApplicationStateBackground) {
[self applicationDidEnterBackground:nil];
return;
}

[NSNotificationCenter.defaultCenter addObserver:self
selector:@selector(applicationDidEnterBackground:)
name:@"UIApplicationDidEnterBackground"
object:nil];
_addedBackgroundNotification = true;
}

- (void)applicationDidEnterBackground:(NSNotification *)notification {
if (notification && !self.inWriteTransaction) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this notification potentially be delivered on a different thread than the one this RLMRealm is associated with? I don't think it's safe to call -[RLMRealm inWriteTransaction] on a Realm from a different thread.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I think it'll always be called on the main thread. inWriteTransaction doesn't check the thread so it'll usually work by coincidence, but it's a race condition.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we're using this approach and actually it's easy to receive Realm accessed from incorrect thread exception when trying to cancel a transaction in the write state.

// No need for a task if we got the notification when not actually in
// a write transaction
return;
}

// The block needs to not retain `self` or we'll leak any Realms which the
// user fails to commit/cancel
__weak RLMRealm *weakSelf = self;
_backgroundTaskIdentifier = [g_sharedApplication beginBackgroundTaskWithExpirationHandler:^{
// If we time out just end the task and let the OS suspend us rather
// than terminating
if (auto self = weakSelf) {
[self endBackgroundTask];
}
}];
}

- (void)endBackgroundTask {
if (auto identifier = _backgroundTaskIdentifier.exchange(0)) {
[g_sharedApplication endBackgroundTask:identifier];
}
}
#else
- (void)beginBackgroundTask {
}

- (void)endBackgroundTask {
}
#endif // REALM_IOS

- (instancetype)initPrivate {
self = [super init];
return self;
Expand Down Expand Up @@ -450,10 +529,12 @@ - (RLMRealmConfiguration *)configuration {
}

- (void)beginWriteTransaction {
[self beginBackgroundTask];
try {
_realm->begin_transaction();
}
catch (std::exception &ex) {
[self endBackgroundTask];
@throw RLMException(ex);
}
}
Expand All @@ -465,15 +546,18 @@ - (void)commitWriteTransaction {
- (BOOL)commitWriteTransaction:(NSError **)outError {
try {
_realm->commit_transaction();
[self endBackgroundTask];
return YES;
}
catch (...) {
[self endBackgroundTask];
RLMRealmTranslateException(outError);
return NO;
}
}

- (BOOL)commitWriteTransactionWithoutNotifying:(NSArray<RLMNotificationToken *> *)tokens error:(NSError **)error {
- (BOOL)commitWriteTransactionWithoutNotifying:(NSArray<RLMNotificationToken *> *)tokens
error:(NSError **)error {
for (RLMNotificationToken *token in tokens) {
if (token.realm != self) {
@throw RLMException(@"Incorrect Realm: only notifications for the Realm being modified can be skipped.");
Expand All @@ -483,9 +567,11 @@ - (BOOL)commitWriteTransactionWithoutNotifying:(NSArray<RLMNotificationToken *>

try {
_realm->commit_transaction();
[self endBackgroundTask];
return YES;
}
catch (...) {
[self endBackgroundTask];
RLMRealmTranslateException(error);
return NO;
}
Expand All @@ -507,6 +593,7 @@ - (BOOL)transactionWithBlock:(void(^)(void))block error:(NSError **)outError {
- (void)cancelWriteTransaction {
try {
_realm->cancel_transaction();
[self endBackgroundTask];
}
catch (std::exception &ex) {
@throw RLMException(ex);
Expand All @@ -528,6 +615,7 @@ - (void)invalidate {
}

_realm->invalidate();
[self endBackgroundTask];

for (auto& objectInfo : _info) {
for (RLMObservationInfo *info : objectInfo.second.observedObjects) {
Expand Down Expand Up @@ -579,13 +667,16 @@ - (BOOL)compact {
}

- (void)dealloc {
if (_realm) {
if (_realm->is_in_transaction()) {
[self cancelWriteTransaction];
NSLog(@"WARNING: An RLMRealm instance was deallocated during a write transaction and all "
"pending changes have been rolled back. Make sure to retain a reference to the "
"RLMRealm for the duration of the write transaction.");
}
if (_realm && _realm->is_in_transaction()) {
[self cancelWriteTransaction];
NSLog(@"WARNING: An RLMRealm instance was deallocated during a write transaction and all "
"pending changes have been rolled back. Make sure to retain a reference to the "
"RLMRealm for the duration of the write transaction.");
}
if (_addedBackgroundNotification) {
[NSNotificationCenter.defaultCenter removeObserver:self
name:@"UIApplicationDidEnterBackground"
object:nil];
}
}

Expand Down