[gjs/gnome-40: 13/30] object: Discard wrapper when GObject is disposed from main thread

[Philip Chimento <pchimento src gnome org>]

commit 80586a039aabdf4e52e715fb1a3d17332c2f43d8
Author: Marco Trevisan (Treviño) <mail 3v1n0 net>
Date:   Fri Mar 26 02:45:01 2021 +0100

    object: Discard wrapper when GObject is disposed from main thread
    
    When disposing the object we can safely mark the object as discarded as
    it's not going to be useful anyway.
    
    As per this we can avoid triggering an error when disposing because we
    are now able to track this case while disassociating the object already.
    
    (cherry-picked from commit 593cf795)

 gi/object.cpp                                      |  10 +-
 .../js/libgjstesttools/gjs-test-tools.cpp          | 157 ++++++++++++++++++++-
 .../js/libgjstesttools/gjs-test-tools.h            |  19 +++
 installed-tests/js/testGObjectDestructionAccess.js |  45 ++++++
 4 files changed, 223 insertions(+), 8 deletions(-)
---
diff --git a/gi/object.cpp b/gi/object.cpp
index 7c8cc732..de63354c 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -1139,6 +1139,9 @@ ObjectInstance::gobj_dispose_notify(void)
         wrapped_gobj_toggle_notify(this, m_ptr, TRUE);
         m_uses_toggle_ref = false;
     }
