[glib: 3/6] Use GSource dispose function for safely disconnecting the GCancellable source cancelled signal handl



commit b3de0c10905d9275c832706e862d3047c732ec0b
Author: Sebastian Dröge <sebastian centricular com>
Date:   Sun Oct 20 11:39:37 2019 +0300

    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.

 gio/gcancellable.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)
---
diff --git a/gio/gcancellable.c b/gio/gcancellable.c
index 72520a333..d9e58b8e8 100644
--- a/gio/gcancellable.c
+++ b/gio/gcancellable.c
@@ -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)


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]