[glib: 5/7] gdebugcontrollerdbus: Add stop() method

[Philip Withnall <pwithnall src gnome org>]

commit 1b3e6bab533537eb7129986f72788ac11b23ae0b
Author: Philip Withnall <pwithnall endlessos org>
Date:   Fri Feb 11 11:36:28 2022 +0000

    gdebugcontrollerdbus: Add stop() method
    
    This allows the controller to explicitly be removed from the bus, in a
    way that allows the caller to synchronise with it and know that all
    other references to the controller should have been dropped (i.e. after
    this method returns, there should be no in-flight D-Bus calls still
    holding a reference to the object).
    
    This is needed to be able to guarantee finalisation of the controller in
    unit tests (and comparable real-world situations).
    
    Signed-off-by: Philip Withnall <pwithnall endlessos org>
    
    Helps: #1190

 docs/reference/gio/gio-sections-common.txt |  1 +
 gio/gdebugcontrollerdbus.c                 | 92 +++++++++++++++++++++++++-----
 gio/gdebugcontrollerdbus.h                 |  3 +
 3 files changed, 83 insertions(+), 13 deletions(-)
---
diff --git a/docs/reference/gio/gio-sections-common.txt b/docs/reference/gio/gio-sections-common.txt
index 0c73afa6f..4e88a597e 100644
--- a/docs/reference/gio/gio-sections-common.txt
+++ b/docs/reference/gio/gio-sections-common.txt
@@ -4230,6 +4230,7 @@ G_DEBUG_CONTROLLER_GET_INTERFACE
 <TITLE>GDebugControllerDBus</TITLE>
 GDebugControllerDBus
 g_debug_controller_dbus_new
+g_debug_controller_dbus_stop
 <SUBSECTION Standard>
 g_debug_controller_dbus_get_type
 G_TYPE_DEBUG_CONTROLLER_DBUS
diff --git a/gio/gdebugcontrollerdbus.c b/gio/gdebugcontrollerdbus.c
index e64390654..e3dcdbde2 100644
--- a/gio/gdebugcontrollerdbus.c
+++ b/gio/gdebugcontrollerdbus.c
@@ -183,8 +183,10 @@ typedef struct
 {
   GObject parent_instance;
 
+  GCancellable *cancellable;  /* (owned) */
   GDBusConnection *connection;  /* (owned) */
   guint object_id;
+  guint n_pending_authorize_tasks;
 
   gboolean debug_enabled;
 } GDebugControllerDBusPrivate;
@@ -202,8 +204,11 @@ G_DEFINE_TYPE_WITH_CODE (GDebugControllerDBus, g_debug_controller_dbus, G_TYPE_O
                                                          30))
 
 static void
-g_debug_controller_dbus_init (GDebugControllerDBus *dbus)
+g_debug_controller_dbus_init (GDebugControllerDBus *self)
 {
+  GDebugControllerDBusPrivate *priv = g_debug_controller_dbus_get_instance_private (self);
+
+  priv->cancellable = g_cancellable_new ();
 }
 
 static void
