diff --git a/gobject/gobject.c b/gobject/gobject.c index 07aaa5e49..8ef9ed42a 100644 --- a/gobject/gobject.c +++ b/gobject/gobject.c @@ -208,6 +208,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); @@ -1872,7 +1873,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); closure_array_destroy_all (object); } @@ -3754,25 +3755,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, @@ -3791,7 +3783,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 { @@ -3801,8 +3799,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; } @@ -3901,6 +3901,57 @@ 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; + + /* We notify weak references in a loop. As this emits external callbacks, a + * callee could register another weak reference, which the loop would notify + * too. + * + * This is an intentional choice. Maybe it would be instead better to only + * only release the entries that were registered when the loop started. That + * would be possible, but is not done that way. */ + 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. @@ -4642,7 +4693,7 @@ retry_decrement: closure_array_destroy_all (object); 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); diff --git a/gobject/tests/references.c b/gobject/tests/references.c index 2eb994cf4..cb71dd7ff 100644 --- a/gobject/tests/references.c +++ b/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 (); }