Keep the metadata Realm open rather than reopening it on each use

Author: tgoyne

src/realm/object-store/sync/async_open_task.cpp

@@ -23,6 +23,7 @@
 #include <realm/object-store/impl/realm_coordinator.hpp>
 #include <realm/object-store/sync/sync_session.hpp>
 #include <realm/object-store/thread_safe_reference.hpp>
+#include <realm/object-store/util/scheduler.hpp>
 
 namespace realm {
 
@@ -126,7 +127,7 @@ void AsyncOpenTask::wait_for_bootstrap_or_complete(AsyncOpenCallback&& callback,
 
     SharedRealm shared_realm;
     try {
-        shared_realm = coordinator->get_realm(nullptr, m_db_first_open);
+        shared_realm = coordinator->get_realm(util::Scheduler::make_dummy(), m_db_first_open);
     }
     catch (...) {
         async_open_complete(std::move(callback), coordinator, exception_to_status());

src/realm/object-store/sync/impl/app_metadata.cpp

@@ -18,13 +18,15 @@
 
 #include <realm/object-store/sync/impl/app_metadata.hpp>
 
-#include <realm/object-store/sync/impl/sync_file.hpp>
+#include <realm/object-store/impl/realm_coordinator.hpp>
 #include <realm/object-store/object_schema.hpp>
 #include <realm/object-store/object_store.hpp>
 #include <realm/object-store/property.hpp>
 #include <realm/object-store/results.hpp>
 #include <realm/object-store/schema.hpp>
+#include <realm/object-store/sync/impl/sync_file.hpp>
 #include <realm/object-store/util/scheduler.hpp>
+
 #if REALM_PLATFORM_APPLE
 #include <realm/object-store/impl/apple/keychain_helper.hpp>
 #endif
@@ -336,13 +338,13 @@ std::shared_ptr<Realm> open_realm(RealmConfig& config, const app::AppConfig& app
 }
 
 struct PersistedSyncMetadataManager : public app::MetadataStore {
-    RealmConfig m_config;
+    std::shared_ptr<_impl::RealmCoordinator> m_coordinator;
     SyncUserSchema m_user_schema;
     FileActionSchema m_file_action_schema;
     UserIdentitySchema m_user_identity_schema;
     CurrentUserSchema m_current_user_schema;
 
-    PersistedSyncMetadataManager(std::string path, const app::AppConfig& app_config, SyncFileManager& file_manager)
+    PersistedSyncMetadataManager(const app::AppConfig& app_config, SyncFileManager& file_manager)
     {
         // Note that there are several deferred schema changes which don't
         // justify bumping the schema version by themself, but should be done
@@ -353,26 +355,27 @@ struct PersistedSyncMetadataManager : public app::MetadataStore {
         // - change most of the nullable columns to non-nullable
         constexpr uint64_t SCHEMA_VERSION = 7;
 
-        m_config.automatic_change_notifications = false;
-        m_config.path = std::move(path);
-        m_config.schema = Schema{
+        RealmConfig config;
+        config.automatic_change_notifications = false;
+        config.path = file_manager.metadata_path();
+        config.schema = Schema{
             UserIdentitySchema::object_schema(),
             SyncUserSchema::object_schema(),
             FileActionSchema::object_schema(),
             CurrentUserSchema::object_schema(),
         };
 
-        m_config.schema_version = SCHEMA_VERSION;
-        m_config.schema_mode = SchemaMode::Automatic;
-        m_config.scheduler = util::Scheduler::make_dummy();
-        m_config.automatically_handle_backlinks_in_migrations = true;
-        m_config.migration_function = [](std::shared_ptr<Realm> old_realm, std::shared_ptr<Realm> realm, Schema&) {
+        config.schema_version = SCHEMA_VERSION;
+        config.schema_mode = SchemaMode::Automatic;
+        config.scheduler = util::Scheduler::make_dummy();
+        config.automatically_handle_backlinks_in_migrations = true;
+        config.migration_function = [](std::shared_ptr<Realm> old_realm, std::shared_ptr<Realm> realm, Schema&) {
             if (old_realm->schema_version() < 7) {
                 migrate_to_v7(old_realm, realm);
             }
         };
 
-        auto realm = open_realm(m_config, app_config);
+        auto realm = open_realm(config, app_config);
         m_user_schema.read(*realm);
         m_file_action_schema.read(*realm);
         m_user_identity_schema.read(*realm);
@@ -382,11 +385,13 @@ struct PersistedSyncMetadataManager : public app::MetadataStore {
         perform_file_actions(*realm, file_manager);
         remove_dead_users(*realm, file_manager);
         realm->commit_transaction();
+
+        m_coordinator = _impl::RealmCoordinator::get_existing_coordinator(config.path);
     }
 
     std::shared_ptr<Realm> get_realm() const
     {
-        return Realm::get_shared_realm(m_config);
+        return m_coordinator->get_realm(util::Scheduler::make_dummy());
     }
 
     void remove_dead_users(Realm& realm, SyncFileManager& file_manager)
@@ -933,5 +938,5 @@ std::unique_ptr<app::MetadataStore> app::create_metadata_store(const AppConfig&
     if (config.metadata_mode == AppConfig::MetadataMode::InMemory) {
         return std::make_unique<InMemoryMetadataStorage>();
     }
-    return std::make_unique<PersistedSyncMetadataManager>(file_manager.metadata_path(), config, file_manager);
+    return std::make_unique<PersistedSyncMetadataManager>(config, file_manager);
 }

test/object-store/realm.cpp

@@ -1344,22 +1344,18 @@ TEST_CASE("Get Realm using Async Open", "[sync][pbs][async open]") {
         // User should be logged in at this point
         REQUIRE(config.sync_config->user->is_logged_in());
 
-        auto task = Realm::get_synchronized_realm(config);
-        auto pf = util::make_promise_future<std::exception_ptr>();
-        task->start([&pf](auto ref, auto error) mutable {
-            REQUIRE(!ref);
-            REQUIRE(error);
-            pf.promise.emplace_value(error);
-        });
+        auto status = async_open_realm(config);
+        REQUIRE_FALSE(status.is_ok());
 
-        auto result = pf.future.get_no_throw();
-        REQUIRE(result.is_ok());
-        REQUIRE(result.get_value());
-        std::lock_guard<std::mutex> lock(mutex);
-        REQUIRE(location_refresh_called);
-        if (failure != FailureMode::location_fails) {
-            REQUIRE(token_refresh_called);
+        {
+            std::lock_guard lock(mutex);
+            REQUIRE(location_refresh_called);
+            if (failure != FailureMode::location_fails) {
+                REQUIRE(token_refresh_called);
+            }
         }
+
+        app->sync_manager()->tear_down_for_testing();
     }
 
 #endif // REALM_APP_SERVICES

test/object-store/sync/flx_schema_migration.cpp

@@ -389,7 +389,7 @@ TEST_CASE("Schema version mismatch between client and server", "[sync][flx][flx
         realm->sync_session()->shutdown_and_wait();
         check_realm_schema(config.path, schema_v0, 0);
     }
-    _impl::RealmCoordinator::assert_no_open_realms();
+    REQUIRE_FALSE(_impl::RealmCoordinator::get_existing_coordinator(config.path));
 
     SECTION("Realm already on the latest schema version") {
         DBOptions options;
@@ -647,7 +647,7 @@ TEST_CASE("An interrupted schema migration can recover on the next session",
         wait_for_upload(*realm);
         check_realm_schema(config.path, schema_v0, 0);
     }
-    _impl::RealmCoordinator::assert_no_open_realms();
+    REQUIRE_FALSE(_impl::RealmCoordinator::get_existing_coordinator(config.path));
 
     const AppSession& app_session = harness.session().app_session();
     auto schema_v1 = get_schema_v1();
@@ -754,7 +754,7 @@ TEST_CASE("Client reset during schema migration", "[sync][flx][flx schema migrat
             std::any(AnyDict{{"_id", ObjectId::gen()}, {"queryable_int_field", static_cast<int64_t>(42)}}));
         realm->commit_transaction();
     }
-    _impl::RealmCoordinator::assert_no_open_realms();
+    REQUIRE_FALSE(_impl::RealmCoordinator::get_existing_coordinator(config.path));
 
     const AppSession& app_session = harness.session().app_session();
     auto schema_v1 = get_schema_v1();
@@ -842,7 +842,7 @@ TEST_CASE("Migrate to new schema version after migration to intermediate version
         realm->commit_transaction();
         realm->close();
     }
-    _impl::RealmCoordinator::assert_no_open_realms();
+    REQUIRE_FALSE(_impl::RealmCoordinator::get_existing_coordinator(config.path));
 
     const AppSession& app_session = harness.session().app_session();
     auto schema_v1 = get_schema_v1();
@@ -989,7 +989,7 @@ TEST_CASE("Client reset and schema migration", "[sync][flx][flx schema migration
         // Trigger a client reset.
         reset_utils::trigger_client_reset(harness.session().app_session(), *realm->sync_session());
     }
-    _impl::RealmCoordinator::assert_no_open_realms();
+    REQUIRE_FALSE(_impl::RealmCoordinator::get_existing_coordinator(config.path));
 
     const AppSession& app_session = harness.session().app_session();
     auto schema_v1 = get_schema_v1();

test/object-store/sync/flx_sync.cpp

@@ -1369,7 +1369,7 @@ TEST_CASE("flx: client reset", "[sync][flx][client reset][baas]") {
             realm->sync_session()->shutdown_and_wait();
             realm->close();
         }
-        _impl::RealmCoordinator::assert_no_open_realms();
+        REQUIRE_FALSE(_impl::RealmCoordinator::get_existing_coordinator(config_local.path));
         {
             // ensure that an additional schema change after the successful reset is also accepted by the server
             changed_schema[0].persisted_properties.push_back(
@@ -2311,7 +2311,8 @@ TEST_CASE("flx: interrupted bootstrap restarts/recovers on reconnect", "[sync][f
         realm->sync_session()->shutdown_and_wait();
     }
 
-    _impl::RealmCoordinator::assert_no_open_realms();
+    // Verify that the file was fully closed
+    REQUIRE(DB::call_with_lock(interrupted_realm_config.path, [](auto&) {}));
 
     {
         DBOptions options;
@@ -2840,7 +2841,7 @@ TEST_CASE("flx: bootstrap batching prevents orphan documents", "[sync][flx][boot
             realm->close();
         }
 
-        _impl::RealmCoordinator::assert_no_open_realms();
+        REQUIRE_FALSE(_impl::RealmCoordinator::get_existing_coordinator(interrupted_realm_config.path));
 
         // Open up the realm without the sync client attached and verify that the realm got interrupted in the state
         // we expected it to be in.
@@ -2933,7 +2934,7 @@ TEST_CASE("flx: bootstrap batching prevents orphan documents", "[sync][flx][boot
             realm->close();
         }
 
-        _impl::RealmCoordinator::assert_no_open_realms();
+        REQUIRE_FALSE(_impl::RealmCoordinator::get_existing_coordinator(interrupted_realm_config.path));
 
         // Open up the realm without the sync client attached and verify that the realm got interrupted in the state
         // we expected it to be in.
@@ -3002,7 +3003,7 @@ TEST_CASE("flx: bootstrap batching prevents orphan documents", "[sync][flx][boot
             realm->close();
         }
 
-        _impl::RealmCoordinator::assert_no_open_realms();
+        REQUIRE_FALSE(_impl::RealmCoordinator::get_existing_coordinator(interrupted_realm_config.path));
 
         // Open up the realm without the sync client attached and verify that the realm got interrupted in the state
         // we expected it to be in.
@@ -3079,7 +3080,7 @@ TEST_CASE("flx: bootstrap batching prevents orphan documents", "[sync][flx][boot
             realm->close();
         }
 
-        _impl::RealmCoordinator::assert_no_open_realms();
+        REQUIRE_FALSE(_impl::RealmCoordinator::get_existing_coordinator(interrupted_realm_config.path));
 
         // Open up the realm without the sync client attached and verify that the realm got interrupted in the state
         // we expected it to be in.
@@ -4583,7 +4584,7 @@ TEST_CASE("flx: open realm + register subscription callback while bootstrapping"
                 realm->close();
             }
 
-            _impl::RealmCoordinator::assert_no_open_realms();
+            REQUIRE_FALSE(_impl::RealmCoordinator::get_existing_coordinator(config.path));
 
             // Client 2 uploads data matching Client 1's subscription at version 1
             harness.load_initial_data([&](SharedRealm realm) {
@@ -4742,7 +4743,7 @@ TEST_CASE("flx sync: resend pending subscriptions when reconnecting", "[sync][fl
         realm->close();
     }
 
-    _impl::RealmCoordinator::assert_no_open_realms();
+    REQUIRE_FALSE(_impl::RealmCoordinator::get_existing_coordinator(interrupted_realm_config.path));
 
     // Check at least one subscription set needs to be resent.
     {