From 3a3cd0d059e0d97e0562da0ca999631c1d421831 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 10 Mar 2014 20:07:20 +0000 Subject: [PATCH 16/23] 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. --- 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 #include #include +#include #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@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 (); } -- 1.9.0