dbus-tube: fail offer/accept if channel is invalidated

3 files changed, 93 insertions, 3 deletions

diff --git a/telepathy-glib/dbus-tube-channel.c b/telepathy-glib/dbus-tube-channel.c
index dfbb710dc..2967fdbe9 100644
--- a/telepathy-glib/dbus-tube-channel.c
+++ b/telepathy-glib/dbus-tube-channel.c
@@ -89,7 +89,8 @@ tp_dbus_tube_channel_dispose (GObject *obj)
   TpDBusTubeChannel *self = (TpDBusTubeChannel *) obj;
 
   tp_clear_pointer (&self->priv->parameters, g_hash_table_unref);
-  g_clear_object (&self->priv->result);
+  /* If priv->result isn't NULL, it owns a ref to self. */
+  g_warn_if_fail (self->priv->result == NULL);
   tp_clear_pointer (&self->priv->address, g_free);
 
   G_OBJECT_CLASS (tp_dbus_tube_channel_parent_class)->dispose (obj);
@@ -123,8 +124,19 @@ tp_dbus_tube_channel_get_property (GObject *object,
 static void
 complete_operation (TpDBusTubeChannel *self)
 {
-  g_simple_async_result_complete (self->priv->result);
-  g_clear_object (&self->priv->result);
+  TpDBusTubeChannelPrivate *priv = self->priv;
+  GSimpleAsyncResult *result = priv->result;
+
+  /* This dance is to ensure that we don't accidentally manipulate priv->result
+   * while calling out to user code. For instance, someone might call
+   * tp_proxy_invalidate() on us, which winds up landing us in here via our
+   * handler for that signal.
+   */
+  g_assert (priv->result != NULL);
+  result = priv->result;
+  priv->result = NULL;
+  g_simple_async_result_complete (result);
+  g_object_unref (result);
 }
 
 static void
@@ -172,6 +184,27 @@ check_tube_open (TpDBusTubeChannel *self)
 }
 
 static void
+dbus_tube_invalidated_cb (
+    TpProxy *proxy,
+    guint domain,
+    gint code,
+    gchar *message,
+    gpointer user_data)
+{
+  TpDBusTubeChannel *self = TP_DBUS_TUBE_CHANNEL (proxy);
+  TpDBusTubeChannelPrivate *priv = self->priv;
+  GError error = { domain, code, message };
+
+  if (priv->result != NULL)
+    {
+      DEBUG ("Tube invalidated: '%s'; failing pending offer/accept method call",
+          message);
+      g_simple_async_result_set_from_error (priv->result, &error);
+      complete_operation (self);
+    }
+}
+
+static void
 tube_state_changed_cb (TpChannel *channel,
     TpTubeChannelState state,
     gpointer user_data,
@@ -245,6 +278,9 @@ tp_dbus_tube_channel_constructed (GObject *obj)
               TP_HASH_TYPE_STRING_VARIANT_MAP, params);
         }
     }
+
+  g_signal_connect (self, "invalidated",
+      G_CALLBACK (dbus_tube_invalidated_cb), NULL);
 }
 
 static void
diff --git a/tests/dbus/dbus-tube.c b/tests/dbus/dbus-tube.c
index cc79d64b0..362fd8163 100644
--- a/tests/dbus/dbus-tube.c
+++ b/tests/dbus/dbus-tube.c
@@ -372,6 +372,26 @@ test_offer (Test *test,
 }
 
 static void
+test_offer_invalidated_before_open (Test *test,
+    gconstpointer data G_GNUC_UNUSED)
+{
+  /* Outgoing tube */
+  create_tube_service (test, TRUE, TRUE);
+  tp_tests_dbus_tube_channel_set_open_mode (test->tube_chan_service,
+      TP_TESTS_DBUS_TUBE_CHANNEL_NEVER_OPEN);
+
+  tp_dbus_tube_channel_offer_async (test->tube, NULL, tube_offer_cb, test);
+
+  test->wait = 1;
+  g_main_loop_run (test->mainloop);
+  /* FIXME: this isn't a particularly good error… it's just what comes out when
+   * the channel gets closed from under us, and there isn't really API on
+   * DBusTube to give a better error.
+   */
+  g_assert_error (test->error, TP_DBUS_ERRORS, TP_DBUS_ERROR_OBJECT_REMOVED);
+}
+
+static void
 tube_accept_cb (GObject *source,
     GAsyncResult *result,
     gpointer user_data)
