Use GSource dispose function for safely disconnecting the GCancellable source cancelled signal handler

If not doing this it might happen that the cancelled signal is emitted
between reaching a reference count of 0 and finalizing the GSource, at
which point part of the GSource is already freed and calling any GSource
functions is dangerous.

Instead do this from the dispose function. At this time the GSource is
not partially freed yet and calling any GSource API is safe as long as
we ensure that we have a strong reference to the GSource before calling
any GSource API.
This commit is contained in:
Sebastian Dröge 2019-10-20 11:39:37 +03:00
parent be1ec9004b
commit b3de0c1090

View File

@ -642,20 +642,19 @@ typedef struct {
GSource source;
GCancellable *cancellable;
guint cancelled_handler;
gulong cancelled_handler;
} GCancellableSource;
/*
* We can't guarantee that the source still has references, so we are
* relying on the fact that g_source_set_ready_time() no longer makes
* assertions about the reference count - the source might be in the
* window between last-unref and finalize, during which its refcount
* is officially 0. However, we *can* guarantee that it's OK to
* dereference it in a limited way, because we know we haven't yet reached
* cancellable_source_finalize() - if we had, then we would have waited
* for signal emission to finish, then disconnected the signal handler
* under the lock.
* See https://bugzilla.gnome.org/show_bug.cgi?id=791754
* The reference count of the GSource might be 0 at this point but it is not
* finalized yet and its dispose function did not run yet, or otherwise we
* would have disconnected the signal handler already and due to the signal
* emission lock it would be impossible to call the signal handler at that
* point. That is: at this point we either have a fully valid GSource, or
* it's not disposed or finalized yet and we can still resurrect it as needed.
*
* As such we first ensure that we have a strong reference to the GSource in
* here before calling any other GSource API.
*/
static void
cancellable_source_cancelled (GCancellable *cancellable,
@ -663,7 +662,9 @@ cancellable_source_cancelled (GCancellable *cancellable,
{
GSource *source = user_data;
g_source_ref (source);
g_source_set_ready_time (source, 0);
g_source_unref (source);
}
static gboolean
@ -679,15 +680,15 @@ cancellable_source_dispatch (GSource *source,
}
static void
cancellable_source_finalize (GSource *source)
cancellable_source_dispose (GSource *source)
{
GCancellableSource *cancellable_source = (GCancellableSource *)source;
if (cancellable_source->cancellable)
{
g_cancellable_disconnect (cancellable_source->cancellable,
cancellable_source->cancelled_handler);
g_object_unref (cancellable_source->cancellable);
g_clear_signal_handler (&cancellable_source->cancelled_handler,
cancellable_source->cancellable);
g_clear_object (&cancellable_source->cancellable);
}
}
@ -720,7 +721,7 @@ static GSourceFuncs cancellable_source_funcs =
NULL,
NULL,
cancellable_source_dispatch,
cancellable_source_finalize,
NULL,
(GSourceFunc)cancellable_source_closure_callback,
};
@ -750,6 +751,7 @@ g_cancellable_source_new (GCancellable *cancellable)
source = g_source_new (&cancellable_source_funcs, sizeof (GCancellableSource));
g_source_set_name (source, "GCancellable");
g_source_set_dispose_function (source, cancellable_source_dispose);
cancellable_source = (GCancellableSource *)source;
if (cancellable)