tests: don't create GTestDBus in tp_tests_dbus_daemon_dup_or_die

Doing weak-ref magic with the TpDBusDaemon returned by
tp_tests_dbus_daemon_dup_or_die() was causing some rather
hard-to-track-down test failures in the Logger tests.

The problem is that g_test_dbus_down() calls g_object_run_dispose()
on the GDBusConnection, after which it is unusable (the "worker"
has been destroyed, and many methods just crash). That's fine
if nothing was holding a reference. Unfortunately, various Telepathy
and Logger objects that behave like singletons hold a reference
to the GDBusConnection.

This meant that this life-cycle was OK:

    / tp_tests_dbus_daemon_dup_or_die()
    |  / tp_dbus_daemon_dup()
    |  |  [test-case 1]
    |  \ g_object_unref()
    |  / tp_dbus_daemon_dup()
    |  |  [test-case 2]
    |  \ g_object_unref()
    \ g_object_unref()

but this will often crash:

    / tp_tests_dbus_daemon_dup_or_die()
    |  [test-case 1]
    \ g_object_unref()

    / tp_tests_dbus_daemon_dup_or_die()
    |  [test-case 2]
    \ g_object_unref()

As a first step towards fixing that, let's implement "explicit is
better than implicit": always manage GTestDBus explicitly
(via either its own API or tp_tests_run_with_bus), and never
create it implicitly.

Reviewed-by: Xavier Claessens

16 files changed, 135 insertions, 40 deletions