@@ -413,6 +433,26 @@ test_accept (Test *test,
   use_tube (test, test->cm_conn, test->tube_conn);
 }
 
+static void
+test_accept_invalidated_before_open (Test *test,
+    gconstpointer data G_GNUC_UNUSED)
+{
+  /* Incoming tube */
+  create_tube_service (test, FALSE, TRUE);
+  tp_tests_dbus_tube_channel_set_open_mode (test->tube_chan_service,
+      TP_TESTS_DBUS_TUBE_CHANNEL_NEVER_OPEN);
+
+  tp_dbus_tube_channel_accept_async (test->tube, tube_accept_cb, test);
+
+  test->wait = 1;
+  g_main_loop_run (test->mainloop);
+  /* FIXME: this isn't a particularly good error… it's just what comes out when
+   * the channel gets closed from under us, and there isn't really API on
+   * DBusTube to give a better error.
+   */
+  g_assert_error (test->error, TP_DBUS_ERRORS, TP_DBUS_ERROR_OBJECT_REMOVED);
+}
+
 int
 main (int argc,
       char **argv)
@@ -431,12 +471,16 @@ main (int argc,
   g_test_add ("/dbus-tube/offer-open-second", Test,
       GUINT_TO_POINTER (TP_TESTS_DBUS_TUBE_CHANNEL_OPEN_SECOND),
       setup, test_offer, teardown);
+  g_test_add ("/dbus-tube/offer-invalidated-before-open", Test, NULL,
+      setup, test_offer_invalidated_before_open, teardown);
   g_test_add ("/dbus-tube/accept-open-first", Test,
       GUINT_TO_POINTER (TP_TESTS_DBUS_TUBE_CHANNEL_OPEN_FIRST),
       setup, test_accept, teardown);
   g_test_add ("/dbus-tube/accept-open-second", Test,
       GUINT_TO_POINTER (TP_TESTS_DBUS_TUBE_CHANNEL_OPEN_SECOND),
       setup, test_accept, teardown);
+  g_test_add ("/dbus-tube/accept-invalidated-before-open", Test, NULL,
+      setup, test_accept_invalidated_before_open, teardown);
 
   return g_test_run ();
 }
diff --git a/tests/lib/dbus-tube-chan.c b/tests/lib/dbus-tube-chan.c
index 7c42698de..1ba075595 100644
--- a/tests/lib/dbus-tube-chan.c
+++ b/tests/lib/dbus-tube-chan.c
@@ -156,6 +156,12 @@ dispose (GObject *object)
 
   if (self->priv->dbus_server != NULL)
     {
+      /* FIXME: this is pretty stupid but apparently unless you start and then
+       * stop the server before freeing it, it doesn't stop listening. Calling
+       * _start() twice is a no-op.
+      */
+      g_dbus_server_start (self->priv->dbus_server);
+
       g_dbus_server_stop (self->priv->dbus_server);
       g_clear_object (&self->priv->dbus_server);
     }
@@ -352,6 +358,8 @@ dbus_tube_offer (TpSvcChannelTypeDBusTube *chan,
 
   if (self->priv->open_mode == TP_TESTS_DBUS_TUBE_CHANNEL_OPEN_SECOND)
     really_open_tube (self);
+  else if (self->priv->open_mode == TP_TESTS_DBUS_TUBE_CHANNEL_NEVER_OPEN)
+    tp_base_channel_close (TP_BASE_CHANNEL (self));
 }
 
 static void
@@ -371,6 +379,8 @@ dbus_tube_accept (TpSvcChannelTypeDBusTube *chan,
 
   if (self->priv->open_mode == TP_TESTS_DBUS_TUBE_CHANNEL_OPEN_SECOND)
     really_open_tube (self);
+  else if (self->priv->open_mode == TP_TESTS_DBUS_TUBE_CHANNEL_NEVER_OPEN)
+    tp_base_channel_close (TP_BASE_CHANNEL (self));
 }
 
 void