luc14n0/glib
1
0
Fork 0
mirror of https://gitlab.gnome.org/GNOME/glib.git synced 2025-02-23 18:52:09 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'wip/3v1n0/gmain-source-reffing-fixes' into 'main'

Browse Source 
glib/gmain: Avoid potential dispose race on GSource due to refcount dance

Closes #3612

See merge request GNOME/glib!4501
This commit is contained in:
Marco Trevisan 2025-02-19 18:26:19 +00:00
commit 30fbe0e859
1 changed files with 43 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
glib/gmain.c
View File

@ -2191,12 +2191,13 @@ g_source_set_name_by_id (guint tag,
GSource * GSource *
g_source_ref (GSource *source) g_source_ref (GSource *source)
{ {
int old_ref G_GNUC_UNUSED;
g_return_val_if_fail (source != NULL, NULL); g_return_val_if_fail (source != NULL, NULL);
old_ref = g_atomic_int_add (&source->ref_count, 1);
/* We allow ref_count == 0 here to allow the dispose function to resurrect /* We allow ref_count == 0 here to allow the dispose function to resurrect
* the GSource if needed */ * the GSource if needed */
g_return_val_if_fail (g_atomic_int_get (&source->ref_count) >= 0, NULL); g_return_val_if_fail (old_ref >= 0, NULL);
g_atomic_int_inc (&source->ref_count);
return source; return source;
} }
@ -2210,34 +2211,60 @@ g_source_unref_internal (GSource *source,
{ {
gpointer old_cb_data = NULL; gpointer old_cb_data = NULL;
GSourceCallbackFuncs *old_cb_funcs = NULL; GSourceCallbackFuncs *old_cb_funcs = NULL;
int old_ref;
g_return_if_fail (source != NULL); g_return_if_fail (source != NULL);
old_ref = g_atomic_int_get (&source->ref_count);
retry_beginning:
if (old_ref > 1)
{
/* We have many references. If we can decrement the ref counter, we are done. */
if (!g_atomic_int_compare_and_exchange_full ((int *) &source->ref_count,
old_ref, old_ref - 1,
&old_ref))
goto retry_beginning;
return;
}
g_return_if_fail (old_ref > 0);
if (!have_lock && context) if (!have_lock && context)
LOCK_CONTEXT (context); LOCK_CONTEXT (context);
if (g_atomic_int_dec_and_test (&source->ref_count)) /* We are about to drop the last reference, there's not guarantee at this
* point that another thread already changed the value at this point or
* that is also entering the disposal phase, but there is no much we can do
* and dropping the reference too early would be still risky since it could
* lead to a preventive finalization.
* So let's just get all the threads that reached this point to get in, while
* the final check on whether is the case or not to continue with the
* finalization will be done by a final unique atomic dec and test.
*/
if (old_ref == 1)
{ {
/* If there's a dispose function, call this first */ /* If there's a dispose function, call this first */
if (source->priv->dispose) if (source->priv->dispose)
{ {
/* Temporarily increase the ref count again so that GSource methods
* can be called from dispose(). */
g_atomic_int_inc (&source->ref_count);
if (context) if (context)
UNLOCK_CONTEXT (context); UNLOCK_CONTEXT (context);
source->priv->dispose (source); source->priv->dispose (source);
if (context) if (context)
LOCK_CONTEXT (context); LOCK_CONTEXT (context);
}
/* Now the reference count might be bigger than 0 again, in which /* At this point the source can have been revived by any of the threads
* case we simply return from here before freeing the source */ * acting on it or it's really ready for being finalized.
if (!g_atomic_int_dec_and_test (&source->ref_count)) */
if (!g_atomic_int_compare_and_exchange_full ((int *) &source->ref_count,
1, 0, &old_ref))
{ {
if (!have_lock && context) if (!have_lock && context)
UNLOCK_CONTEXT (context); UNLOCK_CONTEXT (context);
return;
} goto retry_beginning;
} }
TRACE (GLIB_SOURCE_BEFORE_FREE (source, context, TRACE (GLIB_SOURCE_BEFORE_FREE (source, context,
@ -2334,7 +2361,7 @@ void
g_source_unref (GSource *source) g_source_unref (GSource *source)
{ {
g_return_if_fail (source != NULL); g_return_if_fail (source != NULL);
g_return_if_fail (g_atomic_int_get (&source->ref_count) > 0); /* refcount is checked inside g_source_unref_internal() */
g_source_unref_internal (source, source->context, FALSE); g_source_unref_internal (source, source->context, FALSE);
} }