luc14n0/glib
1
0
Fork 0
mirror of https://gitlab.gnome.org/GNOME/glib.git synced 2025-06-23 04:34:51 +02:00
Code Issues Packages Projects Releases Wiki Activity

gobject: destroy closure watches one by one

Browse Source 
Previously, we would call

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

which called destroy_closure_array() on the CArray.

At that point, it would iterate over the CArray, and invalidate all
closures. But note that this invokes external callbacks, which in turn
can destroy other closures, which can call object_remove_closure().
But now that closure can no longer be found and an assertion fails.

Instead of removing the entire CArray at once, remove each closure one
by one in a loop.

This problem is similar to issue 1002, except here it's about closure
watches instead of GWeakNotify.

Note that now we destroy closures one-by-one in a loop, and we iterate
the loop as long as we have closures. That makes a difference when a new
closure gets registered while we destroy them all. Previously, newly
registered closures would survive. It would be possible to implement the
previous behavior, but I think the new behavior is better. It is anyway
a very remote use case.
This commit is contained in:
Thomas Haller 2025-02-28 16:43:57 +01:00
parent 4845314c01
commit 67fa02bdcf
1 changed files with 44 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

60
gobject/gobject.c
View File

@ -204,6 +204,7 @@ static gchar* g_value_object_lcopy_value (const GValue *value,
static void g_object_dispatch_properties_changed (GObject *object,
guint n_pspecs,
GParamSpec **pspecs);
static void closure_array_destroy_all (GObject *object);
static guint object_floating_flag_handler (GObject *object,
gint job);
static inline void object_set_optional_flags (GObject *object,
@ -1760,7 +1761,7 @@ g_object_real_dispose (GObject *object)
/* GWeakNotify and GClosure can call into user code */
g_datalist_id_set_data (&object->qdata, quark_weak_notifies, NULL);
g_datalist_id_set_data (&object->qdata, quark_closure_array, NULL);
closure_array_destroy_all (object);
}
#ifdef G_ENABLE_DEBUG
@ -4485,7 +4486,7 @@ retry_decrement:
/* The object is almost gone. Finalize. */
g_datalist_id_set_data (&object->qdata, quark_closure_array, NULL);
closure_array_destroy_all (object);
g_signal_handlers_destroy (object);
g_datalist_id_set_data (&object->qdata, quark_weak_notifies, NULL);
@ -5243,24 +5244,47 @@ object_remove_closure (gpointer data,
closure);
}
static void
destroy_closure_array (gpointer data)
static gpointer
closure_array_destroy_all_cb (gpointer *data,
GDestroyNotify *destroy_notify,
gpointer user_data)
{
CArray *carray = data;
GObject *object = carray->object;
guint i, n = carray->n_closures;
for (i = 0; i < n; i++)
CArray *carray = *data;
GClosure *closure;
if (!carray)
return NULL;
closure = carray->closures[--carray->n_closures];
if (carray->n_closures == 0)
{
g_free (carray);
*data = NULL;
}
return closure;
}
static void
closure_array_destroy_all (GObject *object)
{
GClosure *closure;
/* We invalidate closures in a loop. As this emits external callbacks, a callee
* could register another closure, which the loop would invalidate too.
*
* This is an intentional choice. Maybe it would be instead better to only
* only release the closures that were registered when the loop started. That
* would be possible, but is not done that way. */
while ((closure = _g_datalist_id_update_atomic (&object->qdata,
quark_closure_array,
closure_array_destroy_all_cb,
NULL)))
{
GClosure *closure = carray->closures[i];
/* removing object_remove_closure() upfront is probably faster than
* letting it fiddle with quark_closure_array which is empty anyways
*/
g_closure_remove_invalidate_notifier (closure, object, object_remove_closure);
g_closure_invalidate (closure);
}
g_free (carray);
}
static gpointer
@ -5280,7 +5304,11 @@ g_object_watch_closure_cb (gpointer *data,
carray->n_closures = 1;
i = 0;
*destroy_notify = destroy_closure_array;
#if G_ENABLE_DEBUG
/* We never expect there is anything to destroy. We require
* these entries to be released via closure_array_destroy_all(). */
*destroy_notify = g_destroy_notify_assert_not_reached;
#endif
}
else
{