6 files changed, 85 insertions, 59 deletions

diff --git a/src/cadet/cadet_api.c b/src/cadet/cadet_api.c
index 85a8be522..980b9abbf 100644
--- a/src/cadet/cadet_api.c
+++ b/src/cadet/cadet_api.c
@@ -1273,7 +1273,7 @@ GNUNET_CADET_close_port (struct GNUNET_CADET_Port *p)
 /**
  * Destroy an existing channel.
  *
- * The existing end callback for the channel will be called immediately.
+ * The existing end callback for the channel will NOT be called.
  * Any pending outgoing messages will be sent but no incoming messages will be
  * accepted and no data callbacks will be called.
  *
@@ -1296,6 +1296,7 @@ GNUNET_CADET_channel_destroy (struct GNUNET_CADET_Channel *channel)
   }
   GNUNET_log (GNUNET_ERROR_TYPE_INFO,
               "Destroying channel due to GNUNET_CADET_channel_destroy()\n");
+  channel->disconnects = NULL;
   destroy_channel (channel);
 }
 
diff --git a/src/conversation/gnunet-service-conversation.c b/src/conversation/gnunet-service-conversation.c
index fb9eec26e..059bb158b 100644
--- a/src/conversation/gnunet-service-conversation.c
+++ b/src/conversation/gnunet-service-conversation.c
@@ -303,6 +303,47 @@ handle_client_pickup_message (void *cls,
 
 
 /**
+ * Channel went down, notify client and free data
+ * structure.
+ *
+ * @param ch channel that went down
+ */
+static void
+clean_up_channel (struct Channel *ch)
+{
+  struct Line *line = ch->line;
+  struct GNUNET_MQ_Envelope *env;
+  struct ClientPhoneHangupMessage *hup;
+
+  switch (ch->status)
+  {
+  case CS_CALLEE_INIT:
+  case CS_CALLEE_SHUTDOWN:
+  case CS_CALLER_SHUTDOWN:
+    break;
+  case CS_CALLEE_RINGING:
+  case CS_CALLEE_CONNECTED:
+  case CS_CALLER_CALLING:
+  case CS_CALLER_CONNECTED:
+    if (NULL != line)
+    {
+      env = GNUNET_MQ_msg (hup,
+                           GNUNET_MESSAGE_TYPE_CONVERSATION_CS_PHONE_HANG_UP);
+      hup->cid = ch->cid;
+      GNUNET_MQ_send (line->mq,
+                      env);
+    }
+    break;
+  }
+  if (NULL != line)
+    GNUNET_CONTAINER_DLL_remove (line->channel_head,
+                                 line->channel_tail,
+                                 ch);
+  GNUNET_free (ch);
+}
+
+
+/**
  * Destroy a channel.
  *
  * @param ch channel to destroy.
@@ -313,7 +354,11 @@ destroy_line_cadet_channels (struct Channel *ch)
   GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
               "Destroying cadet channels\n");
   if (NULL != ch->channel)
+  {
     GNUNET_CADET_channel_destroy (ch->channel);
+    ch->channel = NULL;
+  }
+  clean_up_channel (ch);
 }
 
 
@@ -1027,40 +1072,13 @@ inbound_end (void *cls,
              const struct GNUNET_CADET_Channel *channel)
 {
   struct Channel *ch = cls;
-  struct Line *line = ch->line;
-  struct GNUNET_MQ_Envelope *env;
-  struct ClientPhoneHangupMessage *hup;
 
   GNUNET_assert (channel == ch->channel);
   ch->channel = NULL;
   GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
               "Channel destroyed by CADET in state %d\n",
               ch->status);
-  switch (ch->status)
-  {
-  case CS_CALLEE_INIT:
-  case CS_CALLEE_SHUTDOWN:
-  case CS_CALLER_SHUTDOWN:
-    break;
-  case CS_CALLEE_RINGING:
-  case CS_CALLEE_CONNECTED:
-  case CS_CALLER_CALLING:
-  case CS_CALLER_CONNECTED:
-    if (NULL != line)
-    {
-      env = GNUNET_MQ_msg (hup,
-                           GNUNET_MESSAGE_TYPE_CONVERSATION_CS_PHONE_HANG_UP);
-      hup->cid = ch->cid;
-      GNUNET_MQ_send (line->mq,
-                      env);
-    }
-    break;
-  }
-  if (NULL != line)
-    GNUNET_CONTAINER_DLL_remove (line->channel_head,
-                                 line->channel_tail,
-                                 ch);
-  GNUNET_free (ch);
+  clean_up_channel (ch);
 }
 
 
diff --git a/src/include/gnunet_cadet_service.h b/src/include/gnunet_cadet_service.h
index 552763055..276fe4dbc 100644
--- a/src/include/gnunet_cadet_service.h
+++ b/src/include/gnunet_cadet_service.h
@@ -151,7 +151,7 @@ typedef void *
 
 
 /**
- * Function called whenever an MQ-channel is destroyed, even if the destruction
+ * Function called whenever an MQ-channel is destroyed, unless the destruction
  * was requested by #GNUNET_CADET_channel_destroy.
  * It must NOT call #GNUNET_CADET_channel_destroy on the channel.
  *
@@ -277,7 +277,7 @@ GNUNET_CADET_channel_create (struct GNUNET_CADET_Handle *h,
 /**
  * Destroy an existing channel.
  *
- * The existing end callback for the channel will be called immediately.
+ * The existing end callback for the channel will NOT be called.
  * Any pending outgoing messages will be sent but no incoming messages will be
  * accepted and no data callbacks will be called.
  *
diff --git a/src/set/gnunet-service-set.c b/src/set/gnunet-service-set.c
index 217b89d6d..75122395d 100644
--- a/src/set/gnunet-service-set.c
+++ b/src/set/gnunet-service-set.c
@@ -221,11 +221,7 @@ incoming_destroy (struct Operation *op)
     GNUNET_SCHEDULER_cancel (op->timeout_task);
     op->timeout_task = NULL;
   }
-  if (NULL != (channel = op->channel))
-  {
-    op->channel = NULL;
-    GNUNET_CADET_channel_destroy (channel);
-  }
+  _GSS_operation_destroy2 (op);
 }
 
 
@@ -1199,9 +1195,30 @@ channel_end_cb (void *channel_ctx,
 {
   struct Operation *op = channel_ctx;
 
+  op->channel = NULL;
+  _GSS_operation_destroy2 (op);
+}
+
+
+/**
+ * This function probably should not exist
+ * and be replaced by inlining more specific
+ * logic in the various places where it is called.
+ */
+void
+_GSS_operation_destroy2 (struct Operation *op)
+{
+  struct GNUNET_CADET_Channel *channel;
+
   GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
               "channel_end_cb called\n");
