[telepathy-mission-control/master] McdDispatcher: correctly drop refs to channels when they abort

[Simon McVittie]

Also emit ChannelLost from McdDispatchOperation, and
make the dispatch operation close appropriately.
---
 src/mcd-dispatch-operation.c |   56 ++++++++++++++++++++++++++++++++++++++++++
 src/mcd-dispatch-operation.h |    2 +
 src/mcd-dispatcher.c         |   51 +++++++++++++++++++++++++++++++++-----
 3 files changed, 102 insertions(+), 7 deletions(-)

diff --git a/src/mcd-dispatch-operation.c b/src/mcd-dispatch-operation.c
index a2232c2..94e66eb 100644
--- a/src/mcd-dispatch-operation.c
+++ b/src/mcd-dispatch-operation.c
@@ -37,6 +37,7 @@
 #include <telepathy-glib/util.h>
 
 #include "mcd-dbusprop.h"
+#include "mcd-misc.h"
 #include "_gen/interfaces.h"
 #include "_gen/gtypes.h"
 
@@ -589,3 +590,58 @@ mcd_dispatch_operation_handle_with (McdDispatchOperation *operation,
     mcd_dispatch_operation_finish (operation);
 }
 
+void
+_mcd_dispatch_operation_lose_channel (McdDispatchOperation *self,
+                                      McdChannel *channel,
+                                      GList **channels)
+{
+    GList *li = g_list_find (self->priv->channels, channel);
+    const gchar *object_path;
+
+    if (li == NULL)
+    {
+        return;
+    }
+
+    self->priv->channels = g_list_delete_link (self->priv->channels, li);
+
+    /* Because the McdDispatcherContext has a borrowed copy of our list
+     * of channels, we need to tell it the new head of the list, in case
+     * we've just removed the first link. Further, we need to do this before
+     * emitting any signals.
+     *
+     * This is amazingly fragile. */
+    *channels = self->priv->channels;
+
+    object_path = mcd_channel_get_object_path (channel);
+
+    if (object_path == NULL)
+    {
+        /* This shouldn't happen, but McdChannel is twisty enough that I
+         * can't be sure */
+        g_critical ("McdChannel has already lost its TpChannel: %p",
+            channel);
+    }
+    else
+    {
+        const GError *error = mcd_channel_get_error (channel);
+        gchar *error_name = _mcd_build_error_string (error);
+
+        mc_svc_channel_dispatch_operation_emit_channel_lost (self, object_path,
+                                                             error_name,
+                                                             error->message);
+        g_free (error_name);
+    }
+
+    /* We previously had a ref in the linked list - drop it */
+    g_object_unref (channel);
+
+    if (self->priv->channels == NULL)
+    {
+        /* no channels left, so the CDO finishes */
+        if (!self->priv->finished)
+        {
+            mcd_dispatch_operation_finish (self);
+        }
+    }
+}
diff --git a/src/mcd-dispatch-operation.h b/src/mcd-dispatch-operation.h
index 0d989f9..d09fb70 100644
--- a/src/mcd-dispatch-operation.h
+++ b/src/mcd-dispatch-operation.h
@@ -60,6 +60,8 @@ struct _McdDispatchOperationClass
 GType mcd_dispatch_operation_get_type (void);
 G_GNUC_INTERNAL McdDispatchOperation *_mcd_dispatch_operation_new (
     TpDBusDaemon *dbus_daemon, GList *channels, GStrv possible_handlers);
+G_GNUC_INTERNAL void _mcd_dispatch_operation_lose_channel (
+    McdDispatchOperation *self, McdChannel *channel, GList **channels);
 
 const gchar *mcd_dispatch_operation_get_path (McdDispatchOperation *operation);
 GHashTable *mcd_dispatch_operation_get_properties
diff --git a/src/mcd-dispatcher.c b/src/mcd-dispatcher.c
index df682a5..d75695d 100644
--- a/src/mcd-dispatcher.c
+++ b/src/mcd-dispatcher.c
@@ -1680,18 +1680,50 @@ static void
 on_channel_abort_context (McdChannel *channel, McdDispatcherContext *context)
 {
     const GError *error;
-    DEBUG ("Channel %p aborted while in a dispatcher context", channel);
+    GList *li = g_list_find (context->channels, channel);
 
-    /* TODO: it's still not clear what we should do with these aborted
-     * channels; for now, we keep them in the context, pretending that nothing
-     * happened -- the channel handler will see that they don't exist anymore
-     */
+    DEBUG ("Channel %p aborted while in a dispatcher context", channel);
 
-    /* but if it was a channel request, and it was cancelled, then the whole
+    /* if it was a channel request, and it was cancelled, then the whole
      * context should be aborted */
     error = mcd_channel_get_error (channel);
     if (error && error->code == TP_ERROR_CANCELLED)
         context->cancelled = TRUE;
+
+    /* Losing the channel might mean we get freed, which would make some of
+     * the operations below very unhappy */
+    mcd_dispatcher_context_ref (context);
+
+    if (context->operation)
+    {
+        /* the CDO owns the linked list and we just borrow it; in case it's
+         * the head of the list that we're deleting, we need to ask the CDO
+         * to update our idea of what the list is before emitting any signals.
+         *
+         * FIXME: this is alarmingly fragile */
+        _mcd_dispatch_operation_lose_channel (context->operation, channel,
+                                              &(context->channels));
+
+        if (li != NULL)
+        {
+            /* we used to have a ref to it, until the CDO removed it from the
+             * linked list. (Do not dereference li at this point - it has
+             * been freed!) */
+            g_object_unref (channel);
+        }
+    }
+    else
+    {
+        /* we own the linked list */
+        context->channels = g_list_delete_link (context->channels, li);
+    }
+
+    if (context->channels == NULL)
+    {
+        DEBUG ("Nothing left in this context");
+    }
+
+    mcd_dispatcher_context_unref (context);
 }
 
 static void
@@ -1708,7 +1740,12 @@ on_operation_finished (McdDispatchOperation *operation,
             context->dispatcher, mcd_dispatch_operation_get_path (operation));
     }
 
-    if (mcd_dispatch_operation_is_claimed (operation))
+    if (context->channels == NULL)
+    {
+        DEBUG ("Nothing left to dispatch");
+        mcd_dispatcher_context_handler_done (context);
+    }
+    else if (mcd_dispatch_operation_is_claimed (operation))
     {
         GList *list;
 
-- 
1.5.6.5