Attachment 82830 Details for Bug 67183 – TpConnectionManager: retry introspection after CM exits, up to once

[patch] TpConnectionManager: retry introspection after CM exits, up to once

0001-TpConnectionManager-retry-introspection-after-CM-exi.patch (text/plain), 7.97 KB, created by Simon McVittie on 2013-07-22 17:42:00 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2013-07-22 17:42:00 UTC

Size: 7.97 KB

patch

obsolete

>From 402401dce04a5afc9b020529123216adb5201269 Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>Date: Mon, 22 Jul 2013 17:57:23 +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.
>---
> telepathy-glib/connection-manager.c | 29 ++++++++++++--
> tests/dbus/cm.c                     | 76 +++++++++++++++++++++++++++++++++----
> 2 files changed, 95 insertions(+), 10 deletions(-)
>
>diff --git a/telepathy-glib/connection-manager.c b/telepathy-glib/connection-manager.c
>index 7130f62..c816bfc 100644
>--- a/telepathy-glib/connection-manager.c
>+++ b/telepathy-glib/connection-manager.c
>@@ -278,6 +278,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,
>@@ -623,8 +626,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)
> {
>   guint i;
> 
>@@ -651,6 +653,14 @@ tp_connection_manager_end_introspection (TpConnectionManager *self,
>       self->priv->pending_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->info_source);
>   g_signal_emit (self, signals[SIGNAL_GOT_INFO], 0, self->info_source);
>   tp_connection_manager_ready_or_failed (self, error);
>@@ -930,7 +940,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 363c1e7..86c7c1d 100644
>--- a/tests/dbus/cm.c
>+++ b/tests/dbus/cm.c
>@@ -18,7 +18,9 @@
> typedef enum {
>     ACTIVATE_CM = (1 << 0),
>     USE_CWR = (1 << 1),
>-    USE_OLD_LIST = (1 << 2)
>+    USE_OLD_LIST = (1 << 2),
>+    DROP_NAME_ON_GET = (1 << 3),
>+    DROP_NAME_ON_GET_TWICE = (1 << 4)
> } TestFlags;
> 
> typedef struct {
>@@ -32,7 +34,10 @@ typedef struct {
>     GError *error /* initialized where needed */;
> } Test;
> 
>-typedef TpTestsEchoConnectionManager PropertylessConnectionManager;
>+typedef struct {
>+    TpTestsEchoConnectionManager parent;
>+    guint drop_name_on_get;
>+} PropertylessConnectionManager;
> typedef TpTestsEchoConnectionManagerClass PropertylessConnectionManagerClass;
> 
> static void stub_properties_iface_init (gpointer iface);
>@@ -65,10 +70,30 @@ stub_get (TpSvcDBusProperties *iface G_GNUC_UNUSED,
> }
> 
> static void
>-stub_get_all (TpSvcDBusProperties *iface G_GNUC_UNUSED,
>+stub_get_all (TpSvcDBusProperties *iface,
>     const gchar *i G_GNUC_UNUSED,
>     DBusGMethodInvocation *context)
> {
>+  PropertylessConnectionManager *cm = (PropertylessConnectionManager *) iface;
>+
>+  /* 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");
>+
>+      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);
>+    }
>+
>   tp_dbus_g_method_return_not_implemented (context);
> }
> 
>@@ -980,6 +1005,7 @@ test_dbus_fallback (Test *test,
>   gchar *name;
>   guint info_source;
>   const TestFlags flags = GPOINTER_TO_INT (data);
>+  PropertylessConnectionManager *service_cm;
>   TpBaseConnectionManager *service_cm_as_base;
>   gboolean ok;
> 
>@@ -987,9 +1013,10 @@ test_dbus_fallback (Test *test,
>    * exercise the fallback path */
>   g_object_unref (test->service_cm);
>   test->service_cm = NULL;
>-  test->service_cm = TP_TESTS_ECHO_CONNECTION_MANAGER (tp_tests_object_new_static_class (
>-        propertyless_connection_manager_get_type (),
>-        NULL));
>+  service_cm = tp_tests_object_new_static_class (
>+      propertyless_connection_manager_get_type (),
>+      NULL);
>+  test->service_cm = TP_TESTS_ECHO_CONNECTION_MANAGER (service_cm);
>   g_assert (test->service_cm != NULL);
>   service_cm_as_base = TP_BASE_CONNECTION_MANAGER (test->service_cm);
>   g_assert (service_cm_as_base != NULL);
>@@ -1003,6 +1030,15 @@ test_dbus_fallback (Test *test,
>   g_assert (TP_IS_CONNECTION_MANAGER (test->cm));
>   g_assert_no_error (test->error);
> 
>+  if (flags & DROP_NAME_ON_GET_TWICE)
>+    {
>+      service_cm->drop_name_on_get = 2;
>+    }
>+  else if (flags & DROP_NAME_ON_GET)
>+    {
>+      service_cm->drop_name_on_get = 1;
>+    }
>+
>   if (flags & ACTIVATE_CM)
>     {
>       g_test_bug ("23524");
>@@ -1029,11 +1065,30 @@ test_dbus_fallback (Test *test,
>     }
>   else
>     {
>-      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");
>+
>+  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_is_ready (test->cm), ==, 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);
>@@ -1190,6 +1245,13 @@ main (int argc,
>       GINT_TO_POINTER (ACTIVATE_CM | USE_CWR),
>       setup, test_dbus_fallback, teardown);
> 
>+  g_test_add ("/cm/dbus-fallback/dies", Test,
>+      GINT_TO_POINTER (DROP_NAME_ON_GET), setup, test_dbus_fallback, teardown);
>+
>+  g_test_add ("/cm/dbus-fallback/dies-twice", Test,
>+      GINT_TO_POINTER (DROP_NAME_ON_GET_TWICE),
>+      setup, test_dbus_fallback, teardown);
>+
>   g_test_add ("/cm/list", Test, GINT_TO_POINTER (0),
>       setup, test_list, teardown);
>   g_test_add ("/cm/list", Test, GINT_TO_POINTER (USE_OLD_LIST),
>-- 
>1.8.3.2
>

Actions: View | Splinter Review

Attachments on bug 67183: 82830

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.