Fix a crash when beginning an async write triggers notifications whic…

…h 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

@@ -11,6 +11,7 @@
 * If you freeze a Results based on a collection of objects, the result will be invalid if you delete the collection ([#6635](https://github.com/realm/realm-core/issues/6635), since V13.11.0)
 * Geospatial polygons now have built in normalization and validation in line with the MongoDB server side behaviour and the geoJSON standard. ([#6607](https://github.com/realm/realm-core/pull/6607), since v13.11.0)
 * Dictionary::get_any() would expose unresolved links rather than mapping them to null. In addition to allowing invalid objects to be read from Dictionaries, this resulted in queries on Dictionaries sometimes having incorrect results ([#6644](https://github.com/realm/realm-core/pull/6644)).
+* 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
 * `platform` and `cpu_arch` fields in the `device_info` structure in App::Config can no longer be provided by the SDK's, they are inferred by the library ([PR #6612](https://github.com/realm/realm-core/pull/6612))

src/realm/object-store/shared_realm.cpp

@@ -803,6 +803,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();
 

test/object-store/realm.cpp

@@ -1759,31 +1759,62 @@ 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;
                     });
                 });
 
                 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);
+            }
         }
     }