diff --git a/tests/dbus/call-cancellation.c b/tests/dbus/call-cancellation.c
index 8b4ab3245..724c8a189 100644
--- a/tests/dbus/call-cancellation.c
+++ b/tests/dbus/call-cancellation.c
@@ -46,6 +46,8 @@ enum {
 };
 
 typedef struct {
+    GTestDBus *test_dbus;
+
     TpDBusDaemon *dbus_daemon;
     TpProxy *proxies[N_PROXIES];
     GObject *cd_service;
@@ -172,6 +174,10 @@ signal_cb (TpProxy *proxy,
 static void
 setup (void)
 {
+  g_test_dbus_unset ();
+  f->test_dbus = g_test_dbus_new (G_TEST_DBUS_NONE);
+  g_test_dbus_up (f->test_dbus);
+
   f->dbus_daemon = tp_tests_dbus_daemon_dup_or_die ();
 
   /* Any random object with an interface: what matters is that it can
@@ -210,6 +216,9 @@ teardown (void)
   tp_tests_assert_last_unref (&f->dbus_daemon);
 
   tp_tests_assert_last_unref (&f->private_dbus_daemon);
+
+  g_test_dbus_down (f->test_dbus);
+  tp_tests_assert_last_unref (&f->test_dbus);
 }
 
 static TpProxy *
diff --git a/tests/dbus/channel-introspect.c b/tests/dbus/channel-introspect.c
index 81a1b82c9..dfefb2497 100644
--- a/tests/dbus/channel-introspect.c
+++ b/tests/dbus/channel-introspect.c
@@ -119,9 +119,15 @@ main (int argc,
   GAsyncResult *prepare_result;
   GQuark group_features[] = { TP_CHANNEL_FEATURE_GROUP, 0 };
   const gchar * const empty[] = { NULL };
+  GTestDBus *test_dbus;
 
   tp_tests_abort_after (10);
   tp_debug_set_flags ("all");
+
+  g_test_dbus_unset ();
+  test_dbus = g_test_dbus_new (G_TEST_DBUS_NONE);
+  g_test_dbus_up (test_dbus);
+
   dbus = tp_tests_dbus_daemon_dup_or_die ();
 
   tp_tests_create_conn (TP_TESTS_TYPE_CONTACTS_CONNECTION, "me@example.com",
@@ -418,5 +424,8 @@ main (int argc,
   g_free (props_chan_path);
   g_free (props_group_chan_path);
 
+  g_test_dbus_down (test_dbus);
+  tp_tests_assert_last_unref (&test_dbus);
+
   return 0;
 }
diff --git a/tests/dbus/cli-group.c b/tests/dbus/cli-group.c
index cb7f299cf..6ecc4b978 100644
--- a/tests/dbus/cli-group.c
+++ b/tests/dbus/cli-group.c
@@ -401,10 +401,15 @@ main (int argc,
 {
   TpBaseConnection *service_conn_as_base;
   GError *error = NULL;
+  GTestDBus *test_dbus;
 
   tp_tests_abort_after (10);
   tp_debug_set_flags ("all");
 
+  g_test_dbus_unset ();
+  test_dbus = g_test_dbus_new (G_TEST_DBUS_NONE);
+  g_test_dbus_up (test_dbus);
+
   tp_tests_create_conn (TP_TESTS_TYPE_CONTACTS_CONNECTION, "me@example.com",
       TRUE, &service_conn_as_base, &conn);
   service_conn = TP_TESTS_SIMPLE_CONNECTION (service_conn_as_base);
@@ -438,5 +443,8 @@ main (int argc,
   service_conn_as_base = NULL;
   g_object_unref (service_conn);
 
+  g_test_dbus_down (test_dbus);
+  tp_tests_assert_last_unref (&test_dbus);
+
   return 0;
 }
diff --git a/tests/dbus/contacts-bug-19101.c b/tests/dbus/contacts-bug-19101.c
index 34637210c..1e43bc457 100644
--- a/tests/dbus/contacts-bug-19101.c
+++ b/tests/dbus/contacts-bug-19101.c
@@ -58,12 +58,17 @@ main (int argc,
   TpTestsContactsConnection *service_conn;
   TpBaseConnection *service_conn_as_base;
   TpConnection *client_conn;
+  GTestDBus *test_dbus;
 
   /* Setup */
 
   tp_tests_abort_after (10);
   tp_debug_set_flags ("all");
 
+  g_test_dbus_unset ();
+  test_dbus = g_test_dbus_new (G_TEST_DBUS_NONE);
+  g_test_dbus_up (test_dbus);
+
   tp_tests_create_conn (TP_TESTS_TYPE_BUG19101_CONNECTION, "me@example.com",
       TRUE, &service_conn_as_base, &client_conn);
   service_conn = TP_TESTS_CONTACTS_CONNECTION (service_conn_as_base);
@@ -80,5 +85,8 @@ main (int argc,
   service_conn_as_base = NULL;
   g_object_unref (service_conn);
 
+  g_test_dbus_down (test_dbus);
+  tp_tests_assert_last_unref (&test_dbus);
+
   return 0;
 }
diff --git a/tests/dbus/contacts-mixin.c b/tests/dbus/contacts-mixin.c
index 638a9c6af..57d9a1efd 100644
--- a/tests/dbus/contacts-mixin.c
+++ b/tests/dbus/contacts-mixin.c
@@ -153,12 +153,17 @@ main (int argc,
       "GON OUT BACKSON" };
   TpHandleRepoIface *service_repo;
   guint i;
+  GTestDBus *test_dbus;
 
   /* Setup */
 
   tp_tests_abort_after (10);
   tp_debug_set_flags ("all");
 
+  g_test_dbus_unset ();
+  test_dbus = g_test_dbus_new (G_TEST_DBUS_NONE);
+  g_test_dbus_up (test_dbus);
+
   tp_tests_create_conn (TP_TESTS_TYPE_CONTACTS_CONNECTION, "me@example.com",
       TRUE, &service_conn_as_base, &client_conn);
   service_conn = TP_TESTS_CONTACTS_CONNECTION (service_conn_as_base);
@@ -195,5 +200,8 @@ main (int argc,
   g_object_unref (service_conn);
   g_array_unref (handles);
 
+  g_test_dbus_down (test_dbus);
+  tp_tests_assert_last_unref (&test_dbus);
+
   return 0;
 }
diff --git a/tests/dbus/disconnection.c b/tests/dbus/disconnection.c
index 034da739f..890158c4a 100644
--- a/tests/dbus/disconnection.c
+++ b/tests/dbus/disconnection.c
@@ -39,6 +39,7 @@ enum {
 };
 
 typedef struct {
+    GTestDBus *test_dbus;
     TpDBusDaemon *dbus_daemon;
     TpProxy *proxies[N_PROXIES];
     GObject *cd_service;
@@ -132,6 +133,10 @@ set_freed (gpointer user_data)
 static void
 setup (void)
 {
+  g_test_dbus_unset ();
+  f->test_dbus = g_test_dbus_new (G_TEST_DBUS_NONE);
+  g_test_dbus_up (f->test_dbus);
+
   f->dbus_daemon = tp_tests_dbus_daemon_dup_or_die ();
 
   /* Any random object with an interface: what matters is that it can
@@ -170,6 +175,9 @@ teardown (void)
   tp_tests_assert_last_unref (&f->dbus_daemon);
 
   tp_tests_assert_last_unref (&f->private_dbus_daemon);
+
+  g_test_dbus_down (f->test_dbus);
+  tp_tests_assert_last_unref (&f->test_dbus);
 }
 
 static TpProxy *
diff --git a/tests/dbus/example-no-protocols.c b/tests/dbus/example-no-protocols.c
index d2b28da3a..b63919adf 100644
--- a/tests/dbus/example-no-protocols.c
+++ b/tests/dbus/example-no-protocols.c
@@ -100,6 +100,7 @@ main (int argc,
   gulong handler;
   GError *error = NULL;
   gboolean saw_exited;
+  GTestDBus *test_dbus;
 
   /* If we're running slowly (for instance in a parallel build)
    * we don't want the CM process in the background to time out and exit. */
@@ -109,6 +110,10 @@ main (int argc,
 
   tp_debug_set_flags ("all");
 
+  g_test_dbus_unset ();
+  test_dbus = g_test_dbus_new (G_TEST_DBUS_NONE);
+  g_test_dbus_up (test_dbus);
+
   mainloop = g_main_loop_new (NULL, FALSE);
 
   dbus_daemon = tp_tests_dbus_daemon_dup_or_die ();
@@ -166,5 +171,8 @@ main (int argc,
   g_object_unref (dbus_daemon);
   g_main_loop_unref (mainloop);
 
+  g_test_dbus_down (test_dbus);
+  tp_tests_assert_last_unref (&test_dbus);
+
   return 0;
 }
diff --git a/tests/dbus/finalized-in-invalidated-handler.c b/tests/dbus/finalized-in-invalidated-handler.c
index e380e120a..298ffd02e 100644
--- a/tests/dbus/finalized-in-invalidated-handler.c
+++ b/tests/dbus/finalized-in-invalidated-handler.c
@@ -79,11 +79,16 @@ main (int argc,
   GError *error = NULL;
   gchar *chan_path;
   TpHandle handle;
+  GTestDBus *test_dbus;
 
   tp_tests_abort_after (10);
   tp_debug_set_flags ("all");
   mainloop = g_main_loop_new (NULL, FALSE);
 
+  g_test_dbus_unset ();
+  test_dbus = g_test_dbus_new (G_TEST_DBUS_NONE);
+  g_test_dbus_up (test_dbus);
+
   tp_tests_create_conn (TP_TESTS_TYPE_CONTACTS_CONNECTION, "me@example.com",
       TRUE, &service_conn_as_base, &conn);
   service_conn = TP_TESTS_SIMPLE_CONNECTION (service_conn_as_base);
@@ -133,5 +138,8 @@ main (int argc,
   g_main_loop_unref (mainloop);
   g_free (chan_path);
 
+  g_test_dbus_down (test_dbus);
+  tp_tests_assert_last_unref (&test_dbus);
+
   return 0;
 }
diff --git a/tests/dbus/group-mixin.c b/tests/dbus/group-mixin.c
index a625b248d..2d00d7b8b 100644
--- a/tests/dbus/group-mixin.c
+++ b/tests/dbus/group-mixin.c
@@ -497,10 +497,15 @@ main (int argc,
   TpConnection *conn;
   GError *error = NULL;
   gchar *chan_path;
+  GTestDBus *test_dbus;
 
   tp_tests_abort_after (10);
   tp_debug_set_flags ("all");
 
+  g_test_dbus_unset ();
+  test_dbus = g_test_dbus_new (G_TEST_DBUS_NONE);
+  g_test_dbus_up (test_dbus);
+
   tp_tests_create_conn (TP_TESTS_TYPE_CONTACTS_CONNECTION, "me@example.com",
       TRUE, &service_conn_as_base, &conn);
   service_conn = TP_TESTS_SIMPLE_CONNECTION (service_conn_as_base);
@@ -548,5 +553,8 @@ main (int argc,
   g_object_unref (service_conn);
   g_free (chan_path);
 
+  g_test_dbus_down (test_dbus);
+  tp_tests_assert_last_unref (&test_dbus);
+
   return 0;
 }
diff --git a/tests/dbus/handle-repo.c b/tests/dbus/handle-repo.c
index e4c298b5c..d7214d216 100644
--- a/tests/dbus/handle-repo.c
+++ b/tests/dbus/handle-repo.c
@@ -75,9 +75,18 @@ test_handles (void)
 
 int main (int argc, char **argv)
 {
+  GTestDBus *test_dbus;
+
   tp_tests_abort_after (10);
 
+  g_test_dbus_unset ();
+  test_dbus = g_test_dbus_new (G_TEST_DBUS_NONE);
+  g_test_dbus_up (test_dbus);
+
   test_handles ();
 
+  g_test_dbus_down (test_dbus);
+  tp_tests_assert_last_unref (&test_dbus);
+
   return 0;
 }
diff --git a/tests/dbus/handle-set.c b/tests/dbus/handle-set.c
index e7c364179..c1782f191 100644
--- a/tests/dbus/handle-set.c
+++ b/tests/dbus/handle-set.c
@@ -23,11 +23,16 @@ main (int argc,
   TpIntset *iset = NULL, *result = NULL;
   GError *error = NULL;
   gchar *s;
+  GTestDBus *test_dbus;
 
   TpHandle h1, h2, h3, h4;
 
   tp_tests_abort_after (10);
 
+  g_test_dbus_unset ();
+  test_dbus = g_test_dbus_new (G_TEST_DBUS_NONE);
+  g_test_dbus_up (test_dbus);
+
   repo = tp_tests_object_new_static_class (TP_TYPE_DYNAMIC_HANDLE_REPO,
       "handle-type", TP_ENTITY_TYPE_CONTACT,
       NULL);
@@ -123,5 +128,8 @@ main (int argc,
 
   g_object_unref (G_OBJECT (repo));
 
+  g_test_dbus_down (test_dbus);
+  tp_tests_assert_last_unref (&test_dbus);
+
   return 0;
 }
diff --git a/tests/dbus/long-connection-name.c b/tests/dbus/long-connection-name.c
index ac2b38b12..9c7dc38ba 100644
--- a/tests/dbus/long-connection-name.c
+++ b/tests/dbus/long-connection-name.c
@@ -41,8 +41,14 @@ main (int argc,
   GError *error = NULL;
   gchar *name;
   gchar *conn_path;
+  GTestDBus *test_dbus;
 
   tp_tests_abort_after (10);
+
+  g_test_dbus_unset ();
+  test_dbus = g_test_dbus_new (G_TEST_DBUS_NONE);
+  g_test_dbus_up (test_dbus);
+
   dbus = tp_tests_dbus_daemon_dup_or_die ();
 
   MYASSERT (strlen (LONG_ACCOUNT_IS_LONG) == 256, "");
@@ -67,5 +73,9 @@ main (int argc,
   g_object_unref (dbus);
   g_free (name);
   g_free (conn_path);
+
+  g_test_dbus_down (test_dbus);
+  tp_tests_assert_last_unref (&test_dbus);
+
   return 0;
 }
diff --git a/tests/dbus/message-mixin.c b/tests/dbus/message-mixin.c
index a8c602ab0..cc1ccb7d8 100644
--- a/tests/dbus/message-mixin.c
+++ b/tests/dbus/message-mixin.c
@@ -162,9 +162,15 @@ main (int argc,
   gboolean ok;
   GHashTable *parameters;
   GQuark connected_feature[] = { TP_CONNECTION_FEATURE_CONNECTED, 0 };
+  GTestDBus *test_dbus;
 
   tp_tests_abort_after (10);
   tp_debug_set_flags ("all");
+
+  g_test_dbus_unset ();
+  test_dbus = g_test_dbus_new (G_TEST_DBUS_NONE);
+  g_test_dbus_up (test_dbus);
+
   dbus = tp_tests_dbus_daemon_dup_or_die ();
 
   service_cm = EXAMPLE_ECHO_2_CONNECTION_MANAGER (
@@ -783,5 +789,8 @@ main (int argc,
   g_free (last_message_sent_token);
   g_free (last_message_sent_sender_id);
 
+  g_test_dbus_down (test_dbus);
+  tp_tests_assert_last_unref (&test_dbus);
+
   return 0;
 }
diff --git a/tests/dbus/self-presence.c b/tests/dbus/self-presence.c
index 78d66c87b..24c072735 100644
--- a/tests/dbus/self-presence.c
+++ b/tests/dbus/self-presence.c
@@ -130,11 +130,17 @@ main (int argc,
   gchar **interfaces;
   GValue *value = NULL;
   GQuark connected_feature[] = { TP_CONNECTION_FEATURE_CONNECTED, 0 };
+  GTestDBus *test_dbus;
 
   /* Setup */
 
   tp_tests_abort_after (10);
   tp_debug_set_flags ("all");
+
+  g_test_dbus_unset ();
+  test_dbus = g_test_dbus_new (G_TEST_DBUS_NONE);
+  g_test_dbus_up (test_dbus);
+
   dbus = tp_tests_dbus_daemon_dup_or_die ();
 
   service_conn = TP_TESTS_CONTACTS_CONNECTION (
@@ -198,5 +204,8 @@ main (int argc,
 
   g_object_unref (dbus);
 
+  g_test_dbus_down (test_dbus);
+  tp_tests_assert_last_unref (&test_dbus);
+
   return 0;
 }
diff --git a/tests/dbus/text-respawn.c b/tests/dbus/text-respawn.c
index 208517998..e31be5780 100644
--- a/tests/dbus/text-respawn.c
+++ b/tests/dbus/text-respawn.c
@@ -146,10 +146,15 @@ main (int argc,
   TpCapabilities *caps;
   GPtrArray *classes;
   GQuark conn_features[] = { TP_CONNECTION_FEATURE_CAPABILITIES, 0 };
+  GTestDBus *test_dbus;
 
   tp_tests_abort_after (10);
   /* tp_debug_set_flags ("all"); */
 
+  g_test_dbus_unset ();
+  test_dbus = g_test_dbus_new (G_TEST_DBUS_NONE);
+  g_test_dbus_up (test_dbus);
+
   tp_tests_create_conn (TP_TESTS_TYPE_ECHO_CONNECTION, "me@example.com",
       TRUE, &service_conn_as_base, &conn);
   service_conn = TP_TESTS_ECHO_CONNECTION (service_conn_as_base);
@@ -342,5 +347,8 @@ main (int argc,
   g_free (last_sent_text);
   g_free (last_received_text);
 
+  g_test_dbus_down (test_dbus);
+  tp_tests_assert_last_unref (&test_dbus);
+
   return 0;
 }
diff --git a/tests/lib/util.c b/tests/lib/util.c
index 86ed0d69a..f0ac8708e 100644
--- a/tests/lib/util.c
+++ b/tests/lib/util.c
@@ -80,39 +80,21 @@ tp_tests_proxy_run_until_prepared_or_failed (gpointer proxy,
   return r;
 }
 
-static GTestDBus *test_dbus = NULL;
-
-static void
-start_dbus_session (void)
+gint
+tp_tests_run_with_bus (void)
 {
-  g_assert (test_dbus == NULL);
+  GTestDBus *test_dbus = NULL;
+  gint ret;
 
   g_test_dbus_unset ();
-
   test_dbus = g_test_dbus_new (G_TEST_DBUS_NONE);
   g_test_dbus_add_service_dir (test_dbus, g_getenv ("TP_TESTS_SERVICES_DIR"));
   g_test_dbus_up (test_dbus);
-}
 
-static void
-stop_dbus_session (void)
-{
-  g_assert (test_dbus != NULL);
-  g_test_dbus_down (test_dbus);
-  g_clear_object (&test_dbus);
-}
-
-gint
-tp_tests_run_with_bus (void)
-{
-  gint ret;
-
-  if (test_dbus != NULL)
-    return g_test_run ();
-
-  start_dbus_session ();
   ret = g_test_run ();
-  stop_dbus_session ();
+
+  g_test_dbus_down (test_dbus);
+  tp_tests_assert_last_unref (&test_dbus);
 
   return ret;
 }
@@ -122,21 +104,7 @@ tp_tests_dbus_daemon_dup_or_die (void)
 {
   TpDBusDaemon *d;
 
-  if (test_dbus == NULL)
-    {
-      /* HACK: Some tests are not yet ported to GTest and thus are not using
-       * tp_tests_run_with_bus(). In that case we make sure to start the dbus
-       * session before aquiring the TpDBusDaemon and we stop the session when
-       * the daemon is disposed. In a perfect world this should not be needed.
-       */
-      start_dbus_session ();
-      d = tp_dbus_daemon_dup (NULL);
-      g_object_weak_ref ((GObject *) d, (GWeakNotify) stop_dbus_session, NULL);
-    }
-  else
-    {
-       d = tp_dbus_daemon_dup (NULL);
-    }
+  d = tp_dbus_daemon_dup (NULL);
 
   /* In a shared library, this would be very bad (see fd.o #18832), but in a
    * regression test that's going to be run under a temporary session bus,