Attachment 85739 Details for Bug 69283 – [next] TpConnectionManager: retry introspection after CM exits, up to once

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

0001-TpConnectionManager-retry-introspection-after-CM-exi.patch (text/plain), 9.54 KB, created by Simon McVittie on 2013-09-12 18:39:57 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2013-09-12 18:39:57 UTC

Size: 9.54 KB

patch

obsolete

>From 93a423dd834c8f1466f1f9b27db05e92dffdab44 Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>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
>

Actions: View | Splinter Review

Attachments on bug 69283: 85732 | 85733 | 85739

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.