[glib/ebassi/dispose-notify] Defer GObject::notify during object destruction

[Emmanuele Bassi <ebassi src gnome org>]

commit d9c873afa939cee133a059f8bc0123025225c9be
Author: Emmanuele Bassi <ebassi gnome org>
Date:   Sun Nov 28 00:43:01 2021 +0000

    Defer GObject::notify during object destruction
    
    Notifying during object destruction is a dubious "feature": objects
    might end up recreating a bunch of state just before clearing it;
    language bindings might get spurious notifications during garbage
    collection runs.
    
    We freeze the notification queue before running the dispose() chain; if
    the object was temporarily vivified during dispose, we thaw the
    notification queue, otherwise we drain the queue right before running
    the finalize() chain.
    
    See: https://gitlab.gnome.org/GNOME/gjs/-/issues/445

 gobject/gobject.c | 38 +++++++++++++++++++++++++++++++-------
 1 file changed, 31 insertions(+), 7 deletions(-)
---
diff --git a/gobject/gobject.c b/gobject/gobject.c
index 33db8090c..ff7eae3e1 100644
--- a/gobject/gobject.c
+++ b/gobject/gobject.c
@@ -303,6 +303,7 @@ g_object_notify_queue_freeze (GObject  *object,
                G_OBJECT_TYPE_NAME (object), object);
   else
     nqueue->freeze_count++;
+
   G_UNLOCK(notify_lock);
 
   return nqueue;
@@ -349,15 +350,11 @@ g_object_notify_queue_thaw (GObject            *object,
   g_free (free_me);
 }
 
+#ifdef G_ENABLE_DEBUG
 static void
-g_object_notify_queue_drain (GObject            *object
+g_object_notify_queue_drain (GObject            *object,
                              GObjectNotifyQueue *nqueue)
 {
-  GSList *slist;
-  guint n_pspecs = 0;
-
-  g_return_if_fail (g_atomic_int_get (&object->ref_count) > 0);
-
   G_LOCK (notify_lock);
 
   /* Just make sure we never get into some nasty race condition */
@@ -376,6 +373,7 @@ g_object_notify_queue_drain (GObject            *object
 
   G_UNLOCK (notify_lock);
 }
+#endif
 
 static void
 g_object_notify_queue_add (GObject            *object,
@@ -3602,6 +3600,7 @@ g_object_unref (gpointer _object)
   else
     {
       GSList **weak_locations;
+      GObjectNotifyQueue *nqueue;
 
       /* The only way that this object can live at this point is if
        * there are outstanding weak references already established
@@ -3643,6 +3642,11 @@ g_object_unref (gpointer _object)
           g_rw_lock_writer_unlock (&weak_locations_lock);
         }
 
+      /* freeze the notification queue, so we don't accidentally emit
+       * notifications during dispose() and finalize()
+       */
+      nqueue = g_object_notify_queue_freeze (object, FALSE);
+
       /* we are about to remove the last reference */
       TRACE (GOBJECT_OBJECT_DISPOSE(object,G_TYPE_FROM_INSTANCE(object), 1));
       G_OBJECT_GET_CLASS (object)->dispose (object);
@@ -3659,6 +3663,9 @@ g_object_unref (gpointer _object)
           if (!g_atomic_int_compare_and_exchange ((int *)&object->ref_count, old_ref, old_ref - 1))
            goto retry_atomic_decrement2;
 
+          /* emit all notifications that have been queued during dispose() */
+          g_object_notify_queue_thaw (object, nqueue);
+
          TRACE (GOBJECT_OBJECT_UNREF(object,G_TYPE_FROM_INSTANCE(object),old_ref));
 
           /* if we went from 2->1 we need to notify toggle refs if any */
@@ -3685,9 +3692,19 @@ g_object_unref (gpointer _object)
        {
          TRACE (GOBJECT_OBJECT_FINALIZE(object,G_TYPE_FROM_INSTANCE(object)));
           G_OBJECT_GET_CLASS (object)->finalize (object);
-
          TRACE (GOBJECT_OBJECT_FINALIZE_END(object,G_TYPE_FROM_INSTANCE(object)));
 
+#ifdef G_ENABLE_DEBUG
+          /* if there were notifications, it's too late to emit them now;
+           * any signal handler for the "notify" signal has been dropped
+           * anyhow; we need to retrieve the notification queue again to
+           * ensure we are not re-entering this code from a temporary
+           * vivification
+           */
+          nqueue = g_object_notify_queue_freeze (object, FALSE);
+          g_object_notify_queue_drain (object, nqueue);
+#endif
+
           GOBJECT_IF_DEBUG (OBJECTS,
            {
               gboolean was_present;
@@ -3703,6 +3720,13 @@ g_object_unref (gpointer _object)
            });
           g_type_free_instance ((GTypeInstance*) object);
        }
+      else
+        {
+          /* The instance acquired a reference between dispose() and
+           * finalize(), so we need to thaw the notification queue
+           */
+          g_object_notify_queue_thaw (object, nqueue);
+        }
     }
 }