From 370bac4c7224993270544cbc4466498624292574 Mon Sep 17 00:00:00 2001
From: Thomas Goyne <tg@realm.io>
Date: Thu, 27 Apr 2023 09:30:31 -0700
Subject: [PATCH] Fix a crash when beginning an async write triggers
 notifications which close the Realm

Beginning a write transaction can result in the Realm being closed as it can
trigger notifications, and the async write loop was missing a check for this.
---
 CHANGELOG.md                            |  1 +
 src/realm/object-store/shared_realm.cpp |  6 +++
 test/object-store/realm.cpp             | 57 +++++++++++++++++++------
 3 files changed, 51 insertions(+), 13 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3bf739e497c..41d439675cb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,7 @@
 * <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
 * Performing a query like "{1, 2, 3, ...} IN list" where the array is longer than 8 and all elements are smaller than some values in list, the program would crash ([#1183](https://github.com/realm/realm-kotlin/issues/1183), v12.5.0)
 * Performing a large number of queries without ever performing a write resulted in steadily increasing memory usage, some of which was never fully freed due to an unbounded cache ([Swift #7978](https://github.com/realm/realm-swift/issues/7978), since v12.0.0)
+* Fix a null pointer dereference if beginning an async write transaction refreshed the Realm and one of the notification handlers closed the Realm ([PR #6548](https://github.com/realm/realm-core/pull/6548), since v11.8.0).
 
 ### Breaking changes
 * None.
diff --git a/src/realm/object-store/shared_realm.cpp b/src/realm/object-store/shared_realm.cpp
index 43fc4e860b4..709616f9918 100644
--- a/src/realm/object-store/shared_realm.cpp
+++ b/src/realm/object-store/shared_realm.cpp
@@ -796,6 +796,12 @@ void Realm::run_writes()
 
         do_begin_transaction();
 
+        // Beginning the transaction may have delivered notifications, which
+        // then may have closed the Realm.
+        if (!m_transaction) {
+            return;
+        }
+
         auto write_desc = std::move(m_async_write_q.front());
         m_async_write_q.pop_front();
 
diff --git a/test/object-store/realm.cpp b/test/object-store/realm.cpp
index 9c945808c71..9a0a511a542 100644
--- a/test/object-store/realm.cpp
+++ b/test/object-store/realm.cpp
@@ -1759,24 +1759,31 @@ TEST_CASE("SharedRealm: async writes") {
                 REQUIRE(complete_count == 1);
                 verify_persisted_count(1);
             }
-            SECTION("within did_change()") {
-                struct Context : public BindingContext {
-                    int i;
-                    Context(int i)
-                        : i(i)
-                    {
-                    }
-                    void did_change(std::vector<ObserverState> const&, std::vector<void*> const&, bool) override
-                    {
-                        std::invoke(close_functions[i], *realm.lock());
-                    }
-                };
-                realm->m_binding_context.reset(new Context(i));
+
+            struct Context : public BindingContext {
+                int i;
+                bool& called;
+                Context(int i, bool& called)
+                    : i(i)
+                    , called(called)
+                {
+                }
+                void did_change(std::vector<ObserverState> const&, std::vector<void*> const&, bool) override
+                {
+                    called = true;
+                    std::invoke(close_functions[i], *realm.lock());
+                }
+            };
+            SECTION("within did_change() after committing") {
+                bool called = false;
+                realm->m_binding_context.reset(new Context(i, called));
                 realm->m_binding_context->realm = realm;
 
                 realm->async_begin_transaction([&] {
                     table->create_object().set(col, 45);
+                    CHECK_FALSE(called);
                     realm->async_commit_transaction([&](std::exception_ptr) {
+                        CHECK(called);
                         done = true;
                     });
                 });
@@ -1784,6 +1791,30 @@ TEST_CASE("SharedRealm: async writes") {
                 wait_for_done();
                 verify_persisted_count(1);
             }
+
+            SECTION("within did_change() when beginning") {
+                realm->m_binding_context.reset(new Context(i, done));
+                realm->m_binding_context->realm = realm;
+
+                // Make a write on a different instance while autorefresh is
+                // off to ensure that beginning the transaction advances the
+                // read version and thus sends notifications
+                realm->set_auto_refresh(false);
+                auto realm2 = Realm::get_shared_realm(config);
+                realm2->begin_transaction();
+                realm2->commit_transaction();
+
+                bool called = false;
+                realm->async_begin_transaction([&] {
+                    called = true;
+                });
+                wait_for_done();
+
+                // close() inside a notification closes the Realm, but invalidate()
+                // is a no-op. This means the write callback should be invoked
+                // if we're testing invalidate() but not if we're testing close().
+                REQUIRE(called == i);
+            }
         }
     }