From 93a423dd834c8f1466f1f9b27db05e92dffdab44 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 12 Sep 2013 19:34:11 +0100 Subject: [PATCH] TpConnectionManager: retry introspection after CM exits, up to once Many connection managers automatically exit after 5 seconds of inactivity. If the CM has no .manager file *and* exits in this way while we are introspecting it, we would previously consider it to have failed introspection - but with sufficiently unfortunate timing, that can result in empathy-accounts not considering Haze to exist. To avoid this, without going into an infinite loop if the CM fails to introspect, retry once, but only once. [This is a forward-port to Telepathy 1.0.] Bug: https://bugs.freedesktop.org/show_bug.cgi?id=67183 Bug: https://bugs.freedesktop.org/show_bug.cgi?id=69283 --- telepathy-glib/connection-manager.c | 28 ++++++- tests/dbus/cm.c | 144 ++++++++++++++++++++++++++++++++++-- 2 files changed, 161 insertions(+), 11 deletions(-) diff --git a/telepathy-glib/connection-manager.c b/telepathy-glib/connection-manager.c index 5b4e17e..290ec7c 100644 --- a/telepathy-glib/connection-manager.c +++ b/telepathy-glib/connection-manager.c @@ -206,6 +206,9 @@ struct _TpConnectionManagerPrivate { /* TRUE if someone asked us to activate but we're putting it off until * name_known */ gboolean want_activation; + /* TRUE if the CM exited (crashed?) during introspection. + * We'll retry, but only once. */ + gboolean retried_introspection; }; G_DEFINE_TYPE (TpConnectionManager, @@ -305,8 +308,7 @@ static void tp_connection_manager_ready_or_failed (TpConnectionManager *self, const GError *error); static void -tp_connection_manager_end_introspection (TpConnectionManager *self, - const GError *error) +tp_connection_manager_reset_introspection (TpConnectionManager *self) { self->priv->introspection_step = INTROSPECT_IDLE; @@ -321,6 +323,13 @@ tp_connection_manager_end_introspection (TpConnectionManager *self, g_hash_table_unref (self->priv->found_protocols); self->priv->found_protocols = NULL; } +} + +static void +tp_connection_manager_end_introspection (TpConnectionManager *self, + const GError *error) +{ + tp_connection_manager_reset_introspection (self); DEBUG ("End of introspection, info source %u", self->priv->info_source); g_signal_emit (self, signals[SIGNAL_GOT_INFO], 0, self->priv->info_source); @@ -489,7 +498,20 @@ tp_connection_manager_name_owner_changed_cb (TpDBusDaemon *bus, /* cancel pending introspection, if any */ if (introspection_in_progress (self)) - tp_connection_manager_end_introspection (self, &e); + { + if (self->priv->retried_introspection) + { + DEBUG ("%s, twice: assuming fatal and not retrying", e.message); + tp_connection_manager_end_introspection (self, &e); + } + else + { + self->priv->retried_introspection = TRUE; + DEBUG ("%s: retrying", e.message); + tp_connection_manager_reset_introspection (self); + tp_connection_manager_continue_introspection (self); + } + } /* If our name wasn't known already, a change to "" is just the initial * state, so we didn't *exit* as such. */ diff --git a/tests/dbus/cm.c b/tests/dbus/cm.c index 64512a3..994b61a 100644 --- a/tests/dbus/cm.c +++ b/tests/dbus/cm.c @@ -18,12 +18,20 @@ typedef enum { ACTIVATE_CM = (1 << 0), + DROP_NAME_ON_GET = (1 << 3), + DROP_NAME_ON_GET_TWICE = (1 << 4) } TestFlags; typedef struct { + ExampleEcho2ConnectionManager parent; + guint drop_name_on_get; +} MyConnectionManager; +typedef ExampleEcho2ConnectionManagerClass MyConnectionManagerClass; + +typedef struct { GMainLoop *mainloop; TpDBusDaemon *dbus; - ExampleEcho2ConnectionManager *service_cm; + MyConnectionManager *service_cm; TpConnectionManager *cm; TpConnectionManager *echo; @@ -31,6 +39,79 @@ typedef struct { GError *error /* initialized where needed */; } Test; +static void my_properties_iface_init (gpointer iface); +static GType my_connection_manager_get_type (void); + +G_DEFINE_TYPE_WITH_CODE (MyConnectionManager, + my_connection_manager, + EXAMPLE_TYPE_ECHO_2_CONNECTION_MANAGER, + G_IMPLEMENT_INTERFACE (TP_TYPE_SVC_DBUS_PROPERTIES, + my_properties_iface_init)) + +static void +my_connection_manager_class_init (MyConnectionManagerClass *cls) +{ +} + +static void +my_connection_manager_init (MyConnectionManager *self) +{ +} + +static void +my_get (TpSvcDBusProperties *iface G_GNUC_UNUSED, + const gchar *i G_GNUC_UNUSED, + const gchar *p G_GNUC_UNUSED, + DBusGMethodInvocation *context) +{ + /* The telepathy-glib client side should never call this: + * GetAll() is better. */ + g_assert_not_reached (); +} + +static void +my_get_all (TpSvcDBusProperties *iface, + const gchar *i, + DBusGMethodInvocation *context) +{ + MyConnectionManager *cm = (MyConnectionManager *) iface; + GHashTable *ht; + + /* If necessary, emulate the CM exiting and coming back. */ + if (cm->drop_name_on_get) + { + TpDBusDaemon *dbus = tp_base_connection_manager_get_dbus_daemon ( + TP_BASE_CONNECTION_MANAGER (cm)); + GString *string = g_string_new (TP_CM_BUS_NAME_BASE); + GError *error = NULL; + + g_string_append (string, "example_echo_2"); + + cm->drop_name_on_get--; + + tp_dbus_daemon_release_name (dbus, string->str, &error); + g_assert_no_error (error); + tp_dbus_daemon_request_name (dbus, string->str, FALSE, &error); + g_assert_no_error (error); + } + + ht = tp_dbus_properties_mixin_dup_all ((GObject *) cm, i); + tp_svc_dbus_properties_return_from_get_all (context, ht); + g_hash_table_unref (ht); +} + +static void +my_properties_iface_init (gpointer iface) +{ + TpSvcDBusPropertiesClass *cls = iface; + +#define IMPLEMENT(x) \ + tp_svc_dbus_properties_implement_##x (cls, my_##x) + IMPLEMENT (get); + IMPLEMENT (get_all); +#undef IMPLEMENT +} + static void setup (Test *test, gconstpointer data) @@ -44,9 +125,9 @@ setup (Test *test, test->mainloop = g_main_loop_new (NULL, FALSE); test->dbus = tp_tests_dbus_daemon_dup_or_die (); - test->service_cm = EXAMPLE_ECHO_2_CONNECTION_MANAGER (g_object_new ( - EXAMPLE_TYPE_ECHO_2_CONNECTION_MANAGER, - NULL)); + test->service_cm = tp_tests_object_new_static_class ( + my_connection_manager_get_type (), + NULL); g_assert (test->service_cm != NULL); service_cm_as_base = TP_BASE_CONNECTION_MANAGER (test->service_cm); g_assert (service_cm_as_base != NULL); @@ -197,9 +278,19 @@ test_file_got_info (Test *test, TP_CM_INFO_SOURCE_FILE); strv = tp_connection_manager_dup_protocol_names (test->cm); - g_assert_cmpuint (g_strv_length (strv), ==, 2); - g_assert (tp_strv_contains ((const gchar * const *) strv, "normal")); - g_assert (tp_strv_contains ((const gchar * const *)strv, "weird")); + + if (tp_strdiff (strv[0], "normal")) + { + g_assert_cmpstr (strv[0], ==, "weird"); + g_assert_cmpstr (strv[1], ==, "normal"); + } + else + { + g_assert_cmpstr (strv[0], ==, "normal"); + g_assert_cmpstr (strv[1], ==, "weird"); + } + + g_assert (strv[2] == NULL); g_strfreev (strv); g_assert (tp_connection_manager_has_protocol (test->cm, "normal")); @@ -850,6 +941,15 @@ test_dbus_ready (Test *test, g_assert (TP_IS_CONNECTION_MANAGER (test->cm)); g_assert_no_error (test->error); + if (flags & DROP_NAME_ON_GET_TWICE) + { + test->service_cm->drop_name_on_get = 2; + } + else if (flags & DROP_NAME_ON_GET) + { + test->service_cm->drop_name_on_get = 1; + } + if (flags & ACTIVATE_CM) { g_test_bug ("23524"); @@ -867,10 +967,28 @@ test_dbus_ready (Test *test, g_test_bug ("18291"); } - tp_tests_proxy_run_until_prepared (test->cm, NULL); + tp_tests_proxy_run_until_prepared_or_failed (test->cm, NULL, + &test->error); g_assert_cmpstr (tp_connection_manager_get_name (test->cm), ==, "example_echo_2"); + + if (flags & DROP_NAME_ON_GET_TWICE) + { + /* If it dies during introspection *twice*, we assume it has crashed + * or something. */ + g_assert_error (test->error, TP_DBUS_ERRORS, + TP_DBUS_ERROR_NAME_OWNER_LOST); + g_clear_error (&test->error); + + g_assert_cmpuint (tp_proxy_is_prepared (test->cm, + TP_CONNECTION_MANAGER_FEATURE_CORE), ==, FALSE); + g_assert_cmpuint (tp_connection_manager_get_info_source (test->cm), ==, + TP_CM_INFO_SOURCE_NONE); + return; + } + + g_assert_no_error (test->error); g_assert_cmpuint (tp_proxy_is_prepared (test->cm, TP_CONNECTION_MANAGER_FEATURE_CORE), ==, TRUE); g_assert (tp_proxy_get_invalidated (test->cm) == NULL); @@ -968,6 +1086,16 @@ main (int argc, test_complex_file_ready, teardown); g_test_add ("/cm/dbus", Test, GINT_TO_POINTER (0), setup, test_dbus_ready, teardown); + g_test_add ("/cm/dbus/activate", Test, GINT_TO_POINTER (ACTIVATE_CM), + setup, test_dbus_ready, teardown); + + g_test_add ("/cm/dbus/dies", Test, + GINT_TO_POINTER (DROP_NAME_ON_GET), + setup, test_dbus_ready, teardown); + + g_test_add ("/cm/dbus/dies-twice", Test, + GINT_TO_POINTER (DROP_NAME_ON_GET_TWICE), + setup, test_dbus_ready, teardown); g_test_add ("/cm/list", Test, GINT_TO_POINTER (0), setup, test_list, teardown); -- 1.8.4.rc3