[next] telepathy-glib: invalidated-while-invoking-signals test: extend test coverage

Simon McVittie smcv at kemper.freedesktop.org
Thu Mar 13 07:08:35 PDT 2014


Module: telepathy-glib
Branch: next
Commit: 6eebc294fb46289a9e55be18bdee642e127f01df
URL:    http://cgit.freedesktop.org/telepathy/telepathy-glib/commit/?id=6eebc294fb46289a9e55be18bdee642e127f01df

Author: Simon McVittie <simon.mcvittie at collabora.co.uk>
Date:   Mon Mar 10 20:07:20 2014 +0000

invalidated-while-invoking-signals test: extend test coverage

Since commit 6f153bca, we didn't wait for on_status_changed() to occur.
At some point it ceased to be reached at all, which meant that
commit db582a0d added an unbalanced unref for @client in what has
now become teardown(). If on_status_changed() was executed, it would
unref the client, and then the unref in teardown() would either be
a use-after-free or indirectly lead to one.

Porting telepathy-glib to GDBus exposed this bug, by making
on_status_changed() reachable again.

While fixing this I noticed that on_status_changed() is not guaranteed
to be the last-unref, so the test does not necessarily even reproduce
the original crash situation, which was the proxy being invalidated
inside the signal callback. I've addressed that by testing once with
the way the test has worked in practice since at least 2012, and
once with explicit forced invalidation.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=76000
Reviewed-by: Guillaume Desmottes

---

 tests/dbus/invalidated-while-invoking-signals.c |   45 +++++++++++++++++++----
 1 file changed, 37 insertions(+), 8 deletions(-)

diff --git a/tests/dbus/invalidated-while-invoking-signals.c b/tests/dbus/invalidated-while-invoking-signals.c
index 1ba8656..e2ae95c 100644
--- a/tests/dbus/invalidated-while-invoking-signals.c
+++ b/tests/dbus/invalidated-while-invoking-signals.c
@@ -15,15 +15,17 @@
 #include <telepathy-glib/connection.h>
 #include <telepathy-glib/dbus.h>
 #include <telepathy-glib/debug.h>
+#include <telepathy-glib/proxy-subclass.h>
 
 #include "tests/lib/myassert.h"
 #include "tests/lib/contacts-conn.h"
 #include "tests/lib/util.h"
 
 typedef struct {
+    const gchar *mode;
     TpBaseConnection *service;
     TpConnection *client;
-    GMainLoop *mainloop;
+    gboolean shutdown_finished;
 } Fixture;
 
 static void
@@ -38,8 +40,22 @@ on_status_changed (TpConnection *connection,
   MYASSERT (status == TP_CONNECTION_STATUS_DISCONNECTED, "%u", status);
 
   MYASSERT (f->client == connection, "%p vs %p", f->client, connection);
-  g_object_unref (f->client);
-  f->client = NULL;
+
+  if (!tp_strdiff (f->mode, "invalidate"))
+    {
+      GError e = { TP_ERROR, TP_ERROR_CANCELLED, "regression test" };
+
+      tp_proxy_invalidate (TP_PROXY (f->client), &e);
+    }
+  else
+    {
+      /* The original test did this, and assumed that this was the
+       * last-unref, and would cause invalidation. That was a failing
+       * test-case for #14854 before it was fixed. However, the fix for
+       * #14854 made that untrue, by taking a reference. */
+      g_object_unref (f->client);
+      f->client = NULL;
+    }
 }
 
 static gboolean
@@ -59,14 +75,15 @@ on_shutdown_finished (TpBaseConnection *base_conn,
 {
   Fixture *f = user_data;
 
-  g_main_loop_quit (f->mainloop);
+  f->shutdown_finished = TRUE;
 }
 
 static void
 setup (Fixture *f,
     gconstpointer data)
 {
-  f->mainloop = g_main_loop_new (NULL, FALSE);
+  f->mode = data;
+  f->shutdown_finished = FALSE;
 
   tp_tests_create_and_connect_conn (TP_TESTS_TYPE_CONTACTS_CONNECTION,
       "me at example.com", &f->service, &f->client);
@@ -84,7 +101,17 @@ test_invalidated_while_invoking_signals (Fixture *f,
 
   g_idle_add (disconnect, f);
 
-  g_main_loop_run (f->mainloop);
+  if (!tp_strdiff (f->mode, "invalidate"))
+    {
+      while (tp_proxy_get_invalidated (f->client) == NULL ||
+          !f->shutdown_finished)
+        g_main_context_iteration (NULL, TRUE);
+    }
+  else
+    {
+      while (f->client != NULL || !f->shutdown_finished)
+        g_main_context_iteration (NULL, TRUE);
+    }
 }
 
 static void
@@ -92,8 +119,7 @@ teardown (Fixture *f,
     gconstpointer data)
 {
   g_object_unref (f->service);
-  g_object_unref (f->client);
-  g_main_loop_unref (f->mainloop);
+  g_clear_object (&f->client);
 }
 
 int
@@ -105,6 +131,9 @@ main (int argc,
 
   g_test_add ("/invalidated-while-invoking-signals/dispose",
       Fixture, NULL, setup, test_invalidated_while_invoking_signals, teardown);
+  g_test_add ("/invalidated-while-invoking-signals/invalidate",
+      Fixture, "invalidate", setup, test_invalidated_while_invoking_signals,
+      teardown);
 
   return tp_tests_run_with_bus ();
 }



More information about the telepathy-commits mailing list