From e87a50722386c47497804305cc468e102e91dcde Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Wed, 19 Aug 2015 12:27:12 -0700 Subject: [PATCH 01/17] Extract cache management and inter-Realm sharing to RealmCoordinator --- src/CMakeLists.txt | 1 + src/impl/apple/external_commit_helper.cpp | 42 ++-- src/impl/apple/external_commit_helper.hpp | 12 +- src/impl/realm_coordinator.cpp | 182 ++++++++++++++++ src/impl/realm_coordinator.hpp | 77 +++++++ src/results.cpp | 2 + src/shared_realm.cpp | 242 ++++++---------------- src/shared_realm.hpp | 34 ++- 8 files changed, 362 insertions(+), 230 deletions(-) create mode 100644 src/impl/realm_coordinator.cpp create mode 100644 src/impl/realm_coordinator.hpp diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index bef8feb4..2c9eade2 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -8,6 +8,7 @@ set(SOURCES results.cpp schema.cpp shared_realm.cpp + impl/realm_coordinator.cpp impl/transact_log_handler.cpp parser/parser.cpp parser/query_builder.cpp) diff --git a/src/impl/apple/external_commit_helper.cpp b/src/impl/apple/external_commit_helper.cpp index 4cf0b523..c22fe5df 100644 --- a/src/impl/apple/external_commit_helper.cpp +++ b/src/impl/apple/external_commit_helper.cpp @@ -18,16 +18,16 @@ #include "external_commit_helper.hpp" -#include "shared_realm.hpp" +#include "realm_coordinator.hpp" #include +#include +#include #include #include #include #include -#include #include -#include using namespace realm; using namespace realm::_impl; @@ -56,11 +56,10 @@ void notify_fd(int fd) void ExternalCommitHelper::FdHolder::close() { - if (m_fd != -1) { - ::close(m_fd); - } - m_fd = -1; - + if (m_fd != -1) { + ::close(m_fd); + } + m_fd = -1; } // Inter-thread and inter-process notifications of changes are done using a @@ -86,16 +85,15 @@ void ExternalCommitHelper::FdHolder::close() // signal the runloop source and wake up the target runloop, and when data is // written to the anonymous pipe the background thread removes the runloop // source from the runloop and and shuts down. -ExternalCommitHelper::ExternalCommitHelper(Realm* realm) +ExternalCommitHelper::ExternalCommitHelper(RealmCoordinator& parent) +: m_parent(parent) { - add_realm(realm); - m_kq = kqueue(); if (m_kq == -1) { throw std::system_error(errno, std::system_category()); } - auto path = realm->config().path + ".note"; + auto path = parent.get_path() + ".note"; // Create and open the named pipe int ret = mkfifo(path.c_str(), 0600); @@ -140,28 +138,14 @@ ExternalCommitHelper::ExternalCommitHelper(Realm* realm) m_shutdown_read_fd = pipeFd[0]; m_shutdown_write_fd = pipeFd[1]; - // Use the minimum allowed stack size, as we need very little in our listener - // https://developer.apple.com/library/ios/documentation/Cocoa/Conceptual/Multithreading/CreatingThreads/CreatingThreads.html#//apple_ref/doc/uid/10000057i-CH15-SW7 - pthread_attr_t attr; - pthread_attr_init(&attr); - pthread_attr_setstacksize(&attr, 16 * 1024); - - auto fn = [](void *self) -> void * { - static_cast(self)->listen(); - return nullptr; - }; - ret = pthread_create(&m_thread, &attr, fn, this); - pthread_attr_destroy(&attr); - if (ret != 0) { - throw std::system_error(errno, std::system_category()); - } + m_thread = std::async(std::launch::async, [=] { listen(); }); } ExternalCommitHelper::~ExternalCommitHelper() { REALM_ASSERT_DEBUG(m_realms.empty()); notify_fd(m_shutdown_write_fd); - pthread_join(m_thread, nullptr); // Wait for the thread to exit + m_thread.wait(); // Wait for the thread to exit } void ExternalCommitHelper::add_realm(realm::Realm* realm) @@ -202,7 +186,6 @@ void ExternalCommitHelper::listen() { pthread_setname_np("RLMRealm notification listener"); - // Set up the kqueue // EVFILT_READ indicates that we care about data being available to read // on the given file descriptor. @@ -248,4 +231,3 @@ void ExternalCommitHelper::notify_others() { notify_fd(m_notify_fd); } - diff --git a/src/impl/apple/external_commit_helper.hpp b/src/impl/apple/external_commit_helper.hpp index d7acb791..9f31d730 100644 --- a/src/impl/apple/external_commit_helper.hpp +++ b/src/impl/apple/external_commit_helper.hpp @@ -20,6 +20,8 @@ #define REALM_EXTERNAL_COMMIT_HELPER_HPP #include +#include +#include #include #include @@ -27,9 +29,13 @@ namespace realm { class Realm; namespace _impl { +class RealmCoordinator; + +// FIXME: split IPC from the local cross-thread stuff +// both are platform-specific but need to be useable separately class ExternalCommitHelper { public: - ExternalCommitHelper(Realm* realm); + ExternalCommitHelper(RealmCoordinator& parent); ~ExternalCommitHelper(); void notify_others(); @@ -67,6 +73,8 @@ private: void listen(); + RealmCoordinator& m_parent; + // Currently registered realms and the signal for delivering notifications // to them std::vector m_realms; @@ -75,7 +83,7 @@ private: std::mutex m_realms_mutex; // The listener thread - pthread_t m_thread; + std::future m_thread; // Read-write file descriptor for the named pipe which is waited on for // changes and written to when a commit is made diff --git a/src/impl/realm_coordinator.cpp b/src/impl/realm_coordinator.cpp new file mode 100644 index 00000000..82a6dff1 --- /dev/null +++ b/src/impl/realm_coordinator.cpp @@ -0,0 +1,182 @@ +//////////////////////////////////////////////////////////////////////////// +// +// Copyright 2015 Realm Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +//////////////////////////////////////////////////////////////////////////// + +#include "realm_coordinator.hpp" + +#include "external_commit_helper.hpp" +#include "object_store.hpp" + +using namespace realm; +using namespace realm::_impl; + +static std::mutex s_coordinator_mutex; +static std::map> s_coordinators_per_path; + +std::shared_ptr RealmCoordinator::get_coordinator(StringData path) +{ + std::lock_guard lock(s_coordinator_mutex); + std::shared_ptr coordinator; + + auto it = s_coordinators_per_path.find(path); + if (it != s_coordinators_per_path.end()) { + coordinator = it->second.lock(); + } + + if (!coordinator) { + s_coordinators_per_path[path] = coordinator = std::make_shared(); + } + + return coordinator; +} + +std::shared_ptr RealmCoordinator::get_existing_coordinator(StringData path) +{ + std::lock_guard lock(s_coordinator_mutex); + auto it = s_coordinators_per_path.find(path); + return it == s_coordinators_per_path.end() ? nullptr : it->second.lock(); +} + +std::shared_ptr RealmCoordinator::get_realm(Realm::Config config) +{ + std::lock_guard lock(m_realm_mutex); + if (!m_notifier && m_cached_realms.empty()) { + m_config = config; + if (!config.read_only) { + m_notifier = std::make_unique(*this); + } + } + else { + if (m_config.read_only != config.read_only) { + throw MismatchedConfigException("Realm at path already opened with different read permissions."); + } + if (m_config.in_memory != config.in_memory) { + throw MismatchedConfigException("Realm at path already opened with different inMemory settings."); + } + if (m_config.encryption_key != config.encryption_key) { + throw MismatchedConfigException("Realm at path already opened with a different encryption key."); + } + if (m_config.schema_version != config.schema_version && config.schema_version != ObjectStore::NotVersioned) { + throw MismatchedConfigException("Realm at path already opened with different schema version."); + } + // FIXME: verify that schema is compatible + // Needs to verify that all tables present in both are identical, and + // then updated m_config with any tables present in config but not in + // it + // Public API currently doesn't make it possible to have non-matching + // schemata so it's not a huge issue + if ((false) && m_config.schema != config.schema) { + throw MismatchedConfigException("Realm at path already opened with different schema"); + } + } + + auto thread_id = std::this_thread::get_id(); + if (config.cache) { + for (auto& weakRealm : m_cached_realms) { + // can be null if we jumped in between ref count hitting zero and + // unregister_realm() getting the lock + if (auto realm = weakRealm.lock()) { + if (realm->thread_id() == thread_id) { + return realm; + } + } + } + } + + auto realm = std::make_shared(config); + realm->init(shared_from_this()); + if (m_notifier) { + m_notifier->add_realm(realm.get()); + } + if (config.cache) { + m_cached_realms.push_back(realm); + } + return realm; +} + +const Schema* RealmCoordinator::get_schema() const noexcept +{ + return m_cached_realms.empty() ? nullptr : m_config.schema.get(); +} + +RealmCoordinator::RealmCoordinator() = default; + +RealmCoordinator::~RealmCoordinator() +{ + std::lock_guard coordinator_lock(s_coordinator_mutex); + for (auto it = s_coordinators_per_path.begin(); it != s_coordinators_per_path.end(); ) { + if (it->second.expired()) { + it = s_coordinators_per_path.erase(it); + } + else { + ++it; + } + } +} + +void RealmCoordinator::unregister_realm(Realm* realm) +{ + std::lock_guard lock(m_realm_mutex); + if (m_notifier) { + m_notifier->remove_realm(realm); + } + for (size_t i = 0; i < m_cached_realms.size(); ++i) { + if (m_cached_realms[i].expired()) { + if (i + 1 < m_cached_realms.size()) { + m_cached_realms[i] = std::move(m_cached_realms.back()); + } + m_cached_realms.pop_back(); + } + } +} + +void RealmCoordinator::clear_cache() +{ + std::vector realms_to_close; + + { + std::lock_guard lock(s_coordinator_mutex); + + // Gather a list of all of the realms which will be removed + for (auto& weak_coordinator : s_coordinators_per_path) { + auto coordinator = weak_coordinator.second.lock(); + if (!coordinator) { + continue; + } + + for (auto& cached_realm : coordinator->m_cached_realms) { + if (auto realm = cached_realm.lock()) { + realms_to_close.push_back(realm); + } + } + } + + s_coordinators_per_path.clear(); + } + + // Close all of the previously cached Realms. This can't be done while + // s_coordinator_mutex is held as it may try to re-lock it. + for (auto& realm : realms_to_close) { + realm->close(); + } +} + +void RealmCoordinator::send_commit_notifications() +{ + REALM_ASSERT(!m_config.read_only); + m_notifier->notify_others(); +} diff --git a/src/impl/realm_coordinator.hpp b/src/impl/realm_coordinator.hpp new file mode 100644 index 00000000..f2adc69f --- /dev/null +++ b/src/impl/realm_coordinator.hpp @@ -0,0 +1,77 @@ +//////////////////////////////////////////////////////////////////////////// +// +// Copyright 2015 Realm Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +//////////////////////////////////////////////////////////////////////////// + +#ifndef REALM_COORDINATOR_HPP +#define REALM_COORDINATOR_HPP + +#include "shared_realm.hpp" + +#include + +namespace realm { +namespace _impl { +class ExternalCommitHelper; + +// RealmCoordinator manages the weak cache of Realm instances and communication +// between per-thread Realm instances for a given file +class RealmCoordinator : public std::enable_shared_from_this { +public: + // Get the coordinator for the given path, creating it if neccesary + static std::shared_ptr get_coordinator(StringData path); + // Get the coordinator for the given path, or null if there is none + static std::shared_ptr get_existing_coordinator(StringData path); + + // Get a thread-local shared Realm with the given configuration + // If the Realm is already open on another thread, validates that the given + // configuration is compatible with the existing one + std::shared_ptr get_realm(Realm::Config config); + + const Schema* get_schema() const noexcept; + uint64_t get_schema_version() const noexcept { return m_config.schema_version; } + const std::string& get_path() const noexcept { return m_config.path; } + + // Asyncronously call notify() on every Realm instance for this coordinator's + // path, including those in other processes + void send_commit_notifications(); + + // Clear the weak Realm cache for all paths + // Should only be called in test code, as continuing to use the previously + // cached instances will have odd results + static void clear_cache(); + + // Explicit constructor/destructor needed for the unique_ptrs to forward-declared types + RealmCoordinator(); + ~RealmCoordinator(); + + // Called by Realm's destructor to ensure the cache is cleaned up promptly + // Do not call directly + void unregister_realm(Realm* realm); + +private: + Realm::Config m_config; + + std::mutex m_realm_mutex; + std::vector> m_cached_realms; + + std::unique_ptr<_impl::ExternalCommitHelper> m_notifier; +}; + +} // namespace _impl +} // namespace realm + +#endif /* REALM_COORDINATOR_HPP */ diff --git a/src/results.cpp b/src/results.cpp index 91a4cc22..a49559ca 100644 --- a/src/results.cpp +++ b/src/results.cpp @@ -18,6 +18,8 @@ #include "results.hpp" +#include "object_store.hpp" + #include using namespace realm; diff --git a/src/shared_realm.cpp b/src/shared_realm.cpp index 30e8f928..85fc8214 100644 --- a/src/shared_realm.cpp +++ b/src/shared_realm.cpp @@ -18,8 +18,10 @@ #include "shared_realm.hpp" -#include "external_commit_helper.hpp" #include "binding_context.hpp" +#include "external_commit_helper.hpp" +#include "object_store.hpp" +#include "realm_coordinator.hpp" #include "schema.hpp" #include "transact_log_handler.hpp" @@ -31,8 +33,6 @@ using namespace realm; using namespace realm::_impl; -RealmCache Realm::s_global_cache; - Realm::Config::Config(const Config& c) : path(c.path) , read_only(c.read_only) @@ -48,7 +48,7 @@ Realm::Config::Config(const Config& c) } } -Realm::Config::Config() = default; +Realm::Config::Config() : schema_version(ObjectStore::NotVersioned) { } Realm::Config::Config(Config&&) = default; Realm::Config::~Config() = default; @@ -104,9 +104,58 @@ Realm::Realm(Config config) } } -Realm::~Realm() { - if (m_notifier) { // might not exist yet if an error occurred during init - m_notifier->remove_realm(this); +void Realm::init(std::shared_ptr coordinator) +{ + m_coordinator = std::move(coordinator); + + // if there is an existing realm at the current path steal its schema/column mapping + if (auto existing = m_coordinator->get_schema()) { + m_config.schema = std::make_unique(*existing); + return; + } + + try { + // otherwise get the schema from the group + auto target_schema = std::move(m_config.schema); + auto target_schema_version = m_config.schema_version; + m_config.schema_version = ObjectStore::get_schema_version(read_group()); + m_config.schema = std::make_unique(ObjectStore::schema_from_group(read_group())); + + // if a target schema is supplied, verify that it matches or migrate to + // it, as neeeded + if (target_schema) { + if (m_config.read_only) { + if (m_config.schema_version == ObjectStore::NotVersioned) { + throw UnitializedRealmException("Can't open an un-initialized Realm without a Schema"); + } + target_schema->validate(); + ObjectStore::verify_schema(*m_config.schema, *target_schema, true); + m_config.schema = std::move(target_schema); + } + else { + update_schema(std::move(target_schema), target_schema_version); + } + + if (!m_config.read_only) { + // End the read transaction created to validation/update the + // schema to avoid pinning the version even if the user never + // actually reads data + invalidate(); + } + } + } + catch (...) { + // Trying to unregister from the coordinator before we finish + // construction will result in a deadlock + m_coordinator = nullptr; + throw; + } +} + +Realm::~Realm() +{ + if (m_coordinator) { + m_coordinator->unregister_realm(this); } } @@ -120,85 +169,7 @@ Group *Realm::read_group() SharedRealm Realm::get_shared_realm(Config config) { - if (config.cache) { - if (SharedRealm realm = s_global_cache.get_realm(config.path)) { - if (realm->config().read_only != config.read_only) { - throw MismatchedConfigException("Realm at path already opened with different read permissions."); - } - if (realm->config().in_memory != config.in_memory) { - throw MismatchedConfigException("Realm at path already opened with different inMemory settings."); - } - if (realm->config().encryption_key != config.encryption_key) { - throw MismatchedConfigException("Realm at path already opened with a different encryption key."); - } - if (realm->config().schema_version != config.schema_version && config.schema_version != ObjectStore::NotVersioned) { - throw MismatchedConfigException("Realm at path already opened with different schema version."); - } - // FIXME - enable schma comparison - /*if (realm->config().schema != config.schema) { - throw MismatchedConfigException("Realm at path already opened with different schema"); - }*/ - realm->m_config.migration_function = config.migration_function; - - return realm; - } - } - - SharedRealm realm(new Realm(std::move(config))); - - auto target_schema = std::move(realm->m_config.schema); - auto target_schema_version = realm->m_config.schema_version; - realm->m_config.schema_version = ObjectStore::get_schema_version(realm->read_group()); - - // we want to ensure we are only initializing a single realm at a time - static std::mutex s_init_mutex; - std::lock_guard lock(s_init_mutex); - if (auto existing = s_global_cache.get_any_realm(realm->config().path)) { - // if there is an existing realm at the current path steal its schema/column mapping - // FIXME - need to validate that schemas match - realm->m_config.schema = std::make_unique(*existing->m_config.schema); - - if (!realm->m_config.read_only) { - realm->m_notifier = existing->m_notifier; - realm->m_notifier->add_realm(realm.get()); - } - } - else { - if (!realm->m_config.read_only) { - realm->m_notifier = std::make_shared(realm.get()); - } - - // otherwise get the schema from the group - realm->m_config.schema = std::make_unique(ObjectStore::schema_from_group(realm->read_group())); - - // if a target schema is supplied, verify that it matches or migrate to - // it, as neeeded - if (target_schema) { - if (realm->m_config.read_only) { - if (realm->m_config.schema_version == ObjectStore::NotVersioned) { - throw UnitializedRealmException("Can't open an un-initialized Realm without a Schema"); - } - target_schema->validate(); - ObjectStore::verify_schema(*realm->m_config.schema, *target_schema, true); - realm->m_config.schema = std::move(target_schema); - } - else { - realm->update_schema(std::move(target_schema), target_schema_version); - } - - if (!realm->m_config.read_only) { - // End the read transaction created to validation/update the - // schema to avoid pinning the version even if the user never - // actually reads data - realm->invalidate(); - } - } - } - - if (config.cache) { - s_global_cache.cache_realm(realm, realm->m_thread_id); - } - return realm; + return RealmCoordinator::get_coordinator(config.path)->get_realm(config); } void Realm::update_schema(std::unique_ptr schema, uint64_t version) @@ -322,7 +293,7 @@ void Realm::commit_transaction() m_in_transaction = false; transaction::commit(*m_shared_group, *m_history, m_binding_context.get()); - m_notifier->notify_others(); + m_coordinator->send_commit_notifications(); } void Realm::cancel_transaction() @@ -392,7 +363,6 @@ void Realm::notify() } } - bool Realm::refresh() { verify_thread(); @@ -421,8 +391,9 @@ bool Realm::refresh() uint64_t Realm::get_schema_version(const realm::Realm::Config &config) { - if (auto existing_realm = s_global_cache.get_any_realm(config.path)) { - return existing_realm->config().schema_version; + auto coordinator = RealmCoordinator::get_existing_coordinator(config.path); + if (coordinator) { + return coordinator->get_schema_version(); } return ObjectStore::get_schema_version(Realm(config).read_group()); @@ -430,97 +401,16 @@ uint64_t Realm::get_schema_version(const realm::Realm::Config &config) void Realm::close() { - if (m_notifier) { - m_notifier->remove_realm(this); + invalidate(); + + if (m_coordinator) { + m_coordinator->unregister_realm(this); } m_group = nullptr; m_shared_group = nullptr; m_history = nullptr; m_read_only_group = nullptr; - m_notifier = nullptr; m_binding_context = nullptr; -} - -SharedRealm RealmCache::get_realm(const std::string &path, std::thread::id thread_id) -{ - std::lock_guard lock(m_mutex); - - auto path_iter = m_cache.find(path); - if (path_iter == m_cache.end()) { - return SharedRealm(); - } - - auto thread_iter = path_iter->second.find(thread_id); - if (thread_iter == path_iter->second.end()) { - return SharedRealm(); - } - - return thread_iter->second.lock(); -} - -SharedRealm RealmCache::get_any_realm(const std::string &path) -{ - std::lock_guard lock(m_mutex); - - auto path_iter = m_cache.find(path); - if (path_iter == m_cache.end()) { - return SharedRealm(); - } - - auto thread_iter = path_iter->second.begin(); - while (thread_iter != path_iter->second.end()) { - if (auto realm = thread_iter->second.lock()) { - return realm; - } - path_iter->second.erase(thread_iter++); - } - - return SharedRealm(); -} - -void RealmCache::remove(const std::string &path, std::thread::id thread_id) -{ - std::lock_guard lock(m_mutex); - - auto path_iter = m_cache.find(path); - if (path_iter == m_cache.end()) { - return; - } - - auto thread_iter = path_iter->second.find(thread_id); - if (thread_iter != path_iter->second.end()) { - path_iter->second.erase(thread_iter); - } - - if (path_iter->second.size() == 0) { - m_cache.erase(path_iter); - } -} - -void RealmCache::cache_realm(SharedRealm &realm, std::thread::id thread_id) -{ - std::lock_guard lock(m_mutex); - - auto path_iter = m_cache.find(realm->config().path); - if (path_iter == m_cache.end()) { - m_cache.emplace(realm->config().path, std::map{{thread_id, realm}}); - } - else { - path_iter->second.emplace(thread_id, realm); - } -} - -void RealmCache::clear() -{ - std::lock_guard lock(m_mutex); - for (auto const& path : m_cache) { - for (auto const& thread : path.second) { - if (auto realm = thread.second.lock()) { - realm->close(); - } - } - } - - m_cache.clear(); + m_coordinator = nullptr; } diff --git a/src/shared_realm.hpp b/src/shared_realm.hpp index ddc625dd..79f55496 100644 --- a/src/shared_realm.hpp +++ b/src/shared_realm.hpp @@ -22,20 +22,24 @@ #include "object_store.hpp" #include -#include +#include #include #include namespace realm { - class ClientHistory; - class Realm; - class RealmCache; class BindingContext; + class ClientHistory; + class Group; + class Realm; + class RealmDelegate; + class Schema; + class SharedGroup; typedef std::shared_ptr SharedRealm; typedef std::weak_ptr WeakRealm; namespace _impl { class ExternalCommitHelper; + class RealmCoordinator; } class Realm : public std::enable_shared_from_this @@ -53,7 +57,7 @@ namespace realm { std::vector encryption_key; std::unique_ptr schema; - uint64_t schema_version = ObjectStore::NotVersioned; + uint64_t schema_version; MigrationFunction migration_function; @@ -109,9 +113,10 @@ namespace realm { ~Realm(); - private: + void init(std::shared_ptr<_impl::RealmCoordinator> coordinator); Realm(Config config); + private: Config m_config; std::thread::id m_thread_id = std::this_thread::get_id(); bool m_in_transaction = false; @@ -123,28 +128,13 @@ namespace realm { Group *m_group = nullptr; - std::shared_ptr<_impl::ExternalCommitHelper> m_notifier; + std::shared_ptr<_impl::RealmCoordinator> m_coordinator; public: std::unique_ptr m_binding_context; // FIXME private Group *read_group(); - static RealmCache s_global_cache; - }; - - class RealmCache - { - public: - SharedRealm get_realm(const std::string &path, std::thread::id thread_id = std::this_thread::get_id()); - SharedRealm get_any_realm(const std::string &path); - void remove(const std::string &path, std::thread::id thread_id); - void cache_realm(SharedRealm &realm, std::thread::id thread_id = std::this_thread::get_id()); - void clear(); - - private: - std::map> m_cache; - std::mutex m_mutex; }; class RealmFileException : public std::runtime_error { From c3a9489b0250dabbbe0884553408fb52816be8e5 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Thu, 20 Aug 2015 12:27:23 -0700 Subject: [PATCH 02/17] Fix a potential deadlock when opening a realm --- src/impl/realm_coordinator.cpp | 21 +++++++++++++-------- src/impl/realm_coordinator.hpp | 3 ++- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/impl/realm_coordinator.cpp b/src/impl/realm_coordinator.cpp index 82a6dff1..297a42d3 100644 --- a/src/impl/realm_coordinator.cpp +++ b/src/impl/realm_coordinator.cpp @@ -24,6 +24,11 @@ using namespace realm; using namespace realm::_impl; +struct realm::_impl::CachedRealm { + std::weak_ptr realm; + std::thread::id thread_id; +}; + static std::mutex s_coordinator_mutex; static std::map> s_coordinators_per_path; @@ -86,11 +91,11 @@ std::shared_ptr RealmCoordinator::get_realm(Realm::Config config) auto thread_id = std::this_thread::get_id(); if (config.cache) { - for (auto& weakRealm : m_cached_realms) { - // can be null if we jumped in between ref count hitting zero and - // unregister_realm() getting the lock - if (auto realm = weakRealm.lock()) { - if (realm->thread_id() == thread_id) { + for (auto& cachedRealm : m_cached_realms) { + if (cachedRealm.thread_id == thread_id) { + // can be null if we jumped in between ref count hitting zero and + // unregister_realm() getting the lock + if (auto realm = cachedRealm.realm.lock()) { return realm; } } @@ -103,7 +108,7 @@ std::shared_ptr RealmCoordinator::get_realm(Realm::Config config) m_notifier->add_realm(realm.get()); } if (config.cache) { - m_cached_realms.push_back(realm); + m_cached_realms.push_back({realm, thread_id}); } return realm; } @@ -135,7 +140,7 @@ void RealmCoordinator::unregister_realm(Realm* realm) m_notifier->remove_realm(realm); } for (size_t i = 0; i < m_cached_realms.size(); ++i) { - if (m_cached_realms[i].expired()) { + if (m_cached_realms[i].realm.expired()) { if (i + 1 < m_cached_realms.size()) { m_cached_realms[i] = std::move(m_cached_realms.back()); } @@ -159,7 +164,7 @@ void RealmCoordinator::clear_cache() } for (auto& cached_realm : coordinator->m_cached_realms) { - if (auto realm = cached_realm.lock()) { + if (auto realm = cached_realm.realm.lock()) { realms_to_close.push_back(realm); } } diff --git a/src/impl/realm_coordinator.hpp b/src/impl/realm_coordinator.hpp index f2adc69f..5813d93f 100644 --- a/src/impl/realm_coordinator.hpp +++ b/src/impl/realm_coordinator.hpp @@ -26,6 +26,7 @@ namespace realm { namespace _impl { class ExternalCommitHelper; +struct CachedRealm; // RealmCoordinator manages the weak cache of Realm instances and communication // between per-thread Realm instances for a given file @@ -66,7 +67,7 @@ private: Realm::Config m_config; std::mutex m_realm_mutex; - std::vector> m_cached_realms; + std::vector m_cached_realms; std::unique_ptr<_impl::ExternalCommitHelper> m_notifier; }; From d6daa052e83c4ac5b284169a383710f2768b1651 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Fri, 21 Aug 2015 12:27:27 -0700 Subject: [PATCH 03/17] Decouple Realm instance tracking from interprocess notifications --- src/CMakeLists.txt | 2 + src/impl/apple/cached_realm.cpp | 93 +++++++++++++++++++++++ src/impl/apple/cached_realm.hpp | 65 ++++++++++++++++ src/impl/apple/external_commit_helper.cpp | 44 +---------- src/impl/apple/external_commit_helper.hpp | 13 ---- src/impl/realm_coordinator.cpp | 39 +++++----- src/impl/realm_coordinator.hpp | 5 +- 7 files changed, 182 insertions(+), 79 deletions(-) create mode 100644 src/impl/apple/cached_realm.cpp create mode 100644 src/impl/apple/cached_realm.hpp diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 2c9eade2..ffb715a2 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -28,8 +28,10 @@ set(HEADERS if(APPLE) include_directories(impl/apple) list(APPEND SOURCES + impl/apple/cached_realm.cpp impl/apple/external_commit_helper.cpp) list(APPEND HEADERS + impl/apple/cached_realm.hpp impl/apple/external_commit_helper.hpp) find_library(CF_LIBRARY CoreFoundation) endif() diff --git a/src/impl/apple/cached_realm.cpp b/src/impl/apple/cached_realm.cpp new file mode 100644 index 00000000..35317b3c --- /dev/null +++ b/src/impl/apple/cached_realm.cpp @@ -0,0 +1,93 @@ +//////////////////////////////////////////////////////////////////////////// +// +// Copyright 2015 Realm Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +//////////////////////////////////////////////////////////////////////////// + +#include "cached_realm.hpp" + +#include "shared_realm.hpp" + +using namespace realm; +using namespace realm::_impl; + +CachedRealm::CachedRealm(const std::shared_ptr& realm, bool cache) +: m_realm(realm) +, m_cache(cache) +{ + struct RefCountedWeakPointer { + std::weak_ptr realm; + std::atomic ref_count = {1}; + }; + + CFRunLoopSourceContext ctx{}; + ctx.info = new RefCountedWeakPointer{realm}; + ctx.perform = [](void* info) { + if (auto realm = static_cast(info)->realm.lock()) { + realm->notify(); + } + }; + ctx.retain = [](const void* info) { + static_cast(const_cast(info))->ref_count.fetch_add(1, std::memory_order_relaxed); + return info; + }; + ctx.release = [](const void* info) { + auto ptr = static_cast(const_cast(info)); + if (ptr->ref_count.fetch_add(-1, std::memory_order_acq_rel) == 1) { + delete ptr; + } + }; + + m_runloop = CFRunLoopGetCurrent(); + CFRetain(m_runloop); + m_signal = CFRunLoopSourceCreate(kCFAllocatorDefault, 0, &ctx); + CFRunLoopAddSource(m_runloop, m_signal, kCFRunLoopDefaultMode); +} + +CachedRealm::CachedRealm(CachedRealm&& rgt) +{ + *this = std::move(rgt); +} + +CachedRealm& CachedRealm::operator=(CachedRealm&& rgt) +{ + m_realm = std::move(rgt.m_realm); + m_thread_id = rgt.m_thread_id; + m_cache = rgt.m_cache; + m_runloop = rgt.m_runloop; + m_signal = rgt.m_signal; + rgt.m_runloop = nullptr; + rgt.m_signal = nullptr; + + return *this; +} + +CachedRealm::~CachedRealm() +{ + if (m_signal) { + CFRunLoopSourceInvalidate(m_signal); + CFRelease(m_signal); + CFRelease(m_runloop); + } +} + +void CachedRealm::notify() +{ + CFRunLoopSourceSignal(m_signal); + // Signalling the source makes it run the next time the runloop gets + // to it, but doesn't make the runloop start if it's currently idle + // waiting for events + CFRunLoopWakeUp(m_runloop); +} diff --git a/src/impl/apple/cached_realm.hpp b/src/impl/apple/cached_realm.hpp new file mode 100644 index 00000000..7b1ddd3e --- /dev/null +++ b/src/impl/apple/cached_realm.hpp @@ -0,0 +1,65 @@ +//////////////////////////////////////////////////////////////////////////// +// +// Copyright 2015 Realm Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +//////////////////////////////////////////////////////////////////////////// + +#ifndef REALM_CACHED_REALM_HPP +#define REALM_CACHED_REALM_HPP + +#include +#include +#include + +namespace realm { +class Realm; + +namespace _impl { + +class CachedRealm { +public: + CachedRealm(const std::shared_ptr& realm, bool cache); + ~CachedRealm(); + + CachedRealm(CachedRealm&&); + CachedRealm& operator=(CachedRealm&&); + + CachedRealm(const CachedRealm&) = delete; + CachedRealm& operator=(const CachedRealm&) = delete; + + // Get a strong reference to the cached realm + std::shared_ptr realm() const { return m_realm.lock(); } + + // Does this CachedRealm store a Realm instance that should be used on the current thread? + bool is_cached_for_current_thread() const { return m_cache && m_thread_id == std::this_thread::get_id(); } + + bool expired() const { return m_realm.expired(); } + + // Asyncronously call notify() on the Realm on the appropriate thread + void notify(); + +private: + std::weak_ptr m_realm; + std::thread::id m_thread_id = std::this_thread::get_id(); + bool m_cache = false; + + CFRunLoopRef m_runloop; + CFRunLoopSourceRef m_signal; +}; + +} // namespace _impl +} // namespace realm + +#endif // REALM_CACHED_REALM_HPP diff --git a/src/impl/apple/external_commit_helper.cpp b/src/impl/apple/external_commit_helper.cpp index c22fe5df..33608ace 100644 --- a/src/impl/apple/external_commit_helper.cpp +++ b/src/impl/apple/external_commit_helper.cpp @@ -143,45 +143,10 @@ ExternalCommitHelper::ExternalCommitHelper(RealmCoordinator& parent) ExternalCommitHelper::~ExternalCommitHelper() { - REALM_ASSERT_DEBUG(m_realms.empty()); notify_fd(m_shutdown_write_fd); m_thread.wait(); // Wait for the thread to exit } -void ExternalCommitHelper::add_realm(realm::Realm* realm) -{ - std::lock_guard lock(m_realms_mutex); - - // Create the runloop source - CFRunLoopSourceContext ctx{}; - ctx.info = realm; - ctx.perform = [](void* info) { - static_cast(info)->notify(); - }; - - CFRunLoopRef runloop = CFRunLoopGetCurrent(); - CFRetain(runloop); - CFRunLoopSourceRef signal = CFRunLoopSourceCreate(kCFAllocatorDefault, 0, &ctx); - CFRunLoopAddSource(runloop, signal, kCFRunLoopDefaultMode); - - m_realms.push_back({realm, runloop, signal}); -} - -void ExternalCommitHelper::remove_realm(realm::Realm* realm) -{ - std::lock_guard lock(m_realms_mutex); - for (auto it = m_realms.begin(); it != m_realms.end(); ++it) { - if (it->realm == realm) { - CFRunLoopSourceInvalidate(it->signal); - CFRelease(it->signal); - CFRelease(it->runloop); - m_realms.erase(it); - return; - } - } - REALM_TERMINATE("Realm not registered"); -} - void ExternalCommitHelper::listen() { pthread_setname_np("RLMRealm notification listener"); @@ -216,14 +181,7 @@ void ExternalCommitHelper::listen() } assert(event.ident == (uint32_t)m_notify_fd); - std::lock_guard lock(m_realms_mutex); - for (auto const& realm : m_realms) { - CFRunLoopSourceSignal(realm.signal); - // Signalling the source makes it run the next time the runloop gets - // to it, but doesn't make the runloop start if it's currently idle - // waiting for events - CFRunLoopWakeUp(realm.runloop); - } + m_parent.on_change(); } } diff --git a/src/impl/apple/external_commit_helper.hpp b/src/impl/apple/external_commit_helper.hpp index 9f31d730..624b8618 100644 --- a/src/impl/apple/external_commit_helper.hpp +++ b/src/impl/apple/external_commit_helper.hpp @@ -20,9 +20,7 @@ #define REALM_EXTERNAL_COMMIT_HELPER_HPP #include -#include #include -#include #include namespace realm { @@ -31,16 +29,12 @@ class Realm; namespace _impl { class RealmCoordinator; -// FIXME: split IPC from the local cross-thread stuff -// both are platform-specific but need to be useable separately class ExternalCommitHelper { public: ExternalCommitHelper(RealmCoordinator& parent); ~ExternalCommitHelper(); void notify_others(); - void add_realm(Realm* realm); - void remove_realm(Realm* realm); private: // A RAII holder for a file descriptor which automatically closes the wrapped @@ -75,13 +69,6 @@ private: RealmCoordinator& m_parent; - // Currently registered realms and the signal for delivering notifications - // to them - std::vector m_realms; - - // Mutex which guards m_realms - std::mutex m_realms_mutex; - // The listener thread std::future m_thread; diff --git a/src/impl/realm_coordinator.cpp b/src/impl/realm_coordinator.cpp index 297a42d3..a25a5036 100644 --- a/src/impl/realm_coordinator.cpp +++ b/src/impl/realm_coordinator.cpp @@ -18,17 +18,13 @@ #include "realm_coordinator.hpp" +#include "cached_realm.hpp" #include "external_commit_helper.hpp" #include "object_store.hpp" using namespace realm; using namespace realm::_impl; -struct realm::_impl::CachedRealm { - std::weak_ptr realm; - std::thread::id thread_id; -}; - static std::mutex s_coordinator_mutex; static std::map> s_coordinators_per_path; @@ -59,9 +55,9 @@ std::shared_ptr RealmCoordinator::get_existing_coordinator(Str std::shared_ptr RealmCoordinator::get_realm(Realm::Config config) { std::lock_guard lock(m_realm_mutex); - if (!m_notifier && m_cached_realms.empty()) { + if ((!m_config.read_only && !m_notifier) || (m_config.read_only && m_cached_realms.empty())) { m_config = config; - if (!config.read_only) { + if (!config.read_only && !m_notifier) { m_notifier = std::make_unique(*this); } } @@ -89,13 +85,12 @@ std::shared_ptr RealmCoordinator::get_realm(Realm::Config config) } } - auto thread_id = std::this_thread::get_id(); if (config.cache) { for (auto& cachedRealm : m_cached_realms) { - if (cachedRealm.thread_id == thread_id) { + if (cachedRealm.is_cached_for_current_thread()) { // can be null if we jumped in between ref count hitting zero and // unregister_realm() getting the lock - if (auto realm = cachedRealm.realm.lock()) { + if (auto realm = cachedRealm.realm()) { return realm; } } @@ -104,12 +99,7 @@ std::shared_ptr RealmCoordinator::get_realm(Realm::Config config) auto realm = std::make_shared(config); realm->init(shared_from_this()); - if (m_notifier) { - m_notifier->add_realm(realm.get()); - } - if (config.cache) { - m_cached_realms.push_back({realm, thread_id}); - } + m_cached_realms.emplace_back(realm, m_config.cache); return realm; } @@ -133,14 +123,11 @@ RealmCoordinator::~RealmCoordinator() } } -void RealmCoordinator::unregister_realm(Realm* realm) +void RealmCoordinator::unregister_realm(Realm*) { std::lock_guard lock(m_realm_mutex); - if (m_notifier) { - m_notifier->remove_realm(realm); - } for (size_t i = 0; i < m_cached_realms.size(); ++i) { - if (m_cached_realms[i].realm.expired()) { + if (m_cached_realms[i].expired()) { if (i + 1 < m_cached_realms.size()) { m_cached_realms[i] = std::move(m_cached_realms.back()); } @@ -164,7 +151,7 @@ void RealmCoordinator::clear_cache() } for (auto& cached_realm : coordinator->m_cached_realms) { - if (auto realm = cached_realm.realm.lock()) { + if (auto realm = cached_realm.realm()) { realms_to_close.push_back(realm); } } @@ -185,3 +172,11 @@ void RealmCoordinator::send_commit_notifications() REALM_ASSERT(!m_config.read_only); m_notifier->notify_others(); } + +void RealmCoordinator::on_change() +{ + std::lock_guard lock(m_realm_mutex); + for (auto& realm : m_cached_realms) { + realm.notify(); + } +} diff --git a/src/impl/realm_coordinator.hpp b/src/impl/realm_coordinator.hpp index 5813d93f..ac889412 100644 --- a/src/impl/realm_coordinator.hpp +++ b/src/impl/realm_coordinator.hpp @@ -25,8 +25,8 @@ namespace realm { namespace _impl { +class CachedRealm; class ExternalCommitHelper; -struct CachedRealm; // RealmCoordinator manages the weak cache of Realm instances and communication // between per-thread Realm instances for a given file @@ -63,6 +63,9 @@ public: // Do not call directly void unregister_realm(Realm* realm); + // Called by m_notifier when there's a new commit to send notifications for + void on_change(); + private: Realm::Config m_config; From 7a0c83929f9c19e1619637fcb533eafa3be58eb5 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Mon, 23 Nov 2015 14:45:33 -0800 Subject: [PATCH 04/17] Use an unordered map for the Realm coordinator cache --- src/impl/realm_coordinator.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/impl/realm_coordinator.cpp b/src/impl/realm_coordinator.cpp index a25a5036..f3d6c1d8 100644 --- a/src/impl/realm_coordinator.cpp +++ b/src/impl/realm_coordinator.cpp @@ -22,11 +22,13 @@ #include "external_commit_helper.hpp" #include "object_store.hpp" +#include + using namespace realm; using namespace realm::_impl; static std::mutex s_coordinator_mutex; -static std::map> s_coordinators_per_path; +static std::unordered_map> s_coordinators_per_path; std::shared_ptr RealmCoordinator::get_coordinator(StringData path) { From e30e2ff278b8eb4122b6a0ccbf8146a25d5ad514 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Mon, 23 Nov 2015 14:46:51 -0800 Subject: [PATCH 05/17] Simplify RealmCoordinator::get_coordinator() --- src/impl/realm_coordinator.cpp | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/impl/realm_coordinator.cpp b/src/impl/realm_coordinator.cpp index f3d6c1d8..cf3289b4 100644 --- a/src/impl/realm_coordinator.cpp +++ b/src/impl/realm_coordinator.cpp @@ -33,17 +33,14 @@ static std::unordered_map> s_coordi std::shared_ptr RealmCoordinator::get_coordinator(StringData path) { std::lock_guard lock(s_coordinator_mutex); - std::shared_ptr coordinator; - auto it = s_coordinators_per_path.find(path); - if (it != s_coordinators_per_path.end()) { - coordinator = it->second.lock(); - } - - if (!coordinator) { - s_coordinators_per_path[path] = coordinator = std::make_shared(); + auto& weak_coordinator = s_coordinators_per_path[path]; + if (auto coordinator = weak_coordinator.lock()) { + return coordinator; } + auto coordinator = std::make_shared(); + weak_coordinator = coordinator; return coordinator; } From ebfca16d003a9cd029fa733413652cf7944e59c0 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Mon, 30 Nov 2015 10:42:09 -0800 Subject: [PATCH 06/17] Eliminate a config copy when opening Realms --- src/shared_realm.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared_realm.cpp b/src/shared_realm.cpp index 85fc8214..45960d8b 100644 --- a/src/shared_realm.cpp +++ b/src/shared_realm.cpp @@ -169,7 +169,7 @@ Group *Realm::read_group() SharedRealm Realm::get_shared_realm(Config config) { - return RealmCoordinator::get_coordinator(config.path)->get_realm(config); + return RealmCoordinator::get_coordinator(config.path)->get_realm(std::move(config)); } void Realm::update_schema(std::unique_ptr schema, uint64_t version) From 9b8a0d53460b9dd9f29176cc5e35f3efe1e9134a Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Wed, 2 Dec 2015 11:53:06 -0800 Subject: [PATCH 07/17] Log uncaught exceptions in the notifier thread By default the thread just silently goes away. --- src/impl/apple/external_commit_helper.cpp | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/impl/apple/external_commit_helper.cpp b/src/impl/apple/external_commit_helper.cpp index 33608ace..f66d4e37 100644 --- a/src/impl/apple/external_commit_helper.cpp +++ b/src/impl/apple/external_commit_helper.cpp @@ -20,6 +20,7 @@ #include "realm_coordinator.hpp" +#include #include #include #include @@ -138,7 +139,21 @@ ExternalCommitHelper::ExternalCommitHelper(RealmCoordinator& parent) m_shutdown_read_fd = pipeFd[0]; m_shutdown_write_fd = pipeFd[1]; - m_thread = std::async(std::launch::async, [=] { listen(); }); + m_thread = std::async(std::launch::async, [=] { + try { + listen(); + } + catch (std::exception const& e) { + fprintf(stderr, "uncaught exception in notifier thread: %s: %s\n", typeid(e).name(), e.what()); + asl_log(nullptr, nullptr, ASL_LEVEL_ERR, "uncaught exception in notifier thread: %s: %s", typeid(e).name(), e.what()); + throw; + } + catch (...) { + fprintf(stderr, "uncaught exception in notifier thread\n"); + asl_log(nullptr, nullptr, ASL_LEVEL_ERR, "uncaught exception in notifier thread"); + throw; + } + }); } ExternalCommitHelper::~ExternalCommitHelper() From 89bd55a5351a7e4f2454dbfa31a5e023f17c8049 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Thu, 3 Dec 2015 08:55:29 -0800 Subject: [PATCH 08/17] Actually remove the Realm from the cache when close() is called --- src/impl/apple/cached_realm.cpp | 1 + src/impl/apple/cached_realm.hpp | 8 ++++++++ src/impl/realm_coordinator.cpp | 15 +++++++++------ 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/impl/apple/cached_realm.cpp b/src/impl/apple/cached_realm.cpp index 35317b3c..03705d52 100644 --- a/src/impl/apple/cached_realm.cpp +++ b/src/impl/apple/cached_realm.cpp @@ -25,6 +25,7 @@ using namespace realm::_impl; CachedRealm::CachedRealm(const std::shared_ptr& realm, bool cache) : m_realm(realm) +, m_realm_key(realm.get()) , m_cache(cache) { struct RefCountedWeakPointer { diff --git a/src/impl/apple/cached_realm.hpp b/src/impl/apple/cached_realm.hpp index 7b1ddd3e..89806e95 100644 --- a/src/impl/apple/cached_realm.hpp +++ b/src/impl/apple/cached_realm.hpp @@ -45,14 +45,22 @@ public: // Does this CachedRealm store a Realm instance that should be used on the current thread? bool is_cached_for_current_thread() const { return m_cache && m_thread_id == std::this_thread::get_id(); } + // Has the Realm instance been destroyed? bool expired() const { return m_realm.expired(); } // Asyncronously call notify() on the Realm on the appropriate thread void notify(); + // Is this a CachedRealm for the given Realm instance? + // This should be used rather than lock() to avoid deadlocks when the + // reference from lock() is the last remaining one (due to another thread + // releasing them at the same time) + bool is_for_realm(Realm* realm) const { return realm == m_realm_key; } + private: std::weak_ptr m_realm; std::thread::id m_thread_id = std::this_thread::get_id(); + void* m_realm_key; bool m_cache = false; CFRunLoopRef m_runloop; diff --git a/src/impl/realm_coordinator.cpp b/src/impl/realm_coordinator.cpp index cf3289b4..cfcdd6ad 100644 --- a/src/impl/realm_coordinator.cpp +++ b/src/impl/realm_coordinator.cpp @@ -122,16 +122,19 @@ RealmCoordinator::~RealmCoordinator() } } -void RealmCoordinator::unregister_realm(Realm*) +void RealmCoordinator::unregister_realm(Realm* realm) { std::lock_guard lock(m_realm_mutex); for (size_t i = 0; i < m_cached_realms.size(); ++i) { - if (m_cached_realms[i].expired()) { - if (i + 1 < m_cached_realms.size()) { - m_cached_realms[i] = std::move(m_cached_realms.back()); - } - m_cached_realms.pop_back(); + auto& cached_realm = m_cached_realms[i]; + if (!cached_realm.expired() && !cached_realm.is_for_realm(realm)) { + continue; } + + if (i + 1 < m_cached_realms.size()) { + cached_realm = std::move(m_cached_realms.back()); + } + m_cached_realms.pop_back(); } } From 513b3d770c8b1333f411ac8b06504b6adc5d4a3c Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Thu, 3 Dec 2015 09:01:32 -0800 Subject: [PATCH 09/17] Add a short explanation of CachedRealm --- src/impl/apple/cached_realm.hpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/impl/apple/cached_realm.hpp b/src/impl/apple/cached_realm.hpp index 89806e95..6b114dc6 100644 --- a/src/impl/apple/cached_realm.hpp +++ b/src/impl/apple/cached_realm.hpp @@ -28,6 +28,11 @@ class Realm; namespace _impl { +// CachedRealm stores a weak reference to a Realm instance, along with all of +// the information about a Realm that needs to be accessed from other threads. +// This is needed to avoid forming strong references to the Realm instances on +// other threads, which can produce deadlocks when the last strong reference to +// a Realm instance is released from within a function holding the cache lock. class CachedRealm { public: CachedRealm(const std::shared_ptr& realm, bool cache); @@ -52,9 +57,6 @@ public: void notify(); // Is this a CachedRealm for the given Realm instance? - // This should be used rather than lock() to avoid deadlocks when the - // reference from lock() is the last remaining one (due to another thread - // releasing them at the same time) bool is_for_realm(Realm* realm) const { return realm == m_realm_key; } private: From 4c195c92e08258357eef3fd2c835759316dba23f Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Thu, 3 Dec 2015 12:13:19 -0800 Subject: [PATCH 10/17] Remove some unused cruft from ExternalCommitHelper --- src/impl/apple/external_commit_helper.hpp | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/impl/apple/external_commit_helper.hpp b/src/impl/apple/external_commit_helper.hpp index 624b8618..c6d795a0 100644 --- a/src/impl/apple/external_commit_helper.hpp +++ b/src/impl/apple/external_commit_helper.hpp @@ -19,9 +19,7 @@ #ifndef REALM_EXTERNAL_COMMIT_HELPER_HPP #define REALM_EXTERNAL_COMMIT_HELPER_HPP -#include #include -#include namespace realm { class Realm; @@ -59,12 +57,6 @@ private: FdHolder(FdHolder const&) = delete; }; - struct PerRealmInfo { - Realm* realm; - CFRunLoopRef runloop; - CFRunLoopSourceRef signal; - }; - void listen(); RealmCoordinator& m_parent; From 178c562f2ca675d4f8ed358922089454aabcacf6 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Thu, 3 Dec 2015 12:31:56 -0800 Subject: [PATCH 11/17] Add an untested non-Apple ExternalCommitHelper implementation --- src/impl/generic/external_commit_helper.cpp | 46 ++++++++++++++++++ src/impl/generic/external_commit_helper.hpp | 54 +++++++++++++++++++++ src/impl/realm_coordinator.hpp | 2 + 3 files changed, 102 insertions(+) create mode 100644 src/impl/generic/external_commit_helper.cpp create mode 100644 src/impl/generic/external_commit_helper.hpp diff --git a/src/impl/generic/external_commit_helper.cpp b/src/impl/generic/external_commit_helper.cpp new file mode 100644 index 00000000..66417642 --- /dev/null +++ b/src/impl/generic/external_commit_helper.cpp @@ -0,0 +1,46 @@ +//////////////////////////////////////////////////////////////////////////// +// +// Copyright 2015 Realm Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +//////////////////////////////////////////////////////////////////////////// + +#include "external_commit_helper.hpp" + +#include "realm_coordinator.hpp" + +#include +#include + +using namespace realm; +using namespace realm::_impl; + +ExternalCommitHelper::ExternalCommitHelper(RealmCoordinator& parent) +: m_parent(parent) +, m_history(realm::make_client_history(parent.get_path(), parent.get_encryption_key().data())) +, m_sg(*m_history, parent.is_in_memory() ? SharedGroup::durability_MemOnly : SharedGroup::durability_Full, + parent.get_encryption_key().data()) +, m_thread(std::async(std::launch::async, [=] { + while (m_sg.wait_for_change()) { + m_parent.on_change(); + } +})) +{ +} + +ExternalCommitHelper::~ExternalCommitHelper() +{ + m_sg.wait_for_change_release(); + m_thread.wait(); // Wait for the thread to exit +} diff --git a/src/impl/generic/external_commit_helper.hpp b/src/impl/generic/external_commit_helper.hpp new file mode 100644 index 00000000..3249430e --- /dev/null +++ b/src/impl/generic/external_commit_helper.hpp @@ -0,0 +1,54 @@ +//////////////////////////////////////////////////////////////////////////// +// +// Copyright 2015 Realm Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +//////////////////////////////////////////////////////////////////////////// + +#ifndef REALM_EXTERNAL_COMMIT_HELPER_HPP +#define REALM_EXTERNAL_COMMIT_HELPER_HPP + +#include + +#include + +namespace realm { +class ClientHistory; + +namespace _impl { +class RealmCoordinator; + +class ExternalCommitHelper { +public: + ExternalCommitHelper(RealmCoordinator& parent); + ~ExternalCommitHelper(); + + // A no-op in this version, but needed for the Apple version + void notify_others() { } + +private: + RealmCoordinator& m_parent; + + // The listener thread + std::future m_thread; + + // A shared group used to listen for changes + std::unique_ptr m_history; + SharedGroup m_sg; +}; + +} // namespace _impl +} // namespace realm + +#endif /* REALM_EXTERNAL_COMMIT_HELPER_HPP */ diff --git a/src/impl/realm_coordinator.hpp b/src/impl/realm_coordinator.hpp index ac889412..46cfcec3 100644 --- a/src/impl/realm_coordinator.hpp +++ b/src/impl/realm_coordinator.hpp @@ -45,6 +45,8 @@ public: const Schema* get_schema() const noexcept; uint64_t get_schema_version() const noexcept { return m_config.schema_version; } const std::string& get_path() const noexcept { return m_config.path; } + const std::vector& get_encryption_key() const noexcept { return m_config.encryption_key; } + bool is_in_memory() const noexcept { return m_config.in_memory; } // Asyncronously call notify() on every Realm instance for this coordinator's // path, including those in other processes From 112c778d8e8af33645e58c57851f028129b392da Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Thu, 3 Dec 2015 14:16:48 -0800 Subject: [PATCH 12/17] Extract the non-Apple specific parts of CachedRealm to a base class --- src/CMakeLists.txt | 1 + src/impl/apple/cached_realm.cpp | 14 +++---- src/impl/apple/cached_realm.hpp | 28 ++------------ src/impl/cached_realm_base.hpp | 68 +++++++++++++++++++++++++++++++++ 4 files changed, 79 insertions(+), 32 deletions(-) create mode 100644 src/impl/cached_realm_base.hpp diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index ffb715a2..716b8f7d 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -21,6 +21,7 @@ set(HEADERS results.hpp schema.hpp shared_realm.hpp + impl/cached_realm_base.hpp impl/transact_log_handler.hpp parser/parser.hpp parser/query_builder.hpp) diff --git a/src/impl/apple/cached_realm.cpp b/src/impl/apple/cached_realm.cpp index 03705d52..ca475e46 100644 --- a/src/impl/apple/cached_realm.cpp +++ b/src/impl/apple/cached_realm.cpp @@ -24,9 +24,7 @@ using namespace realm; using namespace realm::_impl; CachedRealm::CachedRealm(const std::shared_ptr& realm, bool cache) -: m_realm(realm) -, m_realm_key(realm.get()) -, m_cache(cache) +: CachedRealmBase(realm, cache) { struct RefCountedWeakPointer { std::weak_ptr realm; @@ -58,15 +56,17 @@ CachedRealm::CachedRealm(const std::shared_ptr& realm, bool cache) } CachedRealm::CachedRealm(CachedRealm&& rgt) +: CachedRealmBase(std::move(rgt)) +, m_runloop(rgt.m_runloop) +, m_signal(rgt.m_signal) { - *this = std::move(rgt); + rgt.m_runloop = nullptr; + rgt.m_signal = nullptr; } CachedRealm& CachedRealm::operator=(CachedRealm&& rgt) { - m_realm = std::move(rgt.m_realm); - m_thread_id = rgt.m_thread_id; - m_cache = rgt.m_cache; + CachedRealmBase::operator=(std::move(rgt)); m_runloop = rgt.m_runloop; m_signal = rgt.m_signal; rgt.m_runloop = nullptr; diff --git a/src/impl/apple/cached_realm.hpp b/src/impl/apple/cached_realm.hpp index 6b114dc6..15be9488 100644 --- a/src/impl/apple/cached_realm.hpp +++ b/src/impl/apple/cached_realm.hpp @@ -19,21 +19,16 @@ #ifndef REALM_CACHED_REALM_HPP #define REALM_CACHED_REALM_HPP +#include "../cached_realm_base.hpp" + #include -#include -#include namespace realm { class Realm; namespace _impl { -// CachedRealm stores a weak reference to a Realm instance, along with all of -// the information about a Realm that needs to be accessed from other threads. -// This is needed to avoid forming strong references to the Realm instances on -// other threads, which can produce deadlocks when the last strong reference to -// a Realm instance is released from within a function holding the cache lock. -class CachedRealm { +class CachedRealm : public CachedRealmBase { public: CachedRealm(const std::shared_ptr& realm, bool cache); ~CachedRealm(); @@ -44,27 +39,10 @@ public: CachedRealm(const CachedRealm&) = delete; CachedRealm& operator=(const CachedRealm&) = delete; - // Get a strong reference to the cached realm - std::shared_ptr realm() const { return m_realm.lock(); } - - // Does this CachedRealm store a Realm instance that should be used on the current thread? - bool is_cached_for_current_thread() const { return m_cache && m_thread_id == std::this_thread::get_id(); } - - // Has the Realm instance been destroyed? - bool expired() const { return m_realm.expired(); } - // Asyncronously call notify() on the Realm on the appropriate thread void notify(); - // Is this a CachedRealm for the given Realm instance? - bool is_for_realm(Realm* realm) const { return realm == m_realm_key; } - private: - std::weak_ptr m_realm; - std::thread::id m_thread_id = std::this_thread::get_id(); - void* m_realm_key; - bool m_cache = false; - CFRunLoopRef m_runloop; CFRunLoopSourceRef m_signal; }; diff --git a/src/impl/cached_realm_base.hpp b/src/impl/cached_realm_base.hpp new file mode 100644 index 00000000..57feceb5 --- /dev/null +++ b/src/impl/cached_realm_base.hpp @@ -0,0 +1,68 @@ +//////////////////////////////////////////////////////////////////////////// +// +// Copyright 2015 Realm Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +//////////////////////////////////////////////////////////////////////////// + +#ifndef REALM_CACHED_REALM_BASE_HPP +#define REALM_CACHED_REALM_BASE_HPP + +#include +#include + +namespace realm { +class Realm; + +namespace _impl { + +// CachedRealm stores a weak reference to a Realm instance, along with all of +// the information about a Realm that needs to be accessed from other threads. +// This is needed to avoid forming strong references to the Realm instances on +// other threads, which can produce deadlocks when the last strong reference to +// a Realm instance is released from within a function holding the cache lock. +class CachedRealmBase { +public: + CachedRealmBase(const std::shared_ptr& realm, bool cache); + + // Get a strong reference to the cached realm + std::shared_ptr realm() const { return m_realm.lock(); } + + // Does this CachedRealmBase store a Realm instance that should be used on the current thread? + bool is_cached_for_current_thread() const { return m_cache && m_thread_id == std::this_thread::get_id(); } + + // Has the Realm instance been destroyed? + bool expired() const { return m_realm.expired(); } + + // Is this a CachedRealmBase for the given Realm instance? + bool is_for_realm(Realm* realm) const { return realm == m_realm_key; } + +private: + std::weak_ptr m_realm; + std::thread::id m_thread_id = std::this_thread::get_id(); + void* m_realm_key; + bool m_cache = false; +}; + +inline CachedRealmBase::CachedRealmBase(const std::shared_ptr& realm, bool cache) +: m_realm(realm) +, m_realm_key(realm.get()) +, m_cache(cache) +{ +} + +} // namespace _impl +} // namespace realm + +#endif // REALM_CACHED_REALM_BASE_HPP From 4eb49ce6dc2d4a681509ccd8ce43f252a3aa05e8 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Thu, 3 Dec 2015 14:19:24 -0800 Subject: [PATCH 13/17] Add a not-very-useful generic CachedRealm implementation --- src/CMakeLists.txt | 7 ++++++ src/impl/generic/cached_realm.hpp | 40 +++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) create mode 100644 src/impl/generic/cached_realm.hpp diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 716b8f7d..6775bab7 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -35,6 +35,13 @@ if(APPLE) impl/apple/cached_realm.hpp impl/apple/external_commit_helper.hpp) find_library(CF_LIBRARY CoreFoundation) +else() + include_directories(impl/generic) + list(APPEND SOURCES + impl/generic/external_commit_helper.cpp) + list(APPEND HEADERS + impl/generic/cached_realm.hpp + impl/generic/external_commit_helper.hpp) endif() add_library(realm-object-store SHARED ${SOURCES} ${HEADERS}) diff --git a/src/impl/generic/cached_realm.hpp b/src/impl/generic/cached_realm.hpp new file mode 100644 index 00000000..8ceadf0a --- /dev/null +++ b/src/impl/generic/cached_realm.hpp @@ -0,0 +1,40 @@ +//////////////////////////////////////////////////////////////////////////// +// +// Copyright 2015 Realm Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +//////////////////////////////////////////////////////////////////////////// + +#ifndef REALM_CACHED_REALM_HPP +#define REALM_CACHED_REALM_HPP + +#include "../cached_realm_base.hpp" + +namespace realm { +class Realm; + +namespace _impl { + +class CachedRealm : public CachedRealmBase { +public: + using CachedRealmBase::CachedRealmBase; + + // Do nothing, as this can't be implemented portably + void notify() { } +}; + +} // namespace _impl +} // namespace realm + +#endif // REALM_CACHED_REALM_HPP From e557babaad421e48a6c36acec1e75fb8c93f383f Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Fri, 4 Dec 2015 12:07:42 -0800 Subject: [PATCH 14/17] Fix the generic implementation of ExternalCommitHelper --- src/impl/generic/external_commit_helper.cpp | 3 +++ src/impl/generic/external_commit_helper.hpp | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/impl/generic/external_commit_helper.cpp b/src/impl/generic/external_commit_helper.cpp index 66417642..e50e884a 100644 --- a/src/impl/generic/external_commit_helper.cpp +++ b/src/impl/generic/external_commit_helper.cpp @@ -32,7 +32,10 @@ ExternalCommitHelper::ExternalCommitHelper(RealmCoordinator& parent) , m_sg(*m_history, parent.is_in_memory() ? SharedGroup::durability_MemOnly : SharedGroup::durability_Full, parent.get_encryption_key().data()) , m_thread(std::async(std::launch::async, [=] { + m_sg.begin_read(); while (m_sg.wait_for_change()) { + m_sg.end_read(); + m_sg.begin_read(); m_parent.on_change(); } })) diff --git a/src/impl/generic/external_commit_helper.hpp b/src/impl/generic/external_commit_helper.hpp index 3249430e..d056566e 100644 --- a/src/impl/generic/external_commit_helper.hpp +++ b/src/impl/generic/external_commit_helper.hpp @@ -40,12 +40,12 @@ public: private: RealmCoordinator& m_parent; - // The listener thread - std::future m_thread; - // A shared group used to listen for changes std::unique_ptr m_history; SharedGroup m_sg; + + // The listener thread + std::future m_thread; }; } // namespace _impl From ad5db727673e4dd8cb6da2741fca0afd81cd51b7 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Fri, 11 Dec 2015 14:31:07 -0800 Subject: [PATCH 15/17] Destroy all notifiers before closing realms in clear_cache() --- src/impl/realm_coordinator.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/impl/realm_coordinator.cpp b/src/impl/realm_coordinator.cpp index cfcdd6ad..ca61c8e4 100644 --- a/src/impl/realm_coordinator.cpp +++ b/src/impl/realm_coordinator.cpp @@ -140,18 +140,19 @@ void RealmCoordinator::unregister_realm(Realm* realm) void RealmCoordinator::clear_cache() { - std::vector realms_to_close; - + std::vector realms_to_close; { std::lock_guard lock(s_coordinator_mutex); - // Gather a list of all of the realms which will be removed for (auto& weak_coordinator : s_coordinators_per_path) { auto coordinator = weak_coordinator.second.lock(); if (!coordinator) { continue; } + coordinator->m_notifier = nullptr; + + // Gather a list of all of the realms which will be removed for (auto& cached_realm : coordinator->m_cached_realms) { if (auto realm = cached_realm.realm()) { realms_to_close.push_back(realm); @@ -164,8 +165,10 @@ void RealmCoordinator::clear_cache() // Close all of the previously cached Realms. This can't be done while // s_coordinator_mutex is held as it may try to re-lock it. - for (auto& realm : realms_to_close) { - realm->close(); + for (auto& weak_realm : realms_to_close) { + if (auto realm = weak_realm.lock()) { + realm->close(); + } } } From a3dab7e4b195655aa852a9483aee3e5092467110 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Mon, 25 Jan 2016 10:25:28 -0800 Subject: [PATCH 16/17] Add wrappers for platform-specific headers and normalize include paths Building the objectstore code now only requires adding the root `src` directory to the include paths. --- src/CMakeLists.txt | 6 ++--- src/impl/apple/cached_realm.cpp | 2 +- src/impl/apple/cached_realm.hpp | 7 +---- src/impl/apple/external_commit_helper.cpp | 4 +-- src/impl/apple/external_commit_helper.hpp | 5 ---- src/impl/cached_realm.hpp | 30 +++++++++++++++++++++ src/impl/external_commit_helper.hpp | 30 +++++++++++++++++++++ src/impl/generic/cached_realm.hpp | 6 +---- src/impl/generic/external_commit_helper.cpp | 4 +-- src/impl/generic/external_commit_helper.hpp | 4 --- src/impl/realm_coordinator.cpp | 6 ++--- src/impl/transact_log_handler.cpp | 4 +-- src/shared_realm.cpp | 6 ++--- 13 files changed, 77 insertions(+), 37 deletions(-) create mode 100644 src/impl/cached_realm.hpp create mode 100644 src/impl/external_commit_helper.hpp diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 6775bab7..76233013 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -1,5 +1,3 @@ -include_directories(impl) - set(SOURCES index_set.cpp list.cpp @@ -21,13 +19,14 @@ set(HEADERS results.hpp schema.hpp shared_realm.hpp + impl/cached_realm.hpp impl/cached_realm_base.hpp + impl/external_commit_helper.hpp impl/transact_log_handler.hpp parser/parser.hpp parser/query_builder.hpp) if(APPLE) - include_directories(impl/apple) list(APPEND SOURCES impl/apple/cached_realm.cpp impl/apple/external_commit_helper.cpp) @@ -36,7 +35,6 @@ if(APPLE) impl/apple/external_commit_helper.hpp) find_library(CF_LIBRARY CoreFoundation) else() - include_directories(impl/generic) list(APPEND SOURCES impl/generic/external_commit_helper.cpp) list(APPEND HEADERS diff --git a/src/impl/apple/cached_realm.cpp b/src/impl/apple/cached_realm.cpp index ca475e46..1468a8fb 100644 --- a/src/impl/apple/cached_realm.cpp +++ b/src/impl/apple/cached_realm.cpp @@ -16,7 +16,7 @@ // //////////////////////////////////////////////////////////////////////////// -#include "cached_realm.hpp" +#include "impl/cached_realm.hpp" #include "shared_realm.hpp" diff --git a/src/impl/apple/cached_realm.hpp b/src/impl/apple/cached_realm.hpp index 15be9488..5acf874e 100644 --- a/src/impl/apple/cached_realm.hpp +++ b/src/impl/apple/cached_realm.hpp @@ -16,10 +16,7 @@ // //////////////////////////////////////////////////////////////////////////// -#ifndef REALM_CACHED_REALM_HPP -#define REALM_CACHED_REALM_HPP - -#include "../cached_realm_base.hpp" +#include "impl/cached_realm_base.hpp" #include @@ -49,5 +46,3 @@ private: } // namespace _impl } // namespace realm - -#endif // REALM_CACHED_REALM_HPP diff --git a/src/impl/apple/external_commit_helper.cpp b/src/impl/apple/external_commit_helper.cpp index f66d4e37..db04a1de 100644 --- a/src/impl/apple/external_commit_helper.cpp +++ b/src/impl/apple/external_commit_helper.cpp @@ -16,9 +16,9 @@ // //////////////////////////////////////////////////////////////////////////// -#include "external_commit_helper.hpp" +#include "impl/external_commit_helper.hpp" -#include "realm_coordinator.hpp" +#include "impl/realm_coordinator.hpp" #include #include diff --git a/src/impl/apple/external_commit_helper.hpp b/src/impl/apple/external_commit_helper.hpp index c6d795a0..a39876ce 100644 --- a/src/impl/apple/external_commit_helper.hpp +++ b/src/impl/apple/external_commit_helper.hpp @@ -16,9 +16,6 @@ // //////////////////////////////////////////////////////////////////////////// -#ifndef REALM_EXTERNAL_COMMIT_HELPER_HPP -#define REALM_EXTERNAL_COMMIT_HELPER_HPP - #include namespace realm { @@ -77,5 +74,3 @@ private: } // namespace _impl } // namespace realm - -#endif /* REALM_EXTERNAL_COMMIT_HELPER_HPP */ diff --git a/src/impl/cached_realm.hpp b/src/impl/cached_realm.hpp new file mode 100644 index 00000000..3818ca62 --- /dev/null +++ b/src/impl/cached_realm.hpp @@ -0,0 +1,30 @@ +//////////////////////////////////////////////////////////////////////////// +// +// Copyright 2016 Realm Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +//////////////////////////////////////////////////////////////////////////// + +#ifndef REALM_CACHED_REALM_HPP +#define REALM_CACHED_REALM_HPP + +#include + +#if REALM_PLATFORM_APPLE +#include "impl/apple/cached_realm.hpp" +#else +#include "impl/generic/cached_realm.hpp" +#endif + +#endif // REALM_CACHED_REALM_HPP diff --git a/src/impl/external_commit_helper.hpp b/src/impl/external_commit_helper.hpp new file mode 100644 index 00000000..c467a4b9 --- /dev/null +++ b/src/impl/external_commit_helper.hpp @@ -0,0 +1,30 @@ +//////////////////////////////////////////////////////////////////////////// +// +// Copyright 2016 Realm Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +//////////////////////////////////////////////////////////////////////////// + +#ifndef REALM_EXTERNAL_COMMIT_HELPER_HPP +#define REALM_EXTERNAL_COMMIT_HELPER_HPP + +#include + +#if REALM_PLATFORM_APPLE +#include "impl/apple/external_commit_helper.hpp" +#else +#include "impl/generic/external_commit_helper.hpp" +#endif + +#endif // REALM_EXTERNAL_COMMIT_HELPER_HPP diff --git a/src/impl/generic/cached_realm.hpp b/src/impl/generic/cached_realm.hpp index 8ceadf0a..23a7be11 100644 --- a/src/impl/generic/cached_realm.hpp +++ b/src/impl/generic/cached_realm.hpp @@ -16,10 +16,7 @@ // //////////////////////////////////////////////////////////////////////////// -#ifndef REALM_CACHED_REALM_HPP -#define REALM_CACHED_REALM_HPP - -#include "../cached_realm_base.hpp" +#include "impl/cached_realm_base.hpp" namespace realm { class Realm; @@ -37,4 +34,3 @@ public: } // namespace _impl } // namespace realm -#endif // REALM_CACHED_REALM_HPP diff --git a/src/impl/generic/external_commit_helper.cpp b/src/impl/generic/external_commit_helper.cpp index e50e884a..5071304b 100644 --- a/src/impl/generic/external_commit_helper.cpp +++ b/src/impl/generic/external_commit_helper.cpp @@ -16,9 +16,9 @@ // //////////////////////////////////////////////////////////////////////////// -#include "external_commit_helper.hpp" +#include "impl/external_commit_helper.hpp" -#include "realm_coordinator.hpp" +#include "impl/realm_coordinator.hpp" #include #include diff --git a/src/impl/generic/external_commit_helper.hpp b/src/impl/generic/external_commit_helper.hpp index d056566e..cc7fe5eb 100644 --- a/src/impl/generic/external_commit_helper.hpp +++ b/src/impl/generic/external_commit_helper.hpp @@ -16,9 +16,6 @@ // //////////////////////////////////////////////////////////////////////////// -#ifndef REALM_EXTERNAL_COMMIT_HELPER_HPP -#define REALM_EXTERNAL_COMMIT_HELPER_HPP - #include #include @@ -51,4 +48,3 @@ private: } // namespace _impl } // namespace realm -#endif /* REALM_EXTERNAL_COMMIT_HELPER_HPP */ diff --git a/src/impl/realm_coordinator.cpp b/src/impl/realm_coordinator.cpp index ca61c8e4..e305b420 100644 --- a/src/impl/realm_coordinator.cpp +++ b/src/impl/realm_coordinator.cpp @@ -16,10 +16,10 @@ // //////////////////////////////////////////////////////////////////////////// -#include "realm_coordinator.hpp" +#include "impl/realm_coordinator.hpp" -#include "cached_realm.hpp" -#include "external_commit_helper.hpp" +#include "impl/cached_realm.hpp" +#include "impl/external_commit_helper.hpp" #include "object_store.hpp" #include diff --git a/src/impl/transact_log_handler.cpp b/src/impl/transact_log_handler.cpp index ed1630ca..72b2a200 100644 --- a/src/impl/transact_log_handler.cpp +++ b/src/impl/transact_log_handler.cpp @@ -16,9 +16,9 @@ // //////////////////////////////////////////////////////////////////////////// -#include "transact_log_handler.hpp" +#include "impl/transact_log_handler.hpp" -#include "../binding_context.hpp" +#include "binding_context.hpp" #include #include diff --git a/src/shared_realm.cpp b/src/shared_realm.cpp index 45960d8b..81c70117 100644 --- a/src/shared_realm.cpp +++ b/src/shared_realm.cpp @@ -19,11 +19,11 @@ #include "shared_realm.hpp" #include "binding_context.hpp" -#include "external_commit_helper.hpp" +#include "impl/external_commit_helper.hpp" +#include "impl/realm_coordinator.hpp" +#include "impl/transact_log_handler.hpp" #include "object_store.hpp" -#include "realm_coordinator.hpp" #include "schema.hpp" -#include "transact_log_handler.hpp" #include #include From 638b4ec35e11137c361dd8ed5f10f50541be69a3 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Mon, 1 Feb 2016 11:42:33 -0800 Subject: [PATCH 17/17] Actually update the coordinator's copy of the schema --- src/impl/realm_coordinator.cpp | 10 +++++++++- src/impl/realm_coordinator.hpp | 5 +++++ src/shared_realm.cpp | 3 +++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/impl/realm_coordinator.cpp b/src/impl/realm_coordinator.cpp index e305b420..d59cad08 100644 --- a/src/impl/realm_coordinator.cpp +++ b/src/impl/realm_coordinator.cpp @@ -21,6 +21,7 @@ #include "impl/cached_realm.hpp" #include "impl/external_commit_helper.hpp" #include "object_store.hpp" +#include "schema.hpp" #include @@ -96,7 +97,7 @@ std::shared_ptr RealmCoordinator::get_realm(Realm::Config config) } } - auto realm = std::make_shared(config); + auto realm = std::make_shared(std::move(config)); realm->init(shared_from_this()); m_cached_realms.emplace_back(realm, m_config.cache); return realm; @@ -107,6 +108,13 @@ const Schema* RealmCoordinator::get_schema() const noexcept return m_cached_realms.empty() ? nullptr : m_config.schema.get(); } +void RealmCoordinator::update_schema(Schema const& schema) +{ + // FIXME: this should probably be doing some sort of validation and + // notifying all Realm instances of the new schema in some way + m_config.schema = std::make_unique(schema); +} + RealmCoordinator::RealmCoordinator() = default; RealmCoordinator::~RealmCoordinator() diff --git a/src/impl/realm_coordinator.hpp b/src/impl/realm_coordinator.hpp index 46cfcec3..ee1d748b 100644 --- a/src/impl/realm_coordinator.hpp +++ b/src/impl/realm_coordinator.hpp @@ -24,6 +24,8 @@ #include namespace realm { +class Schema; + namespace _impl { class CachedRealm; class ExternalCommitHelper; @@ -68,6 +70,9 @@ public: // Called by m_notifier when there's a new commit to send notifications for void on_change(); + // Update the schema in the cached config + void update_schema(Schema const& new_schema); + private: Realm::Config m_config; diff --git a/src/shared_realm.cpp b/src/shared_realm.cpp index 81c70117..5c5b818a 100644 --- a/src/shared_realm.cpp +++ b/src/shared_realm.cpp @@ -186,6 +186,7 @@ void Realm::update_schema(std::unique_ptr schema, uint64_t version) ObjectStore::verify_schema(*m_config.schema, *schema, m_config.read_only); m_config.schema = std::move(schema); m_config.schema_version = version; + m_coordinator->update_schema(*m_config.schema); return false; }; @@ -243,6 +244,8 @@ void Realm::update_schema(std::unique_ptr schema, uint64_t version) m_config.schema_version = old_config.schema_version; throw; } + + m_coordinator->update_schema(*m_config.schema); } static void check_read_write(Realm *realm)