diff --git a/CHANGELOG.md b/CHANGELOG.md index d2cbc21220b..a07c770327e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,8 @@ ### Fixed * ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?) -* None. - +* Fix a data race reported by thread sanitizer when preparing to deliver change notifications. This probably did not cause observable problems in practice ([PR #5892](https://github.com/realm/realm-core/pull/5892) since 12.7.0). + ### Breaking changes * None. diff --git a/src/realm/object-store/impl/collection_notifier.cpp b/src/realm/object-store/impl/collection_notifier.cpp index e53c42c1c92..5b4a171436c 100644 --- a/src/realm/object-store/impl/collection_notifier.cpp +++ b/src/realm/object-store/impl/collection_notifier.cpp @@ -413,10 +413,11 @@ NotifierPackage::NotifierPackage(std::vector { } -NotifierPackage::NotifierPackage(std::vector> notifiers) - : m_notifiers(std::move(notifiers)) +NotifierPackage::NotifierPackage(std::vector> notifiers, + std::optional version) + : m_version(version) + , m_notifiers(std::move(notifiers)) { - set_version(); } void NotifierPackage::package_and_wait(VersionID::version_type target_version) @@ -424,17 +425,11 @@ void NotifierPackage::package_and_wait(VersionID::version_type target_version) if (!m_coordinator || !*this) return; - m_coordinator->package_notifiers(m_notifiers, target_version); - set_version(); + m_version = m_coordinator->package_notifiers(m_notifiers, target_version); if (m_version && m_version->version < target_version) m_version = util::none; } -void NotifierPackage::set_version() -{ - m_version = m_notifiers.empty() ? util::none : std::make_optional(m_notifiers.front()->version()); -} - void NotifierPackage::before_advance() { for (auto& notifier : m_notifiers) diff --git a/src/realm/object-store/impl/collection_notifier.hpp b/src/realm/object-store/impl/collection_notifier.hpp index 8654a3429ba..cae6685e061 100644 --- a/src/realm/object-store/impl/collection_notifier.hpp +++ b/src/realm/object-store/impl/collection_notifier.hpp @@ -330,7 +330,7 @@ class NotifierPackage { NotifierPackage() = default; // Create a package which contains notifiers which have already been pacakged for delivery - NotifierPackage(std::vector> notifiers); + NotifierPackage(std::vector> notifiers, std::optional version); // Create a package which can have package_and_wait() called on it later NotifierPackage(std::vector> notifiers, RealmCoordinator* coordinator); @@ -362,8 +362,6 @@ class NotifierPackage { util::Optional m_version; std::vector> m_notifiers; RealmCoordinator* m_coordinator = nullptr; - - void set_version(); }; } // namespace realm::_impl diff --git a/src/realm/object-store/impl/realm_coordinator.cpp b/src/realm/object-store/impl/realm_coordinator.cpp index cd6073c5339..3dfe9f1ede1 100644 --- a/src/realm/object-store/impl/realm_coordinator.cpp +++ b/src/realm/object-store/impl/realm_coordinator.cpp @@ -1172,7 +1172,8 @@ void RealmCoordinator::advance_to_ready(Realm& realm) } // We have notifiers for a newer version, so advance to that - transaction::advance(tr, realm.m_binding_context.get(), std::move(notifiers)); + transaction::advance(tr, realm.m_binding_context.get(), + _impl::NotifierPackage(std::move(notifiers), notifier_version)); } std::vector> RealmCoordinator::notifiers_for_realm(Realm& realm) @@ -1198,11 +1199,11 @@ bool RealmCoordinator::advance_to_latest(Realm& realm) util::CheckedUniqueLock lock(m_notifier_mutex); notifiers = notifiers_for_realm(realm); } - package_notifiers(notifiers, m_db->get_version_of_latest_snapshot()); + auto version = package_notifiers(notifiers, m_db->get_version_of_latest_snapshot()); - auto version = tr->get_version_of_current_transaction(); - transaction::advance(tr, realm.m_binding_context.get(), _impl::NotifierPackage(std::move(notifiers))); - return !realm.is_closed() && version != tr->get_version_of_current_transaction(); + auto prev_version = tr->get_version_of_current_transaction(); + transaction::advance(tr, realm.m_binding_context.get(), _impl::NotifierPackage(std::move(notifiers), version)); + return !realm.is_closed() && prev_version != tr->get_version_of_current_transaction(); } void RealmCoordinator::promote_to_write(Realm& realm) @@ -1257,7 +1258,8 @@ void RealmCoordinator::process_available_async(Realm& realm) realm.m_binding_context->did_send_notifications(); } -void RealmCoordinator::package_notifiers(NotifierVector& notifiers, VersionID::version_type target_version) +std::optional RealmCoordinator::package_notifiers(NotifierVector& notifiers, + VersionID::version_type target_version) { auto ready = [&] { util::CheckedUniqueLock notifier_lock(m_notifier_mutex); @@ -1280,6 +1282,7 @@ void RealmCoordinator::package_notifiers(NotifierVector& notifiers, VersionID::v notifier->version().version < target_version; }; notifiers.erase(std::remove_if(begin(notifiers), end(notifiers), package), end(notifiers)); + return notifiers.empty() ? std::optional{} : notifiers.front()->version(); } bool RealmCoordinator::compact() diff --git a/src/realm/object-store/impl/realm_coordinator.hpp b/src/realm/object-store/impl/realm_coordinator.hpp index da5181621bb..e2aa97b0b80 100644 --- a/src/realm/object-store/impl/realm_coordinator.hpp +++ b/src/realm/object-store/impl/realm_coordinator.hpp @@ -207,7 +207,7 @@ class RealmCoordinator : public std::enable_shared_from_this { // Called by NotifierPackage in the cases where we don't know what version // we need notifiers for until after we begin advancing (e.g. when // starting a write transaction). - void package_notifiers(NotifierVector& notifiers, VersionID::version_type) + std::optional package_notifiers(NotifierVector& notifiers, VersionID::version_type) REQUIRES(!m_notifier_mutex, !m_running_notifiers_mutex); // testing hook only to verify that notifiers are not being run at times