toggle: Rely on wrapper to cancel finalized objects in queue

While GObject provides native tools to check if an object has been
finalized using GWeakRef's, they have proved to be too fragile for us
because they don't work well when used with toggle references
notifications (not thread safe as glib#2384 demonstrates) and because
may not be cleared on finalization (glib#2390).

So, instead of adding further qdata to monitor a queued object
finalization, let's just ensure that a wrapper cancels the toggle queue
when its wrapped object get finalized.

Since we don't touch the references of the object while adding it to the
queue, we can also ignore remembering the current handled object, as no
recursion can happen now.

It may still happen (mostly due to the way we lock) that a finalized
or unhandled object will be handled by the toggle handler, but this
can't be really fixed for all the racy cases even at enqueue phase
(like in the case a thread enqueues a toggle event just after we've
disassociated the wrapper for such object).

Fixes: #404

1 files changed, 6 insertions, 4 deletions

diff --git a/gi/object.cpp b/gi/object.cpp
index 920d8c15..f8f9eb6a 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -261,6 +261,8 @@ ObjectInstance::set_object_qdata(void)
                     self->m_ptr.get(), g_type_name(self->gtype()));
                 self->m_gobj_disposed = true;
             }
+            if (ToggleQueue::get_default().cancel(self->m_ptr).first)
+                self->toggle_down();
             self->m_gobj_finalized = true;
             gjs_debug_lifecycle(GJS_DEBUG_GOBJECT,
                                 "Wrapped GObject %p finalized",
@@ -1120,6 +1122,8 @@ void ObjectInstance::track_gobject_finalization() {
     g_object_steal_qdata(m_ptr, quark);
     g_object_set_qdata_full(m_ptr, quark, this, [](void* data) {
         auto* self = static_cast<ObjectInstance*>(data);
+        if (ToggleQueue::get_default().cancel(self->m_ptr).first)
+            self->toggle_down();
         self->m_gobj_finalized = true;
         gjs_debug_lifecycle(GJS_DEBUG_GOBJECT, "Wrapped GObject %p finalized",
                             self->m_ptr.get());
@@ -1271,7 +1275,8 @@ toggle_handler(GObject               *gobj,
     if (G_UNLIKELY(!self)) {
         void* disposed = g_object_get_qdata(gobj, ObjectBase::disposed_quark());
 
-        if (G_UNLIKELY(disposed == gjs_int_to_pointer(DISPOSED_OBJECT))) {
+        if (G_UNLIKELY(!disposed ||
+                       disposed == gjs_int_to_pointer(DISPOSED_OBJECT))) {
             g_critical("Handling toggle %s for an unknown object %p",
                        direction == ToggleQueue::UP ? "up" : "down", gobj);
             return;
@@ -1338,9 +1343,6 @@ void ObjectInstance::wrapped_gobj_toggle_notify(void* instance, GObject* gobj,
     is_main_thread = gjs->is_owner_thread();
 
     auto& toggle_queue = ToggleQueue::get_default();
-    if (is_main_thread && toggle_queue.is_being_handled(gobj))
-        return;
-
     std::tie(toggle_down_queued, toggle_up_queued) = toggle_queue.is_queued(gobj);
 
     if (is_last_ref) {