RCORE-2224 clean up an unused read lock on subscription sets (#7949)

* clean up an unused read lock on subscription sets

* fix type

CHANGELOG.md

@@ -3,6 +3,7 @@
 ### Enhancements
 * Improve sync bootstrap performance by reducing the number of table selections in the replication logs for embedded objects. ([#7945](https://github.com/realm/realm-core/issues/7945))
 * <New feature description> (PR [#????](https://github.com/realm/realm-core/pull/????))
+* Released a read lock which was pinned for the duration of a mutable subscription even after commit. This frees resources earlier, and may improve performance of sync bootstraps where the starting state is large. ([#7946](https://github.com/realm/realm-core/issues/7946))
 * Client reset cycle detection now checks if the previous recovery attempt was made by the same core version, and if not attempts recovery again ([PR #7944](https://github.com/realm/realm-core/pull/7944)).
 
 ### Fixed

src/realm/sync/subscriptions.cpp

@@ -282,7 +282,7 @@ MutableSubscriptionSet::MutableSubscriptionSet(std::weak_ptr<SubscriptionStore>
 
 void MutableSubscriptionSet::check_is_mutable() const
 {
-    if (m_tr->get_transact_stage() != DB::transact_Writing) {
+    if (!m_tr || m_tr->get_transact_stage() != DB::transact_Writing) {
         throw WrongTransactionState("Not a write transaction");
     }
 }
@@ -547,7 +547,7 @@ int64_t SubscriptionStore::get_downloading_query_version(Transaction& tr) const
 
 SubscriptionSet MutableSubscriptionSet::commit()
 {
-    if (m_tr->get_transact_stage() != DB::transact_Writing) {
+    if (!m_tr || m_tr->get_transact_stage() != DB::transact_Writing) {
         throw LogicError(ErrorCodes::WrongTransactionState, "SubscriptionSet has already been committed");
     }
     auto mgr = get_flx_subscription_store(); // Throws
@@ -577,7 +577,12 @@ SubscriptionSet MutableSubscriptionSet::commit()
 
     mgr->report_progress(m_tr);
 
-    return mgr->get_refreshed(m_obj.get_key(), flx_version, m_tr->get_version_of_current_transaction());
+    DB::VersionID commit_version = m_tr->get_version_of_current_transaction();
+    // release the read lock so that this instance doesn't keep a version pinned
+    // for the remainder of its lifetime
+    m_tr.reset();
+
+    return mgr->get_refreshed(m_obj.get_key(), flx_version, commit_version);
 }
 
 std::string SubscriptionSet::to_ext_json() const

test/test_sync_subscriptions.cpp

@@ -1092,4 +1092,34 @@ TEST(Sync_MutableSubscriptionSetOperations)
     }
 }
 
+TEST(Sync_MutableSubscriptionReleasesReadLock)
+{
+    SHARED_GROUP_TEST_PATH(sub_store_path);
+    SubscriptionStoreFixture fixture(sub_store_path);
+    auto store = SubscriptionStore::create(fixture.db);
+
+    auto read_tr = fixture.db->start_read();
+    Query query_a(read_tr->get_table("class_a"));
+    query_a.greater_equal(fixture.bar_col, int64_t(1));
+
+    uint_fast64_t num_versions_before_subscription = fixture.db->get_number_of_versions();
+    auto mut_subs = store->get_latest().make_mutable_copy();
+    auto [it, inserted] = mut_subs.insert_or_assign("a sub", query_a);
+    CHECK(inserted);
+    CHECK_EQUAL(mut_subs.size(), 1);
+    mut_subs.commit();
+
+    // simulate sync writes fullfilling the subscription
+    size_t num_writes = 10;
+    while (--num_writes) {
+        auto wt = fixture.db->start_write();
+        wt->commit();
+        read_tr->advance_read(); // update our reader so that its old version can be cleaned up
+    }
+    uint_fast64_t num_versions_after_sync_writes = fixture.db->get_number_of_versions();
+
+    // check that the mut_subs in not keeping a transaction pinned
+    CHECK_EQUAL(num_versions_after_sync_writes, num_versions_before_subscription);
+}
+
 } // namespace realm::sync