From a1e32f0a40dbb4a83b427d9a3046e0047f90984c Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Tue, 23 Apr 2024 21:39:43 +0100
Subject: [PATCH] tests: Ensure that unsubscribing with GetNameOwner in-flight
 doesn't crash

This was a bug that existed during development of this branch; make sure
it doesn't come back.

This test fails with a use-after-free and crash if we comment out the
part of name_watcher_unref_watched_name() that removes the name watcher
from `map_method_serial_to_name_watcher`.

It would also fail with an assertion failure if we asserted in
name_watcher_unref_watched_name() that get_name_owner_serial == 0
(i.e. that GetNameOwner is not in-flight at destruction).

Signed-off-by: Simon McVittie <smcv@collabora.com>
---
 gio/tests/gdbus-subscribe.c | 52 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/gio/tests/gdbus-subscribe.c b/gio/tests/gdbus-subscribe.c
index 5406ba7e2..4cba4f565 100644
--- a/gio/tests/gdbus-subscribe.c
+++ b/gio/tests/gdbus-subscribe.c
@@ -116,6 +116,7 @@ typedef struct
   const char *member;
   const char *arg0;
   GDBusSignalFlags flags;
+  gboolean unsubscribe_immediately;
 } TestSubscribe;
 
 typedef struct
@@ -141,6 +142,7 @@ typedef struct
     TestEmitSignal signal;
     TestSubscribe subscribe;
     TestOwnName own_name;
+    guint unsubscribe_undo_step;
   } u;
 } TestStep;
 
@@ -505,6 +507,43 @@ static const TestPlan plan_limit_by_well_known_name =
   },
 };
 
+static const TestPlan plan_unsubscribe_immediately =
+{
+  .description = "Unsubscribing before GetNameOwner can return doesn't result in a crash",
+  .steps = {
+    {
+      /* Service already owns one name */
+      .action = TEST_ACTION_OWN_NAME,
+      .u.own_name = {
+        .name = ALREADY_OWNED_NAME,
+        .owner = TEST_CONN_SERVICE
+      },
+    },
+    {
+      .action = TEST_ACTION_SUBSCRIBE,
+      .u.subscribe = {
+        .string_sender = ALREADY_OWNED_NAME,
+        .path = EXAMPLE_PATH,
+        .iface = EXAMPLE_INTERFACE,
+        .unsubscribe_immediately = TRUE
+      },
+    },
+    {
+      .action = TEST_ACTION_EMIT_SIGNAL,
+      .u.signal = {
+        .sender = TEST_CONN_SERVICE,
+        .path = EXAMPLE_PATH,
+        .iface = EXAMPLE_INTERFACE,
+        .member = FOO_SIGNAL,
+        .received_by_conn = 0,
+        /* The proxy can't unsubscribe, except by destroying the proxy
+         * completely, which we don't currently implement in this test */
+        .received_by_proxy = 1
+      },
+    },
+  },
+};
+
 static const TestPlan plan_limit_to_message_bus =
 {
   .description = "A subscription to the message bus only accepts messages "
@@ -855,8 +894,18 @@ fixture_subscribe (Fixture             *f,
                                                subscribe->flags,
                                                subscribed_signal_cb,
                                                f, NULL);
+
       g_assert_cmpuint (id, !=, 0);
-      f->subscriptions[step_number] = id;
+
+      if (subscribe->unsubscribe_immediately)
+        {
+          g_test_message ("\tImmediately unsubscribing");
+          g_dbus_connection_signal_unsubscribe (subscriber, id);
+        }
+      else
+        {
+          f->subscriptions[step_number] = id;
+        }
     }
 
   if (f->mode != SUBSCRIPTION_MODE_CONN)
@@ -1287,6 +1336,7 @@ main (int   argc,
   ADD_SUBSCRIBE_TEST (nonexistent_unique_name);
   ADD_SUBSCRIBE_TEST (limit_by_well_known_name);
   ADD_SUBSCRIBE_TEST (limit_to_message_bus);
+  ADD_SUBSCRIBE_TEST (unsubscribe_immediately);
 
   return g_test_run();
 }