Fix cleanup issues and some leak

3 files changed, 54 insertions, 31 deletions

diff --git a/src/cadet/cadet.conf.in b/src/cadet/cadet.conf.in
index 86ba2e535..296a648e2 100644
--- a/src/cadet/cadet.conf.in
+++ b/src/cadet/cadet.conf.in
@@ -4,7 +4,7 @@ AUTOSTART = @AUTOSTART@
 @JAVAPORT@PORT = 2096
 HOSTNAME = localhost
 BINARY = gnunet-service-cadet-new
-# PREFIX = valgrind --leak-check=yes
+PREFIX = valgrind --leak-check=yes
 ACCEPT_FROM = 127.0.0.1;
 ACCEPT_FROM6 = ::1;
 UNIXPATH = $GNUNET_RUNTIME_DIR/gnunet-service-cadet.sock
diff --git a/src/cadet/gnunet-service-cadet-new.c b/src/cadet/gnunet-service-cadet-new.c
index 1f28745e1..b16767fbb 100644
--- a/src/cadet/gnunet-service-cadet-new.c
+++ b/src/cadet/gnunet-service-cadet-new.c
@@ -1238,14 +1238,14 @@ channel_destroy_iterator (void *cls,
        "Destroying %s, due to %s disconnecting.\n",
        GCCH_2s (ch),
        GSC_2s (c));
-  GNUNET_assert (GNUNET_YES ==
-                 GNUNET_CONTAINER_multihashmap32_remove (c->channels,
-                                                         key,
-                                                         ch));
   ccn.channel_of_client = htonl (key);
   GCCH_channel_local_destroy (ch,
                               c,
                               ccn);
+  GNUNET_assert (GNUNET_YES ==
+                 GNUNET_CONTAINER_multihashmap32_remove (c->channels,
+                                                         key,
+                                                         ch));
   return GNUNET_OK;
 }
 
@@ -1304,6 +1304,7 @@ client_disconnect_cb (void *cls,
     GNUNET_CONTAINER_multihashmap32_iterate (c->channels,
                                              &channel_destroy_iterator,
                                              c);
+    GNUNET_assert (0 == GNUNET_CONTAINER_multihashmap32_size (c->channels));
     GNUNET_CONTAINER_multihashmap32_destroy (c->channels);
   }
   if (NULL != c->ports)
diff --git a/src/cadet/gnunet-service-cadet-new_channel.c b/src/cadet/gnunet-service-cadet-new_channel.c
index 828c3daa7..9f565a7f4 100644
--- a/src/cadet/gnunet-service-cadet-new_channel.c
+++ b/src/cadet/gnunet-service-cadet-new_channel.c
@@ -661,11 +661,8 @@ GCCH_channel_local_new (struct CadetClient *owner,
     }
     else
     {
-      ch->dest = GNUNET_new (struct CadetChannelClient);
-      ch->dest->c = c;
-      ch->dest->client_ready = GNUNET_YES;
       GCCH_bind (ch,
-                 ch->dest->c);
+                 c);
     }
   }
   else
@@ -962,6 +959,7 @@ GCCH_bind (struct CadetChannel *ch,
   if (ch->out_of_order)
     options |= GNUNET_CADET_OPTION_OUT_OF_ORDER;
   cccd = GNUNET_new (struct CadetChannelClient);
+  GNUNET_assert (NULL == ch->dest);
   ch->dest = cccd;
   cccd->c = c;
   cccd->client_ready = GNUNET_YES;
@@ -1000,6 +998,28 @@ GCCH_bind (struct CadetChannel *ch,
 
 
 /**
+ * One of our clients has disconnected, tell the other one that we
+ * are finished. Done asynchronously to avoid concurrent modification
+ * issues if this is the same client.
+ *
+ * @param cls the `struct CadetChannel` where one of the ends is now dead
+ */
+static void
+signal_remote_destroy_cb (void *cls)
+{
+  struct CadetChannel *ch = cls;
+  struct CadetChannelClient *ccc;
+
+  /* Find which end is left... */
+  ccc = (NULL != ch->owner) ? ch->owner : ch->dest;
+  GSC_handle_remote_channel_destroy (ccc->c,
+                                     ccc->ccn,
+                                     ch);
+  channel_destroy (ch);
+}
+
+
+/**
  * Destroy locally created channel.  Called by the local client, so no
  * need to tell the client.
  *
@@ -1052,32 +1072,34 @@ GCCH_channel_local_destroy (struct CadetChannel *ch,
     ch->destroy = GNUNET_YES;
     return;
   }
-  if (GNUNET_YES == ch->is_loopback)
+  if ( (GNUNET_YES == ch->is_loopback) &&
+       ( (NULL != ch->owner) ||
+         (NULL != ch->dest) ) )
   {
-    struct CadetChannelClient *ccc;
-
-    /* Find which end is left... */
-    ccc = (NULL != ch->owner) ? ch->owner : ch->dest;
-    GSC_handle_remote_channel_destroy (ccc->c,
-                                       ccc->ccn,
-                                       ch);
-    channel_destroy (ch);
+    if (NULL != ch->retry_control_task)
+      GNUNET_SCHEDULER_cancel (ch->retry_control_task);
+    ch->retry_control_task
+      = GNUNET_SCHEDULER_add_now (&signal_remote_destroy_cb,
+                                  ch);
     return;
   }
-  /* If the we ever sent the CHANNEL_CREATE, we need to send a destroy message. */
-  switch (ch->state)
+  if (GNUNET_NO == ch->is_loopback)
   {
-  case CADET_CHANNEL_NEW:
-    /* We gave up on a channel that we created as a client to a remote
-       target, but that never went anywhere. Nothing to do here. */
-    break;
-  case CADET_CHANNEL_LOOSE:
-    GSC_drop_loose_channel (&ch->port,
-                            ch);
-    break;
-  default:
-    GCT_send_channel_destroy (ch->t,
-                              ch->ctn);
+    /* If the we ever sent the CHANNEL_CREATE, we need to send a destroy message. */
+    switch (ch->state)
+    {
+    case CADET_CHANNEL_NEW:
+      /* We gave up on a channel that we created as a client to a remote
+         target, but that never went anywhere. Nothing to do here. */
+      break;
+    case CADET_CHANNEL_LOOSE:
+      GSC_drop_loose_channel (&ch->port,
+                              ch);
+      break;
+    default:
+      GCT_send_channel_destroy (ch->t,
+                                ch->ctn);
+    }
   }
   /* Nothing left to do, just finish destruction */
   channel_destroy (ch);