From 564df39557f1a69280f36ff5173807856ff41067 Mon Sep 17 00:00:00 2001
From: =?utf8?q?Julius=20B=C3=BCnger?= <buenger@mytum.de>
Date: Mon, 15 Oct 2018 23:11:31 +0200
Subject: [PATCH] RPS: Cleanup datastructures, NULL out freed pointers

---
 src/rps/rps-sampler_common.c | 19 ++++++++++++++++++-
 src/rps/rps_api.c            | 32 +++++++++++++++++---------------
 src/rps/test_rps.conf        |  2 +-
 3 files changed, 36 insertions(+), 17 deletions(-)

diff --git a/src/rps/rps-sampler_common.c b/src/rps/rps-sampler_common.c
index d004c06a5..5dbb5315a 100644
--- a/src/rps/rps-sampler_common.c
+++ b/src/rps/rps-sampler_common.c
@@ -387,6 +387,10 @@ check_n_peers_ready (void *cls,
 {
   struct RPS_SamplerRequestHandle *req_handle = cls;
   (void) id;
+  RPS_sampler_n_rand_peers_ready_cb tmp_cb;
+  struct GNUNET_PeerIdentity *peers;
+  uint32_t num_peers;
+  void *cb_cls;
 
   req_handle->cur_num_peers++;
   LOG (GNUNET_ERROR_TYPE_DEBUG,
@@ -400,9 +404,20 @@ check_n_peers_ready (void *cls,
     LOG (GNUNET_ERROR_TYPE_DEBUG,
         "returning %" PRIX32 " peers to the client\n",
         req_handle->num_peers);
-    req_handle->callback (req_handle->ids, req_handle->num_peers, req_handle->cls);
 
+    /* Copy pointers and peers temporarily as they
+     * might be deleted from within the callback */
+    tmp_cb = req_handle->callback;
+    num_peers = req_handle->num_peers;
+    peers = GNUNET_new_array (num_peers, struct GNUNET_PeerIdentity);
+    GNUNET_memcpy (peers,
+                   req_handle->ids,
+                   num_peers * sizeof (struct GNUNET_PeerIdentity));
+    cb_cls = req_handle->cls;
     RPS_sampler_request_cancel (req_handle);
+    req_handle = NULL;
+    tmp_cb (peers, num_peers, cb_cls);
+    GNUNET_free (peers);
   }
 }
 
@@ -493,10 +508,12 @@ RPS_sampler_request_cancel (struct RPS_SamplerRequestHandle *req_handle)
                                    req_handle->sampler->notify_ctx_tail,
                                    i->notify_ctx);
       GNUNET_free (i->notify_ctx);
+      i->notify_ctx = NULL;
     }
     GNUNET_free (i);
   }
   GNUNET_free (req_handle->ids);
+  req_handle->ids = NULL;
   GNUNET_CONTAINER_DLL_remove (req_handle->sampler->req_handle_head,
                                req_handle->sampler->req_handle_tail,
                                req_handle);
diff --git a/src/rps/rps_api.c b/src/rps/rps_api.c
index 02d833506..bce567678 100644
--- a/src/rps/rps_api.c
+++ b/src/rps/rps_api.c
@@ -237,11 +237,14 @@ peers_ready_cb (const struct GNUNET_PeerIdentity *peers,
 {
   struct GNUNET_RPS_Request_Handle *rh = cls;
 
+  rh->sampler_rh = NULL;
   rh->ready_cb (rh->ready_cb_cls,
                 num_peers,
                 peers);
-  // TODO cleanup, sampler, rh, cancel stuff
-  // TODO screw this function. We can give the cb,cls directly to the sampler.
+  GNUNET_RPS_stream_cancel (rh->srh);
+  rh->srh = NULL;
+  RPS_sampler_destroy (rh->sampler);
+  rh->sampler = NULL;
 }
 
 
@@ -421,18 +424,13 @@ void
 GNUNET_RPS_stream_cancel (struct GNUNET_RPS_StreamRequestHandle *srh)
 {
   struct GNUNET_RPS_Handle *rps_handle;
-  struct GNUNET_RPS_StreamRequestHandle *srh_iter;
 
+  GNUNET_assert (NULL != srh);
   rps_handle = srh->rps_handle;
-  srh_iter = rps_handle->stream_requests_head;
-  while (NULL != srh_iter && srh_iter != srh) srh_iter = srh_iter->next;
-  if (NULL != srh_iter)
-  {
-    GNUNET_CONTAINER_DLL_remove (rps_handle->stream_requests_head,
-                                 rps_handle->stream_requests_tail,
-                                 srh);
-    GNUNET_free (srh);
-  }
+  GNUNET_CONTAINER_DLL_remove (rps_handle->stream_requests_head,
+                               rps_handle->stream_requests_tail,
+                               srh);
+  GNUNET_free (srh);
   if (NULL == rps_handle->stream_requests_head) cancel_stream (rps_handle);
 }
 
@@ -480,20 +478,24 @@ handle_stream_input (void *cls,
   struct GNUNET_RPS_Handle *h = cls;
   const struct GNUNET_PeerIdentity *peers;
   uint64_t num_peers;
+  struct GNUNET_RPS_StreamRequestHandle *srh_iter;
+  struct GNUNET_RPS_StreamRequestHandle *srh_next;
 
   peers = (struct GNUNET_PeerIdentity *) &msg[1];
   num_peers = ntohl (msg->num_peers);
   LOG (GNUNET_ERROR_TYPE_DEBUG,
        "Received %" PRIu64 " peer(s) from stream input.\n",
        num_peers);
-  for (struct GNUNET_RPS_StreamRequestHandle *srh_iter = h->stream_requests_head;
-       NULL != srh_iter;
-       srh_iter = srh_iter->next)
+  srh_iter = h->stream_requests_head;
+  while (NULL != srh_iter)
   {
     LOG (GNUNET_ERROR_TYPE_DEBUG, "Calling srh \n");
+    /* Store next pointer - srh might be removed/freed in callback */
+    srh_next = srh_iter->next;
     srh_iter->ready_cb (srh_iter->ready_cb_cls,
                         num_peers,
                         peers);
+    srh_iter = srh_next;
   }
 
   if (NULL == h->stream_requests_head)
diff --git a/src/rps/test_rps.conf b/src/rps/test_rps.conf
index c55930649..c7ac1f3b8 100644
--- a/src/rps/test_rps.conf
+++ b/src/rps/test_rps.conf
@@ -1,7 +1,7 @@
 [rps]
 #PREFIX = valgrind --leak-check=full --show-leak-kinds=all --log-file=/tmp/rps/valgrind!gnunet-service-rps!%p
 #PREFIX = valgrind --log-file=/tmp/rps/valgrind!gnunet-service-rps!%p
-#PREFIX = valgrind 
+#PREFIX = valgrind
 UNIXPATH = $GNUNET_TMP/gnunet-service-rps.sock
 HOME = $SERVICEHOME
 # PORT = 2106
-- 
2.25.1