[libdazzle] cancellable: use weak reference for cancellable chain



commit 2dbe048a4c4ff6c68528c0780741f6573272020c
Author: Christian Hergert <chergert redhat com>
Date:   Sun Dec 17 20:18:12 2017 -0800

    cancellable: use weak reference for cancellable chain
    
    This ensures we don't create complex reference chains by using
    a GWeakRef for the given cancellables.

 src/util/dzl-cancellable.c |   71 +++++++++++++++++++++++++++++++++++++------
 tests/test-cancellable.c   |   24 ++++++++++++++-
 2 files changed, 84 insertions(+), 11 deletions(-)
---
diff --git a/src/util/dzl-cancellable.c b/src/util/dzl-cancellable.c
index a84c340..1b9460d 100644
--- a/src/util/dzl-cancellable.c
+++ b/src/util/dzl-cancellable.c
@@ -21,16 +21,56 @@
 #include <gio/gio.h>
 
 #include "util/dzl-cancellable.h"
+#include "util/dzl-macros.h"
+
+typedef struct
+{
+  GWeakRef self;
+  GWeakRef other;
+  gulong   other_handler;
+} ChainedInfo;
+
+static void
+chained_info_free (gpointer data)
+{
+  ChainedInfo *info = data;
+  g_autoptr(GCancellable) self = NULL;
+  g_autoptr(GCancellable) other = NULL;
+
+  g_assert (info != NULL);
+
+  self = g_weak_ref_get (&info->self);
+  other = g_weak_ref_get (&info->other);
+
+  if (other != NULL && info->other_handler != 0)
+    dzl_clear_signal_handler (other, &info->other_handler);
+  else
+    info->other_handler = 0;
+
+  g_weak_ref_clear (&info->other);
+  g_weak_ref_clear (&info->self);
+
+  g_slice_free (ChainedInfo, info);
+}
 
 static void
-dzl_cancellable_cancelled_cb (GCancellable *dep,
-                              GCancellable *parent)
+dzl_cancellable_cancelled_cb (GCancellable *other,
+                              ChainedInfo  *info)
 {
-  g_assert (G_IS_CANCELLABLE (dep));
-  g_assert (G_IS_CANCELLABLE (parent));
+  g_autoptr(GCancellable) self = NULL;
+
+  g_assert (G_IS_CANCELLABLE (other));
+  g_assert (info != NULL);
 
-  if (!g_cancellable_is_cancelled (parent))
-    g_cancellable_cancel (parent);
+  self = g_weak_ref_get (&info->self);
+
+  if (self != NULL)
+    {
+      if (!g_cancellable_is_cancelled (self))
+        g_cancellable_cancel (self);
+    }
+
+  dzl_clear_signal_handler (other, &info->other_handler);
 }
 
 /**
@@ -47,14 +87,25 @@ void
 dzl_cancellable_chain (GCancellable *self,
                        GCancellable *other)
 {
+  ChainedInfo *info;
+
   g_return_if_fail (!self || G_IS_CANCELLABLE (self));
   g_return_if_fail (!other || G_IS_CANCELLABLE (other));
 
   if (self == NULL || other == NULL)
     return;
 
-  g_cancellable_connect (other,
-                         G_CALLBACK (dzl_cancellable_cancelled_cb),
-                         g_object_ref (self),
-                         g_object_unref);
+  /*
+   * We very much want to avoid taking a reference in the process
+   * here because that makes it difficult to know if we've created
+   * any sort of reference cycles or cancellable leaks.
+   */
+
+  info = g_slice_new0 (ChainedInfo);
+  g_weak_ref_init (&info->self, self);
+  g_weak_ref_init (&info->other, other);
+  info->other_handler = g_cancellable_connect (other,
+                                               G_CALLBACK (dzl_cancellable_cancelled_cb),
+                                               info,
+                                               chained_info_free);
 }
diff --git a/tests/test-cancellable.c b/tests/test-cancellable.c
index 020a3c7..879f00c 100644
--- a/tests/test-cancellable.c
+++ b/tests/test-cancellable.c
@@ -25,11 +25,33 @@ test_basic (void)
   g_assert_cmpint (FALSE, ==, g_cancellable_is_cancelled (b));
 }
 
+static void
+test_weak (void)
+{
+  g_autoptr(GCancellable) root = g_cancellable_new ();
+  g_autoptr(GCancellable) a = g_cancellable_new ();
+  g_autoptr(GCancellable) b = g_cancellable_new ();
+  g_autoptr(GCancellable) a1 = g_cancellable_new ();
+  g_autoptr(GCancellable) a2 = g_cancellable_new ();
+
+  dzl_cancellable_chain (root, a);
+  dzl_cancellable_chain (root, b);
+  dzl_cancellable_chain (a, a1);
+  dzl_cancellable_chain (a, a2);
+
+  g_clear_object (&root);
+  g_clear_object (&a);
+  g_clear_object (&b);
+  g_clear_object (&a1);
+  g_clear_object (&a2);
+}
+
 gint
 main (gint   argc,
       gchar *argv[])
 {
   g_test_init (&argc, &argv, NULL);
-  g_test_add_func ("/Dazzle/Cancellable", test_basic);
+  g_test_add_func ("/Dazzle/Cancellable/basic", test_basic);
+  g_test_add_func ("/Dazzle/Cancellable/weak", test_weak);
   return g_test_run ();
 }


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