Skip to content

Commit

Permalink
Fix spurious incorrect thread exceptions due to threadid reuse
Browse files Browse the repository at this point in the history
We key the Realm cache on the thread ID, but thread IDs are not actually unique
over the lifetime of the process. This means that the cached Realm may have
been created on a different thread with the same ID, which means that it'll be
bound to a different runloop than the current thread's runloop. Check for this,
and simply open a new Realm instead of reusing the cached Realm when this
happens.
  • Loading branch information
tgoyne committed Aug 20, 2020
1 parent 93a9722 commit 4462f4c
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 4 deletions.
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@ x.y.z Release notes (yyyy-MM-dd)

### Fixed
* <How to hit and notice issue? what was the impact?> ([#????](https://github.com/realm/realm-cocoa/issues/????), since v?.?.?)
* None.
* Opening Realms on background threads could produce spurious Incorrect Thread
exceptions when a cached Realm existed for a previously existing thread with
the same thread ID as the current thread.
([#6659](https://github.com/realm/realm-cocoa/issues/6659),
[#6689](https://github.com/realm/realm-cocoa/issues/6689),
[#6712](https://github.com/realm/realm-cocoa/issues/6712), since 5.0.0).

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

Expand Down
13 changes: 12 additions & 1 deletion Realm/RLMRealmUtil.mm
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

#import "binding_context.hpp"
#import "shared_realm.hpp"
#import "util/scheduler.hpp"

#import <map>
#import <mutex>
Expand All @@ -54,7 +55,17 @@ void RLMCacheRealm(std::string const& path, void *key, __unsafe_unretained RLMRe

RLMRealm *RLMGetThreadLocalCachedRealmForPath(std::string const& path, void *key) {
std::lock_guard<std::mutex> lock(s_realmCacheMutex);
return [s_realmsPerPath[path] objectForKey:(__bridge id)key];
RLMRealm *realm = [s_realmsPerPath[path] objectForKey:(__bridge id)key];
if (realm && !realm->_realm->scheduler()->is_on_thread()) {
// We can get here in two cases: if the user is trying to open a
// queue-bound Realm from the wrong queue, or if we have a stale cached
// Realm which is bound to a thread that no longer exists. In the first
// case we'll throw an error later on; in the second we'll just create
// a new RLMRealm and replace the cache entry with one bound to the
// thread that now exists.
realm = nil;
}
return realm;
}

void RLMClearRealmCache() {
Expand Down
39 changes: 37 additions & 2 deletions Realm/Tests/RealmTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#import <mach/vm_map.h>
#import <sys/resource.h>
#import <thread>
#import <unordered_set>

#import <realm/util/file.hpp>
#import <realm/db_options.hpp>
Expand Down Expand Up @@ -1561,17 +1562,31 @@ - (void)testQueueBoundRealmCaching {
@autoreleasepool {
RLMRealm *backgroundThreadRealm = [RLMRealm defaultRealm];
RLMRealm *q1Realm3 = [RLMRealm defaultRealmForQueue:q1];
RLMRealm *q2Realm3 = [RLMRealm defaultRealmForQueue:q2];
XCTAssertThrows([RLMRealm defaultRealmForQueue:q2]);

XCTAssertNotEqual(backgroundThreadRealm, mainThreadRealm1);
XCTAssertNotEqual(backgroundThreadRealm, mainQueueRealm1);
XCTAssertNotEqual(backgroundThreadRealm, q1Realm1);
XCTAssertNotEqual(backgroundThreadRealm, q1Realm2);
XCTAssertEqual(q1Realm1, q1Realm3);
XCTAssertEqual(q2Realm2, q2Realm3);
}
});
dispatch_sync(q1, ^{});

dispatch_async(q2, ^{
@autoreleasepool {
RLMRealm *backgroundThreadRealm = [RLMRealm defaultRealm];
XCTAssertThrows([RLMRealm defaultRealmForQueue:q1]);
RLMRealm *q2Realm3 = [RLMRealm defaultRealmForQueue:q2];

XCTAssertNotEqual(backgroundThreadRealm, mainThreadRealm1);
XCTAssertNotEqual(backgroundThreadRealm, mainQueueRealm1);
XCTAssertNotEqual(backgroundThreadRealm, q1Realm1);
XCTAssertNotEqual(backgroundThreadRealm, q1Realm2);
XCTAssertEqual(q2Realm2, q2Realm3);
}
});
dispatch_sync(q2, ^{});
}

- (void)testQueueValidation {
Expand Down Expand Up @@ -2085,6 +2100,26 @@ - (void)testCanCreate100RealmsWithoutBreakingGCD
[self waitForExpectationsWithTimeout:1 handler:nil];
}

- (void)testThreadIDReuse {
// Open Realms on new threads until we get repeated thread IDs, while
// retaining each Realm opened. This verifies that we don't get a Realm from
// an old thread that no longer exists from the cache.
NSMutableArray *realms = [NSMutableArray array];
std::unordered_set<pthread_t> threadIds;
bool done = false;
while (!done) {
std::thread([&] {
RLMRealm *realm = [RLMRealm defaultRealm];
[realms addObject:realm];
(void)[IntObject allObjectsInRealm:realm].count;
[realm refresh];
if (!threadIds.insert(pthread_self()).second) {
done = true;
}
}).join();
}
}

- (void)testAuxiliaryFilesAreExcludedFromBackup {
RLMSetSkipBackupAttribute(true);
@autoreleasepool { [RLMRealm defaultRealm]; }
Expand Down

0 comments on commit 4462f4c

Please sign in to comment.