@@ -212,6 +217,9 @@ set_debug_enabled (GDebugControllerDBus *self,
 {
   GDebugControllerDBusPrivate *priv = g_debug_controller_dbus_get_instance_private (self);
 
+  if (g_cancellable_is_cancelled (priv->cancellable))
+    return;
+
   if (debug_enabled != priv->debug_enabled)
     {
       GVariantBuilder builder;
@@ -288,6 +296,7 @@ authorize_cb (GObject      *object,
               gpointer      user_data)
 {
   GDebugControllerDBus *self = G_DEBUG_CONTROLLER_DBUS (object);
+  GDebugControllerDBusPrivate *priv = g_debug_controller_dbus_get_instance_private (self);
   GTask *task = G_TASK (result);
   GDBusMethodInvocation *invocation = g_task_get_task_data (task);
   GVariant *parameters = g_dbus_method_invocation_get_parameters (invocation);
@@ -301,14 +310,18 @@ authorize_cb (GObject      *object,
       GError *local_error = g_error_new (G_DBUS_ERROR, G_DBUS_ERROR_ACCESS_DENIED,
                                          _("Not authorized to change debug settings"));
       g_dbus_method_invocation_take_error (invocation, g_steal_pointer (&local_error));
-      return;
     }
+  else
+    {
+      /* Update the property value. */
+      g_variant_get (parameters, "(b)", &enabled);
+      set_debug_enabled (self, enabled);
 
-  /* Update the property value. */
-  g_variant_get (parameters, "(b)", &enabled);
-  set_debug_enabled (self, enabled);
+      g_dbus_method_invocation_return_value (invocation, NULL);
+    }
 
-  g_dbus_method_invocation_return_value (invocation, NULL);
+  g_assert (priv->n_pending_authorize_tasks > 0);
+  priv->n_pending_authorize_tasks--;
 }
 
 /* Called in the #GMainContext which was default when the #GDebugControllerDBus
@@ -324,6 +337,7 @@ dbus_method_call (GDBusConnection       *connection,
                   gpointer               user_data)
 {
   GDebugControllerDBus *self = user_data;
+  GDebugControllerDBusPrivate *priv = g_debug_controller_dbus_get_instance_private (self);
   GDebugControllerDBusClass *klass = G_DEBUG_CONTROLLER_DBUS_GET_CLASS (self);
 
   /* Only on the org.gtk.Debugging interface */
@@ -331,10 +345,13 @@ dbus_method_call (GDBusConnection       *connection,
     {
       GTask *task = NULL;
 
-      task = g_task_new (self, NULL, authorize_cb, NULL);
+      task = g_task_new (self, priv->cancellable, authorize_cb, NULL);
       g_task_set_source_tag (task, dbus_method_call);
       g_task_set_task_data (task, g_object_ref (invocation), (GDestroyNotify) g_object_unref);
 
+      g_assert (priv->n_pending_authorize_tasks < G_MAXUINT);
+      priv->n_pending_authorize_tasks++;
+
       /* Check the calling peer is authorised to change the debug mode. So that
        * the signal handler can block on checking polkit authorisation (which
        * definitely involves D-Bus calls, and might involve user interaction),
@@ -448,13 +465,10 @@ g_debug_controller_dbus_dispose (GObject *object)
   GDebugControllerDBus *self = G_DEBUG_CONTROLLER_DBUS (object);
   GDebugControllerDBusPrivate *priv = g_debug_controller_dbus_get_instance_private (self);
 
-  if (priv->object_id != 0)
-    {
-      g_dbus_connection_unregister_object (priv->connection, priv->object_id);
-      priv->object_id = 0;
-    }
-
+  g_debug_controller_dbus_stop (self);
+  g_assert (priv->n_pending_authorize_tasks == 0);
   g_clear_object (&priv->connection);
+  g_clear_object (&priv->cancellable);
 
   G_OBJECT_CLASS (g_debug_controller_dbus_parent_class)->dispose (object);
 }
@@ -587,3 +601,55 @@ g_debug_controller_dbus_new (GDBusConnection  *connection,
                          "connection", connection,
                          NULL);
 }
+
+/**
+ * g_debug_controller_dbus_stop:
+ * @self: a #GDebugControllerDBus
+ *
+ * Stop the debug controller, unregistering its object from the bus.
+ *
+ * Any pending method calls to the object will complete successfully, but new
+ * ones will return an error. This method will block until all pending
+ * #GDebugControllerDBus::authorize signals have been handled. This is expected
+ * to not take long, as it will just be waiting for threads to join. If any
+ * #GDebugControllerDBus::authorize signal handlers are still executing in other
+ * threads, this will block until after they have returned.
+ *
+ * This method will be called automatically when the final reference to the
+ * #GDebugControllerDBus is dropped. You may want to call it explicitly to know
+ * when the controller has been fully removed from the bus, or to break
+ * reference count cycles.
+ *
+ * Calling this method from within a #GDebugControllerDBus::authorize signal
+ * handler will cause a deadlock and must not be done.
+ *
+ * Since: 2.72
+ */
+void
+g_debug_controller_dbus_stop (GDebugControllerDBus *self)
+{
+  GDebugControllerDBusPrivate *priv = g_debug_controller_dbus_get_instance_private (self);
+
+  g_cancellable_cancel (priv->cancellable);
+
+  if (priv->object_id != 0)
+    {
+      g_dbus_connection_unregister_object (priv->connection, priv->object_id);
+      priv->object_id = 0;
+    }
+
+  /* Wait for any pending authorize tasks to finish. These will just be waiting
+   * for threads to join at this point, as the D-Bus object has been
+   * unregistered and the cancellable cancelled.
+   *
+   * FIXME: There is still a narrow race here where (we think) the worker thread
+   * briefly holds a reference to the #GTask after the task thread function has
+   * returned.
+   *
+   * The loop will also never terminate if g_debug_controller_dbus_stop() is
+   * called from within an ::authorize callback.
+   *
+   * See discussion in https://gitlab.gnome.org/GNOME/glib/-/merge_requests/2486 */
+  while (priv->n_pending_authorize_tasks > 0)
+    g_thread_yield ();
+}
diff --git a/gio/gdebugcontrollerdbus.h b/gio/gdebugcontrollerdbus.h
index 2d41c25cf..5e54bbfa1 100644
--- a/gio/gdebugcontrollerdbus.h
+++ b/gio/gdebugcontrollerdbus.h
@@ -61,6 +61,9 @@ GDebugControllerDBus *g_debug_controller_dbus_new (GDBusConnection  *connection,
                                                    GCancellable     *cancellable,
                                                    GError          **error);
 
+GLIB_AVAILABLE_IN_2_72
+void g_debug_controller_dbus_stop (GDebugControllerDBus *self);
+
 G_END_DECLS
 
 #endif /* __G_DEBUG_CONTROLLER_DBUS_H__ */