+
+    if (GjsContextPrivate::from_current_context()->is_owner_thread())
+        discard_wrapper();
 }
 
 void ObjectInstance::iterate_wrapped_gobjects(
@@ -1735,13 +1738,6 @@ ObjectInstance::~ObjectInstance() {
         bool had_toggle_up;
         bool had_toggle_down;
 
-        if (m_gobj_finalized || G_UNLIKELY(m_ptr->ref_count <= 0)) {
-            g_error(
-                "Finalizing wrapper for an already freed object of type: "
-                "%s.%s: %p\n",
-                ns(), name(), m_ptr.get());
-        }
-
         auto& toggle_queue = ToggleQueue::get_default();
         std::tie(had_toggle_down, had_toggle_up) = toggle_queue.cancel(m_ptr);
 
diff --git a/installed-tests/js/libgjstesttools/gjs-test-tools.cpp 
b/installed-tests/js/libgjstesttools/gjs-test-tools.cpp
index 4474e6e0..1e588c97 100644
--- a/installed-tests/js/libgjstesttools/gjs-test-tools.cpp
+++ b/installed-tests/js/libgjstesttools/gjs-test-tools.cpp
@@ -4,9 +4,43 @@
 
 #include "installed-tests/js/libgjstesttools/gjs-test-tools.h"
 
+#include <mutex>
+#include <unordered_set>
+
+#include "gjs/jsapi-util.h"
+
+static GObject* m_tmp_object = NULL;
+static std::unordered_set<GObject*> m_finalized_objects;
+static std::mutex m_finalized_objects_lock;
+
+struct FinalizedObjectsLocked {
+    FinalizedObjectsLocked() : hold(m_finalized_objects_lock) {}
+
+    std::unordered_set<GObject*>* operator->() { return &m_finalized_objects; }
+    std::lock_guard<std::mutex> hold;
+};
+
 void gjs_test_tools_init() {}
 
-void gjs_test_tools_reset() {}
+void gjs_test_tools_reset() {
+    if (!FinalizedObjectsLocked()->count(m_tmp_object))
+        g_clear_object(&m_tmp_object);
+    else
+        m_tmp_object = nullptr;
+
+    FinalizedObjectsLocked()->clear();
+}
+
+// clang-format off
+static G_DEFINE_QUARK(gjs-test-utils::finalize, finalize);
+// clang-format on
+
+static void monitor_object_finalization(GObject* object) {
+    g_object_steal_qdata(object, finalize_quark());
+    g_object_set_qdata_full(object, finalize_quark(), object, [](void* data) {
+        FinalizedObjectsLocked()->insert(static_cast<GObject*>(data));
+    });
+}
 
 void gjs_test_tools_delayed_ref(GObject* object, int interval) {
     g_timeout_add(
@@ -37,3 +71,124 @@ void gjs_test_tools_delayed_dispose(GObject* object, int interval) {
         },
         object);
 }
+
+void gjs_test_tools_save_object(GObject* object) {
+    g_assert(!m_tmp_object);
+    g_set_object(&m_tmp_object, object);
+    monitor_object_finalization(object);
+}
+
+void gjs_test_tools_ref_other_thread(GObject* object) {
+    // cppcheck-suppress leakNoVarFunctionCall
+    g_thread_join(g_thread_new("ref_object", g_object_ref, object));
+}
+
+typedef enum {
+    REF = 1 << 0,
+    UNREF = 1 << 1,
+} RefType;
+
+typedef struct {
+    GObject* object;
+    RefType ref_type;
+    int delay;
+} RefThreadData;
+
+static RefThreadData* ref_thread_data_new(GObject* object, int interval,
+                                          RefType ref_type) {
+    auto* ref_data = g_new(RefThreadData, 1);
+
+    ref_data->object = object;
+    ref_data->delay = interval;
+    ref_data->ref_type = ref_type;
+
+    monitor_object_finalization(object);
+
+    return ref_data;
+}
+
+static void* ref_thread_func(void* data) {
+    GjsAutoPointer<RefThreadData, void, g_free> ref_data =
+        static_cast<RefThreadData*>(data);
+
+    if (FinalizedObjectsLocked()->count(ref_data->object))
+        return nullptr;
+
+    if (ref_data->delay > 0)
+        g_usleep(ref_data->delay);
+
+    if (FinalizedObjectsLocked()->count(ref_data->object))
+        return nullptr;
+
+    if (ref_data->ref_type & REF)
+        g_object_ref(ref_data->object);
+
+    if (!(ref_data->ref_type & UNREF)) {
+        return ref_data->object;
+    } else if (ref_data->ref_type & REF) {
+        g_usleep(ref_data->delay);
+
+        if (FinalizedObjectsLocked()->count(ref_data->object))
+            return nullptr;
+    }
+
+    if (ref_data->object != m_tmp_object)
+        g_object_steal_qdata(ref_data->object, finalize_quark());
+    g_object_unref(ref_data->object);
+    return nullptr;
+}
+
+void gjs_test_tools_unref_other_thread(GObject* object) {
+    // cppcheck-suppress leakNoVarFunctionCall
+    g_thread_join(g_thread_new("unref_object", ref_thread_func,
+                               ref_thread_data_new(object, -1, UNREF)));
+}
+
+void gjs_test_tools_delayed_ref_other_thread(GObject* object, int interval) {
+    g_thread_unref(g_thread_new("ref_object", ref_thread_func,
+                                ref_thread_data_new(object, interval, REF)));
+}
+
+void gjs_test_tools_delayed_unref_other_thread(GObject* object, int interval) {
+    g_thread_unref(g_thread_new("unref_object", ref_thread_func,
+                                ref_thread_data_new(object, interval, UNREF)));
+}
+
+void gjs_test_tools_delayed_ref_unref_other_thread(GObject* object,
+                                                   int interval) {
+    g_thread_unref(
+        g_thread_new("ref_unref_object", ref_thread_func,
+                     ref_thread_data_new(object, interval,
+                                         static_cast<RefType>(REF | UNREF))));
+}
+
+void gjs_test_tools_run_dispose_other_thread(GObject* object) {
+    // cppcheck-suppress leakNoVarFunctionCall
+    g_thread_join(g_thread_new(
+        "run_dispose",
+        [](void* object) -> void* {
+            g_object_run_dispose(G_OBJECT(object));
+            return nullptr;
+        },
+        object));
+}
+
+/**
+ * gjs_test_tools_get_saved:
+ * Returns: (transfer full)
+ */
+GObject* gjs_test_tools_get_saved() {
+    if (FinalizedObjectsLocked()->count(m_tmp_object))
+        m_tmp_object = nullptr;
+
+    return static_cast<GObject*>(g_steal_pointer(&m_tmp_object));
+}
+
+/**
+ * gjs_test_tools_get_disposed:
+ * Returns: (transfer none)
+ */
+GObject* gjs_test_tools_get_disposed(GObject* object) {
+    g_object_run_dispose(G_OBJECT(object));
+    return object;
+}
diff --git a/installed-tests/js/libgjstesttools/gjs-test-tools.h 
b/installed-tests/js/libgjstesttools/gjs-test-tools.h
index 478a2bc3..d56fa0f3 100644
--- a/installed-tests/js/libgjstesttools/gjs-test-tools.h
+++ b/installed-tests/js/libgjstesttools/gjs-test-tools.h
@@ -20,4 +20,23 @@ void gjs_test_tools_delayed_unref(GObject* object, int interval);
 
 void gjs_test_tools_delayed_dispose(GObject* object, int interval);
 
+void gjs_test_tools_ref_other_thread(GObject* object);
+
+void gjs_test_tools_delayed_ref_other_thread(GObject* object, int interval);
+
+void gjs_test_tools_unref_other_thread(GObject* object);
+
+void gjs_test_tools_delayed_unref_other_thread(GObject* object, int interval);
+
+void gjs_test_tools_delayed_ref_unref_other_thread(GObject* object,
+                                                   int interval);
+
+void gjs_test_tools_run_dispose_other_thread(GObject* object);
+
+void gjs_test_tools_save_object(GObject* object);
+
+GObject* gjs_test_tools_get_saved();
+
+GObject* gjs_test_tools_get_disposed(GObject* object);
+
 G_END_DECLS
diff --git a/installed-tests/js/testGObjectDestructionAccess.js 
b/installed-tests/js/testGObjectDestructionAccess.js
index a0f2c99a..a7999be5 100644
--- a/installed-tests/js/testGObjectDestructionAccess.js
+++ b/installed-tests/js/testGObjectDestructionAccess.js
@@ -229,4 +229,49 @@ describe('Disposed or finalized GObject', function () {
         GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectDestructionAccess.js', 0,
             'generates a warn if already disposed at garbage collection');
     });
+
+    it('created from other function is marked as disposed', function () {
+        let file = Gio.File.new_for_path('/');
+        GjsTestTools.save_object(file);
+        file.run_dispose();
+        file = null;
+        System.gc();
+
+        expect(GjsTestTools.get_saved()).toMatch(
+            /\[object \(DISPOSED\) instance wrapper GType:GLocalFile jsobj@0x[a-f0-9]+ 
native@0x[a-f0-9]+\]/);
+    });
+
+    it('returned from function is marked as disposed', function () {
+        expect(GjsTestTools.get_disposed(Gio.File.new_for_path('/'))).toMatch(
+            /\[object \(DISPOSED\) instance wrapper GType:GLocalFile jsobj@0x[a-f0-9]+ 
native@0x[a-f0-9]+\]/);
+    });
+
+    it('returned from function is marked as disposed and then as finalized', function () {
+        let file = Gio.File.new_for_path('/');
+        GjsTestTools.save_object(file);
+        GjsTestTools.delayed_unref(file, 30);
+        file.run_dispose();
+
+        let disposedFile = GjsTestTools.get_saved();
+        expect(disposedFile).toEqual(file);
+        expect(disposedFile).toMatch(
+            /\[object \(DISPOSED\) instance wrapper GType:GLocalFile jsobj@0x[a-f0-9]+ 
native@0x[a-f0-9]+\]/);
+
+        file = null;
+        System.gc();
+
+        const loop = new GLib.MainLoop(null, false);
+        GLib.timeout_add(GLib.PRIORITY_DEFAULT, 50, () => loop.quit());
+        loop.run();
+
+        expect(disposedFile).toMatch(
+            /\[object \(FINALIZED\) instance wrapper GType:GLocalFile jsobj@0x[a-f0-9]+ 
native@0x[a-f0-9]+\]/);
+
+        GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
+            '*Object 0x* has been finalized *');
+        disposedFile = null;
+        System.gc();
+        GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectDestructionAccess.js', 0,
+            'returned from function is marked as disposed and then as finalized');
+    });
 });