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

gobject: avoid potential race and use g_datalist_id_update_atomic() for closure array

Browse Source 
There are two calls to

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

that don't take a OPTIONAL_BIT_LOCK_CLOSURE_ARRAY lock. These are inside
g_object_real_dispose() and right before finalize(). The one before
finalize() is fine, becase we are already in a situation where nobody
else holds a reference on object.

However not so with g_object_real_dispose().  That is called after we
checked that there is only one strong reference left and we are inside
the call to dispose(). However, at that point (before chaining up
g_object_real_dispose()) the callee is able can pass the reference
to another thread. That other thread could create a Closure and destroy it
again. This calles object_remove_closure() (accessing CArray) which now
races against g_object_real_dispose() (destroying CArray).

Granted, this is very unlikely to happen. But let's try to avoid such
races in principle.

We can avoid this problem with less overhead by doing everything while
holding the GData lock, using g_datalist_id_update_atomic().  This is
probably even faster, as we don't need the additional
OPTIONAL_BIT_LOCK_CLOSURE_ARRAY lock.

Also free the empty closure data during object_remove_closure(). This
frees some unused memory.
This commit is contained in:
Thomas Haller 2024-01-29 15:41:40 +01:00
parent 84f555a3aa
commit dec3ba69e8
1 changed files with 76 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

119
gobject/gobject.c
View File

@ -127,7 +127,6 @@ enum {
#define OPTIONAL_BIT_LOCK_WEAK_REFS 1
#define OPTIONAL_BIT_LOCK_NOTIFY 2
#define OPTIONAL_BIT_LOCK_TOGGLE_REFS 3
#define OPTIONAL_BIT_LOCK_CLOSURE_ARRAY 4
#if SIZEOF_INT == 4 && GLIB_SIZEOF_VOID_P >= 8
#define HAVE_OPTIONAL_FLAGS_IN_GOBJECT 1
@ -5204,27 +5203,44 @@ typedef struct {
GClosure *closures[1]; /* flexible array */
} CArray;
static gpointer
object_remove_closure_cb (gpointer *data,
GDestroyNotify *destroy_notify,
gpointer user_data)
{
GClosure *closure = user_data;
CArray *carray = *data;
guint i;
for (i = 0; i < carray->n_closures; i++)
{
if (carray->closures[i] == closure)
{
carray->n_closures--;
if (carray->n_closures == 0)
{
g_free (carray);
*data = NULL;
}
else if (i < carray->n_closures)
carray->closures[i] = carray->closures[carray->n_closures];
return NULL;
}
}
g_return_val_if_reached (NULL);
}
static void
object_remove_closure (gpointer data,
GClosure *closure)
object_remove_closure (gpointer data,
GClosure *closure)
{
GObject *object = data;
CArray *carray;
guint i;
object_bit_lock (object, OPTIONAL_BIT_LOCK_CLOSURE_ARRAY);
carray = g_object_get_qdata (object, quark_closure_array);
for (i = 0; i < carray->n_closures; i++)
if (carray->closures[i] == closure)
{
carray->n_closures--;
if (i < carray->n_closures)
carray->closures[i] = carray->closures[carray->n_closures];
object_bit_unlock (object, OPTIONAL_BIT_LOCK_CLOSURE_ARRAY);
return;
}
object_bit_unlock (object, OPTIONAL_BIT_LOCK_CLOSURE_ARRAY);
g_assert_not_reached ();
_g_datalist_id_update_atomic (&object->qdata,
quark_closure_array,
object_remove_closure_cb,
closure);
}
static void
@ -5247,6 +5263,38 @@ destroy_closure_array (gpointer data)
g_free (carray);
}
static gpointer
g_object_watch_closure_cb (gpointer *data,
GDestroyNotify *destroy_notify,
gpointer user_data)
{
GObject *object = ((gpointer *) user_data)[0];
GClosure *closure = ((gpointer *) user_data)[1];
CArray *carray = *data;
guint i;
if (!carray)
{
carray = g_new (CArray, 1);
carray->object = object;
carray->n_closures = 1;
i = 0;
*destroy_notify = destroy_closure_array;
}
else
{
i = carray->n_closures++;
carray = g_realloc (carray, sizeof (*carray) + sizeof (carray->closures[0]) * i);
}
*data = carray;
carray->closures[i] = closure;
return NULL;
}
/**
* g_object_watch_closure:
* @object: #GObject restricting lifetime of @closure
@ -5266,36 +5314,21 @@ void
g_object_watch_closure (GObject *object,
GClosure *closure)
{
CArray *carray;
guint i;
g_return_if_fail (G_IS_OBJECT (object));
g_return_if_fail (closure != NULL);
g_return_if_fail (closure->is_invalid == FALSE);
g_return_if_fail (closure->in_marshal == FALSE);
g_return_if_fail (g_atomic_int_get (&object->ref_count) > 0); /* this doesn't work on finalizing objects */
g_return_if_fail (g_atomic_int_get (&object->ref_count) > 0); /* this doesn't work on finalizing objects */
g_closure_add_invalidate_notifier (closure, object, object_remove_closure);
g_closure_add_marshal_guards (closure,
object, (GClosureNotify) g_object_ref,
object, (GClosureNotify) g_object_unref);
object_bit_lock (object, OPTIONAL_BIT_LOCK_CLOSURE_ARRAY);
carray = g_datalist_id_remove_no_notify (&object->qdata, quark_closure_array);
if (!carray)
{
carray = g_renew (CArray, NULL, 1);
carray->object = object;
carray->n_closures = 1;
i = 0;
}
else
{
i = carray->n_closures++;
carray = g_realloc (carray, sizeof (*carray) + sizeof (carray->closures[0]) * i);
}
carray->closures[i] = closure;
g_datalist_id_set_data_full (&object->qdata, quark_closure_array, carray, destroy_closure_array);
object_bit_unlock (object, OPTIONAL_BIT_LOCK_CLOSURE_ARRAY);
object, (GClosureNotify) g_object_ref,
object, (GClosureNotify) g_object_unref);
_g_datalist_id_update_atomic (&object->qdata,
quark_closure_array,
g_object_watch_closure_cb,
((gpointer[]){ object, closure }));
}
/**