-  op->channel = NULL;
+  if (NULL != (channel = op->channel))
+  {
+    /* This will free op; called conditionally as this helper function
+       is also called from within the channel disconnect handler. */
+    op->channel = NULL;
+    GNUNET_CADET_channel_destroy (channel);
+  }
   if (NULL != op->listener)
     incoming_destroy (op);
   else if (NULL != op->set)
@@ -1376,7 +1393,7 @@ handle_client_reject (void *cls,
               "Peer request (op %u, app %s) rejected by client\n",
               op->listener->operation,
               GNUNET_h2s (&cs->listener->app_id));
-  GNUNET_CADET_channel_destroy (op->channel);
+  _GSS_operation_destroy2 (op);
   GNUNET_SERVICE_client_continue (cs->client);
 }
 
diff --git a/src/set/gnunet-service-set.h b/src/set/gnunet-service-set.h
index f7c262eac..a58b22995 100644
--- a/src/set/gnunet-service-set.h
+++ b/src/set/gnunet-service-set.h
@@ -630,6 +630,15 @@ _GSS_operation_destroy (struct Operation *op,
 
 
 /**
+ * This function probably should not exist
+ * and be replaced by inlining more specific
+ * logic in the various places where it is called.
+ */
+void
+_GSS_operation_destroy2 (struct Operation *op);
+
+
+/**
  * Get the table with implementing functions for set union.
  *
  * @return the operation specific VTable
diff --git a/src/set/gnunet-service-set_union.c b/src/set/gnunet-service-set_union.c
index c3c14f1ba..8c0c52d64 100644
--- a/src/set/gnunet-service-set_union.c
+++ b/src/set/gnunet-service-set_union.c
@@ -1367,25 +1367,6 @@ send_client_element (struct Operation *op,
 
 
 /**
- * Destroy remote channel.
- *
- * @param op operation
- */
-void destroy_channel (struct Operation *op)
-{
-  struct GNUNET_CADET_Channel *channel;
-
-  if (NULL != (channel = op->channel))
-  {
-    /* This will free op; called conditionally as this helper function
-       is also called from within the channel disconnect handler. */
-    op->channel = NULL;
-    GNUNET_CADET_channel_destroy (channel);
-  }
-}
-
-
-/**
  * Signal to the client that the operation has finished and
  * destroy the operation.
  *
@@ -1467,7 +1448,7 @@ maybe_finish (struct Operation *op)
     {
       op->state->phase = PHASE_DONE;
       send_client_done (op);
-      destroy_channel (op);
+      _GSS_operation_destroy2 (op);
     }
   }
 }
@@ -1896,7 +1877,7 @@ handle_union_p2p_full_done (void *cls,
       op->state->phase = PHASE_DONE;
       GNUNET_CADET_receive_done (op->channel);
       send_client_done (op);
-      destroy_channel (op);
+      _GSS_operation_destroy2 (op);
       return;
     }
     break;