gcancellable: Ignore cancelled callback on disposed source

Prevent to access to a disposed gsource in the GCancellable cancelled
signal callback.

Due to the fact that the signal is called in the same thread in which
the cancellable get cancelled, we may receive the callback just after
the GSource has been disposed or during its disposal.

To prevent this, let's pass to the signal a pointer to the source itself
and nullify atomically the first time we're calling a callback or
disposing it.

In case the dispose function wins the race, then the callback will just
do nothing, while the disposal will continue as expected.

In case the callback wins the race, during the concurrent disposal we'll
just wait for the callback to be completed, before returning the disposal
itself that will likely lead to freeing the GSource.

Closes: #3448
This commit is contained in:
Marco Trevisan (Treviño) 2024-08-23 16:37:48 -04:00 committed by Philip Withnall
parent eec7ba6ef3
commit d7c8bb0726

View File

@ -640,6 +640,8 @@ g_cancellable_disconnect (GCancellable *cancellable,
typedef struct {
GSource source;
/* Atomic: */
GSource **self_ptr;
/* Atomic: */
GCancellable *cancellable;
gulong cancelled_handler;
@ -662,10 +664,16 @@ static void
cancellable_source_cancelled (GCancellable *cancellable,
gpointer user_data)
{
GSource *source = user_data;
GCancellableSource *cancellable_source = (GCancellableSource *) source;
GSource *source = g_atomic_pointer_exchange ((GSource **) user_data, NULL);
GCancellableSource *cancellable_source;
gboolean callback_was_not_called G_GNUC_UNUSED;
/* The source is being disposed, so don't bother marking it as ready */
if (source == NULL)
return;
cancellable_source = (GCancellableSource *) source;
g_source_ref (source);
g_source_set_ready_time (source, 0);
@ -688,7 +696,10 @@ cancellable_source_prepare (GSource *source,
cancellable = g_atomic_pointer_get (&cancellable_source->cancellable);
if (cancellable && !g_atomic_int_get (&cancellable->priv->cancelled_running))
g_atomic_int_set (&cancellable_source->cancelled_callback_called, FALSE);
{
g_atomic_int_set (&cancellable_source->cancelled_callback_called, FALSE);
g_atomic_pointer_set (cancellable_source->self_ptr, source);
}
return FALSE;
}
@ -715,7 +726,10 @@ cancellable_source_dispose (GSource *source)
if (cancellable)
{
if (g_atomic_int_get (&cancellable->priv->cancelled_running))
GSource *self_ptr =
g_atomic_pointer_exchange (cancellable_source->self_ptr, NULL);
if (self_ptr == NULL)
{
/* There can be a race here: if thread A has called
* g_cancellable_cancel() and has got as far as committing to call
@ -810,14 +824,17 @@ g_cancellable_source_new (GCancellable *cancellable)
if (cancellable)
{
cancellable_source->cancellable = g_object_ref (cancellable);
cancellable_source->self_ptr = g_new (GSource *, 1);
g_atomic_pointer_set (cancellable_source->self_ptr, source);
/* We intentionally don't use g_cancellable_connect() here,
* because we don't want the "at most once" behavior.
*/
cancellable_source->cancelled_handler =
g_signal_connect (cancellable, "cancelled",
G_CALLBACK (cancellable_source_cancelled),
source);
g_signal_connect_data (cancellable, "cancelled",
G_CALLBACK (cancellable_source_cancelled),
cancellable_source->self_ptr,
(GClosureNotify) g_free, G_CONNECT_DEFAULT);
if (g_cancellable_is_cancelled (cancellable))
g_source_set_ready_time (source, 0);
}