[glib: 6/7] tests: Add D-Bus object/subtree unregistration tests

[Philip Withnall <pwithnall src gnome org>]

commit 34ce204fd758e2ce0ab6bf152051534f46cdb336
Author: Philip Withnall <pwithnall endlessos org>
Date:   Fri Sep 24 10:57:20 2021 +0100

    tests: Add D-Bus object/subtree unregistration tests
    
    These tests cover the fixes from the previous two commits.
    
    Signed-off-by: Philip Withnall <pwithnall endlessos org>
    
    Helps: #2400

 gio/tests/gdbus-export.c | 180 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 180 insertions(+)
---
diff --git a/gio/tests/gdbus-export.c b/gio/tests/gdbus-export.c
index aec21d7d0..4cdc24492 100644
--- a/gio/tests/gdbus-export.c
+++ b/gio/tests/gdbus-export.c
@@ -1792,6 +1792,184 @@ test_async_properties (void)
   g_object_unref (c);
 }
 
+typedef struct
+{
+  GDBusConnection *connection;  /* (owned) */
+  guint registration_id;
+  guint subtree_registration_id;
+} ThreadedUnregistrationData;
+
+static gpointer
+unregister_thread_cb (gpointer user_data)
+{
+  ThreadedUnregistrationData *data = user_data;
+
+  /* Sleeping here makes the race more likely to be hit, as it balances the
+   * time taken to set up the thread and unregister, with the time taken to
+   * make and handle the D-Bus call. This will likely change with future kernel
+   * versions, but there isn’t a more deterministic synchronisation point that
+   * I can think of to use instead. */
+  usleep (330);
+
+  if (data->registration_id > 0)
+    g_assert_true (g_dbus_connection_unregister_object (data->connection, data->registration_id));
+
+  if (data->subtree_registration_id > 0)
+    g_assert_true (g_dbus_connection_unregister_subtree (data->connection, data->subtree_registration_id));
+
+  return NULL;
+}
+
+static void
+async_result_cb (GObject      *source_object,
+                 GAsyncResult *result,
+                 gpointer      user_data)
+{
+  GAsyncResult **result_out = user_data;
+
+  *result_out = g_object_ref (result);
+  g_main_context_wakeup (NULL);
+}
+
+/* Returns %TRUE if this iteration resolved the race with the unregistration
+ * first, %FALSE if the call handler was invoked first. */
+static gboolean
+test_threaded_unregistration_iteration (gboolean subtree)
+{
+  ThreadedUnregistrationData data = { NULL, 0, 0 };
+  ObjectRegistrationData object_registration_data = { 0, 0, 2 };
+  GError *local_error = NULL;
+  GThread *unregister_thread = NULL;
+  const gchar *object_path;
+  GVariant *value = NULL;
+  const gchar *value_str;
+  GAsyncResult *call_result = NULL;
+  gboolean unregistration_was_first;
+
+  data.connection = g_bus_get_sync (G_BUS_TYPE_SESSION, NULL, &local_error);
+  g_assert_no_error (local_error);
+  g_assert_nonnull (data.connection);
+
+  /* Register an object or a subtree */
+  if (!subtree)
+    {
+      data.registration_id = g_dbus_connection_register_object (data.connection,
+                                                                "/foo/boss",
+                                                                (GDBusInterfaceInfo *) &foo_interface_info,
+                                                                &foo_vtable,
+                                                                &object_registration_data,
+                                                                on_object_unregistered,
+                                                                &local_error);
+      g_assert_no_error (local_error);
+      g_assert_cmpint (data.registration_id, >, 0);
+
+      object_path = "/foo/boss";
+    }
+  else
+    {
+      data.subtree_registration_id = g_dbus_connection_register_subtree (data.connection,
+                                                                         "/foo/boss/executives",
+                                                                         &subtree_vtable,
+                                                                         G_DBUS_SUBTREE_FLAGS_NONE,
+                                                                         &object_registration_data,
+                                                                         on_subtree_unregistered,
+                                                                         &local_error);
+      g_assert_no_error (local_error);
+      g_assert_cmpint (data.subtree_registration_id, >, 0);
+
+      object_path = "/foo/boss/executives/vp0";
+    }
+
+  /* Allow the registrations to go through. */
+  g_main_context_iteration (NULL, FALSE);
+
+  /* Spawn a thread to unregister the object/subtree. This will race with
+   * the call we subsequently make. */
+  unregister_thread = g_thread_new ("unregister-object",
+                                    unregister_thread_cb, &data);
+
+  /* Call a method on the object (or an object in the subtree). The callback
+   * will be invoked in this main context. */
+  g_dbus_connection_call (data.connection,
+                          g_dbus_connection_get_unique_name (data.connection),
+                          object_path,
+                          "org.example.Foo",
+                          "Method1",
+                          g_variant_new ("(s)", "winwinwin"),
+                          NULL,
+                          G_DBUS_CALL_FLAGS_NONE,
+                          -1,
+                          NULL,
+                          async_result_cb,
+                          &call_result);
+
+  while (call_result == NULL)
+    g_main_context_iteration (NULL, TRUE);
+
+  value = g_dbus_connection_call_finish (data.connection, call_result, &local_error);
+
+  /* The result of the method could either be an error (that the object doesn’t
+   * exist) or a valid result, depending on how the thread was scheduled
+   * relative to the call. */
+  unregistration_was_first = (value == NULL);
+  if (value != NULL)
+    {
+      g_assert_no_error (local_error);
+      g_assert_true (g_variant_is_of_type (value, G_VARIANT_TYPE ("(s)")));
+      g_variant_get (value, "(&s)", &value_str);
+      g_assert_cmpstr (value_str, ==, "You passed the string 'winwinwin'. Jolly good!");
+    }
+  else
+    {
+      g_assert_null (value);
+      g_assert_error (local_error, G_DBUS_ERROR, G_DBUS_ERROR_UNKNOWN_METHOD);
+    }
+
+  g_clear_pointer (&value, g_variant_unref);
+  g_clear_error (&local_error);
+
+  /* Tidy up. */
+  g_thread_join (g_steal_pointer (&unregister_thread));
+
+  g_clear_object (&call_result);
+  g_clear_object (&data.connection);
+
+  return unregistration_was_first;
+}
+
+static void
+test_threaded_unregistration (gconstpointer test_data)
+{
+  gboolean subtree = GPOINTER_TO_INT (test_data);
+  guint i;
+  guint n_iterations_unregistration_first = 0;
+  guint n_iterations_call_first = 0;
+
+  g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/2400";);
+  g_test_summary ("Test that object/subtree unregistration from one thread "
+                  "doesn’t cause problems when racing with method callbacks "
+                  "in another thread for that object or subtree");
+
+  /* Run iterations of the test until it’s likely we’ve hit the race. Limit the
+   * number of iterations so the test doesn’t run forever if not. The choice of
+   * 100 is arbitrary. */
+  for (i = 0; i < 1000 && (n_iterations_unregistration_first < 100 || n_iterations_call_first < 100); i++)
+    {
+      if (test_threaded_unregistration_iteration (subtree))
+        n_iterations_unregistration_first++;
+      else
+        n_iterations_call_first++;
+    }
+
+  /* If the condition below is met, we probably failed to reproduce the race.
+   * Don’t fail the test, though, as we can’t always control whether we hit the
+   * race, and spurious test failures are annoying. */
+  if (n_iterations_unregistration_first < 100 ||
+      n_iterations_call_first < 100)
+    g_test_skip_printf ("Failed to reproduce race (%u iterations with unregistration first, %u with call 
first); skipping test",
+                        n_iterations_unregistration_first, n_iterations_call_first);
+}
+
 /* ---------------------------------------------------------------------------------------------------- */
 
 int
@@ -1809,6 +1987,8 @@ main (int   argc,
   g_test_add_func ("/gdbus/object-registration-with-closures", test_object_registration_with_closures);
   g_test_add_func ("/gdbus/registered-interfaces", test_registered_interfaces);
   g_test_add_func ("/gdbus/async-properties", test_async_properties);
+  g_test_add_data_func ("/gdbus/threaded-unregistration/object", GINT_TO_POINTER (FALSE), 
test_threaded_unregistration);
+  g_test_add_data_func ("/gdbus/threaded-unregistration/subtree", GINT_TO_POINTER (TRUE), 
test_threaded_unregistration);
 
   /* TODO: check that we spit out correct introspection data */
   /* TODO: check that registering a whole subtree works */