Skip to content

Commit

Permalink
Fix a deadlock with simultaneous sync and nonsync commit notification…
Browse files Browse the repository at this point in the history
…s while tearing down RealmCoordinator

The scenario which deadlocked is the following:

Sync worker thread: Reporting a sync commit made to object store
1. SessionWrapper::report_sync_transact()
2. SyncSession::create_sync_session() wrapped_callback lambda
    holding strong reference to self and acquires SyncSession::m_state_mutex
3. RealmCoordinator::init_external_helpers() sync callback lambda
    Acquires strong refence to self, does some work, then releases it, triggering destructor
4. ~RealmCoordinator()
5. ~ExternalCommitHelper()
6. ExternalCommitHelper::m_thread::join()
    Waiting for the ECH thread to complete, while holding SyncSession::m_state_mutex

Notifier thread: Trying to report a non-sync commit to sync
1. ExternalCommitHelper::listen()
2. void RealmCoordinator::on_change()
3. SyncSession::nonsync_transact_notify()
    Trying to acquire SyncSession::m_state_mutex

To fix this, don't hold SyncSession::m_state_mutex while making the SyncSession
-> RealmCoordinator call.
  • Loading branch information
tgoyne committed Oct 19, 2022
1 parent 71621af commit 0710b20
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
* <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
* CompensatingWriteErrorInfo reported string primary keys as boolean values instead ([PR #5938](https://github.com/realm/realm-core/pull/5938), since the introduction of CompensatingWriteErrorInfo in 12.1.0).
* Fix a use-after-free if the last external reference to an encrypted Realm was closed between when a client reset error was received and when the download of the new Realm began. ([PR #5949](https://github.com/realm/realm-core/pull/5949), since 12.4.0).
* Fix a rare deadlock which could occur when closing a synchronized Realm immediately after committing a write transaction when the sync worker thread has also just finished processing a changeset from the server ([PR #5948](https://github.com/realm/realm-core/pull/5948)).

### Breaking changes
* Rename RealmConfig::automatic_handle_backlicks_in_migrations to RealmConfig::automatically_handle_backlinks_in_migrations ([PR #5897](https://github.com/realm/realm-core/pull/5897)).
Expand Down
10 changes: 6 additions & 4 deletions src/realm/object-store/sync/sync_session.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -785,11 +785,13 @@ void SyncSession::create_sync_session()

// Configure the sync transaction callback.
auto wrapped_callback = [weak_self](VersionID old_version, VersionID new_version) {
std::function<TransactionCallback> callback;
if (auto self = weak_self.lock()) {
util::CheckedLockGuard l(self->m_state_mutex);
if (self->m_sync_transact_callback) {
self->m_sync_transact_callback(old_version, new_version);
}
callback = self->m_sync_transact_callback;
}
if (callback) {
callback(old_version, new_version);
}
};
m_session->set_sync_transact_callback(std::move(wrapped_callback));
Expand Down Expand Up @@ -845,7 +847,7 @@ void SyncSession::create_sync_session()
});
}

void SyncSession::set_sync_transact_callback(util::UniqueFunction<sync::Session::SyncTransactCallback> callback)
void SyncSession::set_sync_transact_callback(std::function<sync::Session::SyncTransactCallback>&& callback)
{
util::CheckedLockGuard l(m_state_mutex);
m_sync_transact_callback = std::move(callback);
Expand Down
7 changes: 3 additions & 4 deletions src/realm/object-store/sync/sync_session.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,7 @@ class SyncSession : public std::enable_shared_from_this<SyncSession> {
class Internal {
friend class _impl::RealmCoordinator;

static void set_sync_transact_callback(SyncSession& session,
util::UniqueFunction<TransactionCallback> callback)
static void set_sync_transact_callback(SyncSession& session, std::function<TransactionCallback>&& callback)
{
session.set_sync_transact_callback(std::move(callback));
}
Expand Down Expand Up @@ -346,7 +345,7 @@ class SyncSession : public std::enable_shared_from_this<SyncSession> {
void handle_progress_update(uint64_t, uint64_t, uint64_t, uint64_t, uint64_t, uint64_t);
void handle_new_flx_sync_query(int64_t version);

void set_sync_transact_callback(util::UniqueFunction<TransactionCallback>) REQUIRES(!m_state_mutex);
void set_sync_transact_callback(std::function<TransactionCallback>&&) REQUIRES(!m_state_mutex);
void nonsync_transact_notify(VersionID::version_type) REQUIRES(!m_state_mutex);

void create_sync_session() REQUIRES(m_state_mutex, !m_config_mutex);
Expand All @@ -365,7 +364,7 @@ class SyncSession : public std::enable_shared_from_this<SyncSession> {

util::Future<std::string> send_test_command(std::string body) REQUIRES(!m_state_mutex);

util::UniqueFunction<TransactionCallback> m_sync_transact_callback GUARDED_BY(m_state_mutex);
std::function<TransactionCallback> m_sync_transact_callback GUARDED_BY(m_state_mutex);

template <typename Field>
auto config(Field f) REQUIRES(!m_config_mutex)
Expand Down

0 comments on commit 0710b20

Please sign in to comment.