[gjs: 1/3] object: Do not call any function on disposed GObject pointers

[Philip Chimento <pchimento src gnome org>]

commit d37d6604ca0d50b9be408acd637e8073e30500c7
Author: Marco Trevisan (Treviño) <mail 3v1n0 net>
Date:   Fri Mar 12 21:25:24 2021 +0100

    object: Do not call any function on disposed GObject pointers
    
    After an object is disposed we should really not perform anything with
    it, however we may end up calling some GObject functions during
    disposition.
    
    In particular, we try to print its name, unset the qdata and unref it
    again.
    
    So during disassociation always ensure that the object we're
    disassociating has not been already destroyed, before doing such
    operations.

 gi/object.cpp                  | 15 +++++++++------
 gi/toggle.cpp                  | 16 +++++++---------
 gi/toggle.h                    |  2 +-
 installed-tests/js/testGtk3.js |  7 ++-----
 4 files changed, 19 insertions(+), 21 deletions(-)
---
diff --git a/gi/object.cpp b/gi/object.cpp
index c811a065..68410514 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -1297,7 +1297,9 @@ void
 ObjectInstance::release_native_object(void)
 {
     discard_wrapper();
-    if (m_uses_toggle_ref)
+    if (m_gobj_disposed)
+        m_ptr.release();
+    else if (m_uses_toggle_ref)
         g_object_remove_toggle_ref(m_ptr.release(), wrapped_gobj_toggle_notify,
                                    nullptr);
     else
@@ -1479,9 +1481,6 @@ ObjectInstance::disassociate_js_gobject(void)
 {
     bool had_toggle_down, had_toggle_up;
 
-    if (!m_gobj_disposed)
-        g_object_weak_unref(m_ptr.get(), wrapped_gobj_dispose_notify, this);
-
     auto& toggle_queue = ToggleQueue::get_default();
     std::tie(had_toggle_down, had_toggle_up) = toggle_queue.cancel(m_ptr.get());
     if (had_toggle_down != had_toggle_up) {
@@ -1491,8 +1490,12 @@ ObjectInstance::disassociate_js_gobject(void)
             m_ptr.get(), type_name());
     }
 
-    /* Fist, remove the wrapper pointer from the wrapped GObject */
-    unset_object_qdata();
+    if (!m_gobj_disposed) {
+        g_object_weak_unref(m_ptr.get(), wrapped_gobj_dispose_notify, this);
+
+        /* Fist, remove the wrapper pointer from the wrapped GObject */
+        unset_object_qdata();
+    }
 
     /* Now release all the resources the current wrapper has */
     invalidate_closure_list(&m_closures);
diff --git a/gi/toggle.cpp b/gi/toggle.cpp
index 5f3d4386..4f7db32f 100644
--- a/gi/toggle.cpp
+++ b/gi/toggle.cpp
@@ -71,19 +71,17 @@ ToggleQueue::is_queued(GObject *gobj) const
     return {has_toggle_down, has_toggle_up};
 }
 
-std::pair<bool, bool>
-ToggleQueue::cancel(GObject *gobj)
-{
+std::pair<bool, bool> ToggleQueue::cancel(GObject* gobj) {
     debug("cancel", gobj);
     std::lock_guard<std::mutex> hold(lock);
     bool had_toggle_down = find_and_erase_operation_locked(gobj, DOWN);
     bool had_toggle_up = find_and_erase_operation_locked(gobj, UP);
-    gjs_debug_lifecycle(GJS_DEBUG_GOBJECT, "ToggleQueue: %p (%s) was %s", gobj,
-                        G_OBJECT_TYPE_NAME(gobj),
-                        had_toggle_down && had_toggle_up ? "queued to toggle BOTH"
-                            : had_toggle_down ? "queued to toggle DOWN"
-                            : had_toggle_up ? "queued to toggle UP"
-                            : "not queued");
+    gjs_debug_lifecycle(GJS_DEBUG_GOBJECT, "ToggleQueue: %p was %s", gobj,
+                        had_toggle_down && had_toggle_up
+                            ? "queued to toggle BOTH"
+                        : had_toggle_down ? "queued to toggle DOWN"
+                        : had_toggle_up   ? "queued to toggle UP"
+                                          : "not queued");
     return {had_toggle_down, had_toggle_up};
 }
 
diff --git a/gi/toggle.h b/gi/toggle.h
index f7f8eac8..e77c5419 100644
--- a/gi/toggle.h
+++ b/gi/toggle.h
@@ -66,7 +66,7 @@ private:
      * are / were queued. is_queued() just checks and does not modify. */
     [[nodiscard]] std::pair<bool, bool> is_queued(GObject* gobj) const;
     /* Cancels pending toggles and returns whether any were queued. */
-    std::pair<bool, bool> cancel(GObject *gobj);
+    std::pair<bool, bool> cancel(GObject* gobj);
 
     /* Pops a toggle from the queue and processes it. Call this if you don't
      * want to wait for it to be processed in idle time. Returns false if queue
diff --git a/installed-tests/js/testGtk3.js b/installed-tests/js/testGtk3.js
index f3f6e724..15187b67 100644
--- a/installed-tests/js/testGtk3.js
+++ b/installed-tests/js/testGtk3.js
@@ -180,14 +180,11 @@ describe('Gtk overrides', function () {
         GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
             '*destroy*');
 
-        let BadLabel = GObject.registerClass(class BadLabel extends Gtk.Label {
+        const BadLabel = GObject.registerClass(class BadLabel extends Gtk.Label {
             vfunc_destroy() {}
         });
 
-        let w = new Gtk.Window();
-        w.add(new BadLabel());
-
-        w.destroy();
+        new BadLabel();
         System.gc();
 
         GLib.test_assert_expected_messages_internal('Gjs', 'testGtk3.js', 0,