gobject: invoke g_object_weak_ref() one-by-one during destruction

Previously, at two places (in g_object_real_dispose() and shortly before
finalize()), we would call

    g_datalist_id_set_data (&object->qdata, quark_weak_notifies, NULL);

This clears @quark_weak_notifies at once and then invokes all
notifications.

This means, if you were inside a notification and called
g_object_weak_unref() on the object for *another* weak-reference, then
an exception failed:

  GLib-GObject-FATAL-CRITICAL: g_object_weak_unref_cb: couldn't find weak ref 0x401320(0x16b9fe0)

Granted, maybe inside a GWeakNotify you shouldn't call anything on
where_the_object_was. However, unregistering things (like calling
g_object_weak_unref() should still reasonably work.

Instead, now remove each weak notification one by one and invoke it.

https://gitlab.gnome.org/GNOME/glib/-/issues/1002

gobject/gobject.c

@@ -206,6 +206,7 @@ static guint               object_floating_flag_handler (GObject        *object,
                                                          gint            job);
 static inline void object_set_optional_flags (GObject *object,
                                               guint flags);
+static void g_object_weak_release_all (GObject *object);
 
 static void object_interface_check_properties           (gpointer        check_data,
 							 gpointer        g_iface);
@@ -1812,7 +1813,7 @@ g_object_real_dispose (GObject *object)
   g_signal_handlers_destroy (object);
 
   /* GWeakNotify and GClosure can call into user code */
-  g_datalist_id_set_data (&object->qdata, quark_weak_notifies, NULL);
+  g_object_weak_release_all (object);
   g_datalist_id_set_data (&object->qdata, quark_closure_array, NULL);
 }
 
@@ -3694,25 +3695,16 @@ g_object_disconnect (gpointer     _object,
 }
 
 typedef struct {
-  GObject *object;
-  guint n_weak_refs;
-  struct {
     GWeakNotify notify;
     gpointer    data;
-  } weak_refs[1];  /* flexible array */
+} WeakRefTuple;
+
+typedef struct {
+  GObject *object;
+  guint n_weak_refs;
+  WeakRefTuple weak_refs[1]; /* flexible array */
 } WeakRefStack;
 
-static void
-weak_refs_notify (gpointer data)
-{
-  WeakRefStack *wstack = data;
-  guint i;
-
-  for (i = 0; i < wstack->n_weak_refs; i++)
-    wstack->weak_refs[i].notify (wstack->weak_refs[i].data, wstack->object);
-  g_free (wstack);
-}
-
 static gpointer
 g_object_weak_ref_cb (gpointer *data,
                       GDestroyNotify *destroy_notify,
@@ -3731,7 +3723,13 @@ g_object_weak_ref_cb (gpointer *data,
       wstack->n_weak_refs = 1;
       i = 0;
 
-      *destroy_notify = weak_refs_notify;
+      /* We don't set a @destroy_notify. Shortly before finalize(), we call
+       * g_object_weak_release_all(), which frees the WeakRefStack. At that
+       * point, it is a bug to resurrect the object or call g_object_weak_ref()
+       * again. */
+#if G_ENABLE_DEBUG
+      *destroy_notify = g_destroy_notify_assert_not_reached;
+#endif
     }
   else
     {
@@ -3741,8 +3739,10 @@ g_object_weak_ref_cb (gpointer *data,
 
   *data = wstack;
 
-  wstack->weak_refs[i].notify = notify;
-  wstack->weak_refs[i].data = notify_data;
+  wstack->weak_refs[i] = (WeakRefTuple){
+    .notify = notify,
+    .data = notify_data,
+  };
 
   return NULL;
 }
@@ -3841,6 +3841,50 @@ g_object_weak_unref (GObject *object,
                                 ((gpointer[]){ notify, data }));
 }
 
+static gpointer
+g_object_weak_release_all_cb (gpointer *data,
+                              GDestroyNotify *destroy_notify,
+                              gpointer user_data)
+{
+  WeakRefTuple *tuple = user_data;
+  WeakRefStack *wstack = *data;
+
+  if (!wstack)
+    return NULL;
+
+#if G_ENABLE_DEBUG
+  g_assert (wstack->n_weak_refs > 0);
+#endif
+
+  /* We invoke the weak notifications in reverse order, in which
+   * they were registered. Maybe the other order would make more
+   * sense, but this way it's cheaper. Better don't rely on it. */
+
+  wstack->n_weak_refs--;
+
+  *tuple = wstack->weak_refs[wstack->n_weak_refs];
+
+  if (wstack->n_weak_refs == 0)
+    {
+      g_free (wstack);
+      *data = NULL;
+    }
+
+  return tuple;
+}
+
+static void
+g_object_weak_release_all (GObject *object)
+{
+  WeakRefTuple tuple;
+
+  while (_g_datalist_id_update_atomic (&object->qdata,
+                                       quark_weak_notifies,
+                                       g_object_weak_release_all_cb,
+                                       &tuple))
+    tuple.notify (tuple.data, object);
+}
+
 /**
  * g_object_add_weak_pointer: (skip)
  * @object: The object that should be weak referenced.
@@ -4582,7 +4626,7 @@ retry_decrement:
 
   g_datalist_id_set_data (&object->qdata, quark_closure_array, NULL);
   g_signal_handlers_destroy (object);
-  g_datalist_id_set_data (&object->qdata, quark_weak_notifies, NULL);
+  g_object_weak_release_all (object);
 
   TRACE (GOBJECT_OBJECT_FINALIZE (object, G_TYPE_FROM_INSTANCE (object)));
   G_OBJECT_GET_CLASS (object)->finalize (object);

gobject/gtype-private.h

@@ -99,6 +99,8 @@ gboolean    _g_object_has_signal_handler     (GObject     *object);
 void        _g_object_set_has_signal_handler (GObject     *object,
                                               guint        signal_id);
 
+void g_destroy_notify_assert_not_reached (gpointer object);
+
 /**
  * _G_DEFINE_TYPE_EXTENDED_WITH_PRELUDE:
  *

gobject/gtype.c

@@ -5082,3 +5082,9 @@ g_type_ensure (GType type)
   if (G_UNLIKELY (type == (GType)-1))
     g_error ("can't happen");
 }
+
+void
+g_destroy_notify_assert_not_reached (gpointer object)
+{
+  g_return_if_reached ();
+}

gobject/tests/references.c

@@ -282,6 +282,39 @@ test_references (void)
   g_assert_true (object_destroyed);
 }
 
+/*****************************************************************************/
+
+static void notify_two (gpointer data, GObject *former_object);
+
+static void
+notify_one (gpointer data, GObject *former_object)
+{
+  g_object_weak_unref (data, notify_two, former_object);
+}
+
+static void
+notify_two (gpointer data, GObject *former_object)
+{
+  g_object_weak_unref (data, notify_one, former_object);
+}
+
+static void
+test_references_two (void)
+{
+  GObject *obj;
+
+  /* https://gitlab.gnome.org/GNOME/gtk/-/issues/5542#note_1688809 */
+
+  obj = g_object_new (G_TYPE_OBJECT, NULL);
+
+  g_object_weak_ref (obj, notify_one, obj);
+  g_object_weak_ref (obj, notify_two, obj);
+
+  g_object_unref (obj);
+}
+
+/*****************************************************************************/
+
 int
 main (int   argc,
       char *argv[])
@@ -293,6 +326,7 @@ main (int   argc,
   g_test_init (&argc, &argv, NULL);
 
   g_test_add_func ("/gobject/references", test_references);
+  g_test_add_func ("/gobject/references_two", test_references_two);
 
   return g_test_run ();
 }