mirror of
https://gitlab.gnome.org/GNOME/glib.git
synced 2025-02-23 10:42:11 +01:00
glib/gmain: Avoid potential dispose race on GSource due to refcount dance
During g_source_unref() we might end up calling dispose from multiple threads, and due to the ref/unref dance we were doing we could end up initiating a GSource finalization while another thread was about to revive the source. This was because we were unreffing a source in a thread, and potentially re-reffing it, at the same time, but it was not guaranteed that the final decrement and test couldn't be followed by a further re-ref. To avoid this, do not do any ref/unref/re-ref/re-unref dance while we're about to dispose, but instead follow a bit more the g_object_unref() logic, and start the disposal phase only if we're about to drop the last reference, and only after the potential disposal call is done, we do an actual ref count decrement, which may lead to the finalization or not. We don't bother following the same logic at later point, since after disposal we should really have just one thread running and revival of a finalizing GSource isn't supported anyways. With this logic we can also avoid doing unneeded context locking when we've enough references on a GSource that disposal is unlikely to happen. Closes: #3612
This commit is contained in:
parent
67a2f8c593
commit
7b61cc8277
37
glib/gmain.c
37
glib/gmain.c
@ -2210,35 +2210,58 @@ g_source_unref_internal (GSource *source,
|
||||
{
|
||||
gpointer old_cb_data = NULL;
|
||||
GSourceCallbackFuncs *old_cb_funcs = NULL;
|
||||
int old_ref;
|
||||
|
||||
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;
|
||||
}
|
||||
|
||||
if (!have_lock && 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 (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)
|
||||
UNLOCK_CONTEXT (context);
|
||||
source->priv->dispose (source);
|
||||
if (context)
|
||||
LOCK_CONTEXT (context);
|
||||
}
|
||||
|
||||
/* Now the reference count might be bigger than 0 again, in which
|
||||
* case we simply return from here before freeing the source */
|
||||
/* At this point the source can have been revived by any of the threads
|
||||
* acting on it or it's really ready for being finalized.
|
||||
*/
|
||||
if (!g_atomic_int_dec_and_test (&source->ref_count))
|
||||
{
|
||||
if (!have_lock && context)
|
||||
UNLOCK_CONTEXT (context);
|
||||
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
TRACE (GLIB_SOURCE_BEFORE_FREE (source, context,
|
||||
source->source_funcs->finalize));
|
||||
|
Loading…
x
Reference in New Issue
Block a user