Only perform launch actions if the metadata store initiated the session

Author: tgoyne

CHANGELOG.md

@@ -2,7 +2,7 @@
 
 ### Enhancements
 * <New feature description> (PR [#????](https://github.com/realm/realm-core/pull/????))
-* None.
+* "Next launch" metadata file actions are now performed in a multi-process safe manner ([#7576](https://github.com/realm/realm-core/pull/7576)).
 
 ### Fixed
 * <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)

src/realm/db.cpp

@@ -2860,20 +2860,27 @@ DBRef DB::create(BinaryData buffer, bool take_ownership) NO_THREAD_SAFETY_ANALYS
     return retval;
 }
 
-void DB::claim_sync_agent()
+bool DB::try_claim_sync_agent()
 {
     REALM_ASSERT(is_attached());
-    std::unique_lock<InterprocessMutex> lock(m_controlmutex);
+    std::lock_guard lock(m_controlmutex);
     if (m_info->sync_agent_present)
-        throw MultipleSyncAgents{};
+        return false;
     m_info->sync_agent_present = 1; // Set to true
     m_is_sync_agent = true;
+    return true;
+}
+
+void DB::claim_sync_agent()
+{
+    if (!try_claim_sync_agent())
+        throw MultipleSyncAgents{};
 }
 
 void DB::release_sync_agent()
 {
     REALM_ASSERT(is_attached());
-    std::unique_lock<InterprocessMutex> lock(m_controlmutex);
+    std::lock_guard lock(m_controlmutex);
     if (!m_is_sync_agent)
         return;
     REALM_ASSERT(m_info->sync_agent_present);

src/realm/db.hpp

@@ -432,6 +432,7 @@ class DB : public std::enable_shared_from_this<DB> {
     /// Mark this DB as the sync agent for the file.
     /// \throw MultipleSyncAgents if another DB is already the sync agent.
     void claim_sync_agent();
+    bool try_claim_sync_agent();
     void release_sync_agent();
 
     /// Returns true if there are threads waiting to acquire the write lock, false otherwise.

src/realm/object-store/impl/realm_coordinator.hpp

@@ -228,6 +228,11 @@ class RealmCoordinator : public std::enable_shared_from_this<RealmCoordinator>,
         return m_audit_context.get();
     }
 
+    bool try_claim_sync_agent()
+    {
+        return m_db->try_claim_sync_agent();
+    }
+
 private:
     friend Realm::Internal;
     Realm::Config m_config;

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

@@ -297,10 +297,6 @@ std::shared_ptr<Realm> open_realm(RealmConfig& config, const app::AppConfig& app
         }
     };
 
-    // This logic is all a giant race condition once we have multi-process sync.
-    // Wrapping it all (including the keychain accesses) in DB::call_with_lock()
-    // might suffice.
-
     // First try to open the Realm with a key already stored in the keychain.
     // This works for both the case where everything is sensible and valid and
     // when we have a key but no metadata Realm.
@@ -381,12 +377,13 @@ struct PersistedSyncMetadataManager : public app::MetadataStore {
         m_user_identity_schema.read(*realm);
         m_current_user_schema.read(*realm);
 
-        realm->begin_transaction();
-        perform_file_actions(*realm, file_manager);
-        remove_dead_users(*realm, file_manager);
-        realm->commit_transaction();
-
         m_coordinator = _impl::RealmCoordinator::get_existing_coordinator(config.path);
+        if (m_coordinator->try_claim_sync_agent()) {
+            realm->begin_transaction();
+            perform_file_actions(*realm, file_manager);
+            remove_dead_users(*realm, file_manager);
+            realm->commit_transaction();
+        }
     }
 
     std::shared_ptr<Realm> get_realm() const

test/object-store/sync/metadata.cpp

@@ -613,6 +613,83 @@ TEST_CASE("app metadata: persisted", "[sync][metadata]") {
     }
 }
 
+TEST_CASE("app metadata: multiple stores", "[sync][metadata]") {
+    test_util::TestDirGuard test_dir(base_path);
+
+    AppConfig config;
+    config.app_id = "app id";
+    config.metadata_mode = AppConfig::MetadataMode::NoEncryption;
+    config.base_file_path = base_path;
+    SyncFileManager file_manager(config);
+
+    auto store_1 = create_metadata_store(config, file_manager);
+    auto store_2 = create_metadata_store(config, file_manager);
+    REQUIRE(store_1 != store_2);
+
+    SECTION("create user in one store then read from the other") {
+        store_1->create_user(user_id, refresh_token, access_token, device_id);
+        auto user = store_2->get_user(user_id);
+        REQUIRE(user);
+    }
+
+    SECTION("update existing user in one store then read from the other") {
+        store_1->create_user(user_id, refresh_token, access_token, device_id);
+        auto user_1 = store_1->get_user(user_id);
+
+        std::vector<UserIdentity> identities{{"identity", "provider"}};
+        store_2->update_user(user_id, [&](UserData& data) {
+            data.identities = identities;
+        });
+
+        auto user_2 = store_1->get_user(user_id);
+        CHECK(user_1->identities.empty());
+        CHECK(user_2->identities == identities);
+    }
+
+    SECTION("non-conflicting writes from multiple stores") {
+        store_1->create_user(user_id, refresh_token, access_token, device_id);
+        auto user_1 = store_1->get_user(user_id);
+        auto user_2 = store_2->get_user(user_id);
+
+        UserProfile profile = bson::BsonDocument{{"name", "user's name"}, {"email", "user's email"}};
+        std::vector<UserIdentity> identities{{"identity", "provider"}};
+        store_1->update_user(user_id, [&](UserData& data) {
+            data.identities = identities;
+        });
+        store_2->update_user(user_id, [&](UserData& data) {
+            data.profile = profile;
+        });
+
+        // The second write should not have discarded the change made by the first store
+        user_1 = store_1->get_user(user_id);
+        user_2 = store_2->get_user(user_id);
+        REQUIRE(user_1->identities == user_2->identities);
+        REQUIRE(user_1->identities == identities);
+        REQUIRE(user_1->profile.data() == user_2->profile.data());
+        REQUIRE(user_1->profile.data() == profile.data());
+    }
+
+    SECTION("file actions are not performed on open if there is already a live store") {
+        auto path = util::make_temp_file("file_to_delete");
+        store_1->create_user(user_id, refresh_token, access_token, device_id);
+        store_1->add_realm_path(user_id, path);
+        store_1->log_out(user_id, SyncUser::State::Removed);
+
+        create_metadata_store(config, file_manager);
+        REQUIRE(File::exists(path));
+
+        store_1.reset();
+
+        create_metadata_store(config, file_manager);
+        REQUIRE(File::exists(path));
+
+        store_2.reset();
+
+        create_metadata_store(config, file_manager);
+        REQUIRE_FALSE(File::exists(path));
+    }
+}
+
 #if REALM_ENABLE_ENCRYPTION
 TEST_CASE("app metadata: encryption", "[sync][metadata]") {
     test_util::TestDirGuard test_dir(base_path);
@@ -739,7 +816,7 @@ TEST_CASE("app metadata: encryption", "[sync][metadata]") {
 #endif // REALM_PLATFORM_APPLE
 }
 
-#endif
+#endif // REALM_ENABLE_ENCRYPTION
 
 #ifndef SWIFT_PACKAGE // The SPM build currently doesn't copy resource files
 TEST_CASE("sync metadata: can open old metadata realms", "[sync][metadata]") {