From 457de7f08a2ab07ac7e65a6b27e5c9e09403df16 Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Thu, 18 Mar 2021 18:18:00 -0700 Subject: [PATCH 2/2] Revert "object: Do not call any function on disposed GObject pointers" This reverts commit d37d6604ca0d50b9be408acd637e8073e30500c7. --- gi/object.cpp | 15 ++++++--------- gi/toggle.cpp | 16 +++++++++------- gi/toggle.h | 2 +- installed-tests/js/testGtk3.js | 7 +++++-- 4 files changed, 21 insertions(+), 19 deletions(-) diff --git a/gi/object.cpp b/gi/object.cpp index 68410514..c811a065 100644 --- a/gi/object.cpp +++ b/gi/object.cpp @@ -1297,9 +1297,7 @@ void ObjectInstance::release_native_object(void) { discard_wrapper(); - if (m_gobj_disposed) - m_ptr.release(); - else if (m_uses_toggle_ref) + if (m_uses_toggle_ref) g_object_remove_toggle_ref(m_ptr.release(), wrapped_gobj_toggle_notify, nullptr); else @@ -1481,6 +1479,9 @@ 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) { @@ -1490,12 +1491,8 @@ ObjectInstance::disassociate_js_gobject(void) m_ptr.get(), type_name()); } - 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(); - } + /* 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 4f7db32f..5f3d4386 100644 --- a/gi/toggle.cpp +++ b/gi/toggle.cpp @@ -71,17 +71,19 @@ ToggleQueue::is_queued(GObject *gobj) const return {has_toggle_down, has_toggle_up}; } -std::pair ToggleQueue::cancel(GObject* gobj) { +std::pair +ToggleQueue::cancel(GObject *gobj) +{ debug("cancel", gobj); std::lock_guard 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 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"); + 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"); return {had_toggle_down, had_toggle_up}; } diff --git a/gi/toggle.h b/gi/toggle.h index e77c5419..f7f8eac8 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 is_queued(GObject* gobj) const; /* Cancels pending toggles and returns whether any were queued. */ - std::pair cancel(GObject* gobj); + std::pair 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 15187b67..f3f6e724 100644 --- a/installed-tests/js/testGtk3.js +++ b/installed-tests/js/testGtk3.js @@ -180,11 +180,14 @@ describe('Gtk overrides', function () { GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL, '*destroy*'); - const BadLabel = GObject.registerClass(class BadLabel extends Gtk.Label { + let BadLabel = GObject.registerClass(class BadLabel extends Gtk.Label { vfunc_destroy() {} }); - new BadLabel(); + let w = new Gtk.Window(); + w.add(new BadLabel()); + + w.destroy(); System.gc(); GLib.test_assert_expected_messages_internal('Gjs', 'testGtk3.js', 0, -- 2.30.2