[HICN-668] Fix leaks + double free

Change-Id: I88795e5dc2a55df7ffee5cf66a9bd4fa5652e353
Signed-off-by: Jordan Augé <jordan.auge+fdio@cisco.com>

5 files changed, 96 insertions, 110 deletions

diff --git a/hicn-light/src/hicn/command_line/daemon/hicnLightDaemon_main.c b/hicn-light/src/hicn/command_line/daemon/hicnLightDaemon_main.c
index 89a80d0b1..ab7039f5e 100644
--- a/hicn-light/src/hicn/command_line/daemon/hicnLightDaemon_main.c
+++ b/hicn-light/src/hicn/command_line/daemon/hicnLightDaemon_main.c
@@ -374,6 +374,7 @@ int main(int argc, const char *argv[]) {
     logger_Log(logger, LoggerFacility_Core, PARCLogLevel_Error, "daemon",
                "Forwarder initialization failed. Are you running it with sudo "
                "privileges?");
+    logger_Release(&logger);
     return -1;
   }
 
@@ -402,9 +403,9 @@ int main(int argc, const char *argv[]) {
   forwarder_Destroy(&forwarder);
 
 #ifndef _WIN32
-  sleep(2);
+  //sleep(2);
 #else
-  Sleep(2000);
+  //Sleep(2000);
   WSACleanup();
 #endif
 
diff --git a/hicn-light/src/hicn/core/mapme.c b/hicn-light/src/hicn/core/mapme.c
index 4ee3d5bff..2a64833a8 100644
--- a/hicn-light/src/hicn/core/mapme.c
+++ b/hicn-light/src/hicn/core/mapme.c
@@ -144,21 +144,31 @@ ERR_MALLOC:
   return NULL;
 }
 
-void mapmeTFIB_Release(MapMeTFIB **tfibPtr) {
-  MapMeTFIB *tfib = *tfibPtr;
-  /* TODO; Release all timers: see mapmeTFIB_Remove */
-  parcHashMap_Release(&tfib->nexthops);
-  free(tfib);
-  *tfibPtr = NULL;
+static PARCIterator *mapmeTFIB_CreateKeyIterator(const MapMeTFIB *tfib) {
+  /*
+   * Creating an iterator on an empty HashMap seems to raise an exception
+   * due to :
+   *   parcTrapOutOfMemoryIf(state->listIterator == NULL,
+   *     "Cannot create parcLinkedList_CreateIterator");
+   * in : _parcHashMap_Init
+   *
+   * All buckets are empty, as they are after creation, and as they should be,
+   * but the error is triggered.
+   */
+  if (parcHashMap_Size(tfib->nexthops) == 0)
+      return NULL;
+  return parcHashMap_CreateKeyIterator(tfib->nexthops);
 }
 
+void mapmeTFIB_Release(const MapMe * mapme, MapMeTFIB **tfibPtr);
+
 /**
  * @function mapme_CreateTFIB
  * @abstract Associate a new TFIB entry to a FIB entry.
  * @param [in] - Pointer to the FIB entry.
  * @return Boolean indicating the success of the operation.
  */
-static void mapme_CreateTFIB(FibEntry *fibEntry) {
+static void mapme_CreateTFIB(const MapMe * mapme, FibEntry *fibEntry) {
   MapMeTFIB *tfib;
 
   /* Make sure we don't already have an associated TFIB entry */
@@ -166,7 +176,7 @@ static void mapme_CreateTFIB(FibEntry *fibEntry) {
   // assertNull(tfib);
 
   tfib = mapmeTFIB_Create();
-  fibEntry_setUserData(fibEntry, tfib, (void (*)(void **))mapmeTFIB_Release);
+  fibEntry_setUserData(fibEntry, mapme, tfib, (void (*)(const void*, void**))mapmeTFIB_Release);
 }
 
 #define TFIB(fibEntry) ((MapMeTFIB *)fibEntry_getUserData(fibEntry))
@@ -197,9 +207,9 @@ static void mapmeTFIB_Put(MapMeTFIB *tfib, unsigned conn_id,
    * syntax...
    */
   PARCUnsigned *cid = parcUnsigned_Create(conn_id);
-  PARCBuffer *buffer =
-      parcBuffer_CreateFromArray(&timer, sizeof(PARCEventTimer *));
+  PARCBuffer *buffer = parcBuffer_CreateFromArray(&timer, sizeof(PARCEventTimer *));
   parcHashMap_Put(tfib->nexthops, cid, buffer);
+
   parcUnsigned_Release(&cid);
   parcBuffer_Release(&buffer);
 }
@@ -213,6 +223,7 @@ static void mapmeTFIB_Remove(const MapMe * mapme, MapMeTFIB *tfib, unsigned conn
     PARCByteArray *array = parcBuffer_Array(buffer);
     PARCEventTimer * timer = *((PARCEventTimer **)parcByteArray_Array(array));
     if (timer) {
+      parcEventTimer_Stop(timer);
       Dispatcher *dispatcher = forwarder_GetDispatcher(mapme->forwarder);
       dispatcher_DestroyTimerEvent(dispatcher, &timer);
     }
@@ -222,22 +233,27 @@ static void mapmeTFIB_Remove(const MapMe * mapme, MapMeTFIB *tfib, unsigned conn
   parcUnsigned_Release(&cid);
 }
 
-static PARCIterator *mapmeTFIB_CreateKeyIterator(const MapMeTFIB *tfib) {
-  /*
-   * Creating an iterator on an empty HashMap seems to raise an exception
-   * due to :
-   *   parcTrapOutOfMemoryIf(state->listIterator == NULL,
-   *     "Cannot create parcLinkedList_CreateIterator");
-   * in : _parcHashMap_Init
-   *
-   * All buckets are empty, as they are after creation, and as they should be,
-   * but the error is triggered.
-   */
-  if (parcHashMap_Size(tfib->nexthops) == 0)
-      return NULL;
-  return parcHashMap_CreateKeyIterator(tfib->nexthops);
+void mapmeTFIB_Release(const MapMe * mapme, MapMeTFIB **tfibPtr) {
+  MapMeTFIB *tfib = *tfibPtr;
+
+  /* Release all TFIB entries, incl. timers */
+  PARCIterator *iterator = mapmeTFIB_CreateKeyIterator(tfib);
+  if (iterator) {
+    /* No iterator is created if the TFIB is empty */
+    while (parcIterator_HasNext(iterator)) {
+      PARCUnsigned *cid = parcIterator_Next(iterator);
+      unsigned conn_id = parcUnsigned_GetUnsigned(cid);
+      mapmeTFIB_Remove(mapme, tfib, conn_id);
+    }
+    parcIterator_Release(&iterator);
+  }
+
+  parcHashMap_Release(&tfib->nexthops);
+  free(tfib);
+  *tfibPtr = NULL;
 }
 
+
 int hicn_prefix_from_name(const Name *name, hicn_prefix_t *prefix) {
   NameBitvector *bv = name_GetContentName(name);
   ip_prefix_t ip_prefix;
@@ -250,15 +266,11 @@ int hicn_prefix_from_name(const Name *name, hicn_prefix_t *prefix) {
 static Message *mapme_createMessage(const MapMe *mapme, const Name *name,
                                     mapme_params_t *params) {
   Ticks now = forwarder_GetTicks(mapme->forwarder);
-  Logger *logger = logger_Acquire(forwarder_GetLogger(mapme->forwarder));
+  Logger *logger = forwarder_GetLogger(mapme->forwarder);
 
   INFO(mapme, "[MAP-Me] CreateMessage type=%d seq=%d", params->type,
        params->seq);
 
-  size_t size = (params->protocol == IPPROTO_IPV6) ? HICN_MAPME_V6_HDRLEN
-                                                   : HICN_MAPME_V4_HDRLEN;
-  uint8_t *icmp_pkt = parcMemory_AllocateAndClear(size);
-
   hicn_prefix_t prefix;
   int rc = hicn_prefix_from_name(name, &prefix);
   if (rc < 0) {
@@ -266,6 +278,11 @@ static Message *mapme_createMessage(const MapMe *mapme, const Name *name,
     goto ERR_NAME;
   }
 
+
+  size_t size = (params->protocol == IPPROTO_IPV6) ? HICN_MAPME_V6_HDRLEN
+                                                   : HICN_MAPME_V4_HDRLEN;
+  uint8_t *icmp_pkt = parcMemory_AllocateAndClear(size);
+
   INFO(mapme, "[MAP-Me] Creating MAP-Me packet");
   size_t len = hicn_mapme_create_packet(icmp_pkt, &prefix, params);
   if (len == 0) {
@@ -279,6 +296,7 @@ static Message *mapme_createMessage(const MapMe *mapme, const Name *name,
                                      MessagePacketType_Interest, now, logger);
 
 ERR_CREATE:
+  parcMemory_Deallocate(&icmp_pkt);
 ERR_NAME:
   return NULL;
 }
@@ -287,7 +305,7 @@ static Message *mapme_createAckMessage(const MapMe *mapme,
                                        const uint8_t *msgBuffer,
                                        const mapme_params_t *params) {
   Ticks now = forwarder_GetTicks(mapme->forwarder);
-  Logger *logger = logger_Acquire(forwarder_GetLogger(mapme->forwarder));
+  Logger *logger = forwarder_GetLogger(mapme->forwarder);
 
   size_t size = (params->protocol == IPPROTO_IPV6) ? HICN_MAPME_V6_HDRLEN
                                                    : HICN_MAPME_V4_HDRLEN;
@@ -297,11 +315,15 @@ static Message *mapme_createAckMessage(const MapMe *mapme,
   size_t len = hicn_mapme_create_ack(icmp_pkt, params);
   if (len != size) {
     ERR(mapme, "[MAP-Me] Failed to create mapme ack packet through lib");
-    return NULL;
+    goto ERR;
   }
 
   return message_CreateFromByteArray(
       NO_INGRESS, icmp_pkt, MessagePacketType_ContentObject, now, logger);
+
+ERR:
+  parcMemory_Deallocate(&icmp_pkt);
+  return NULL;
 }
 
 struct setFacePendingArgs {
@@ -329,6 +351,7 @@ static void mapme_setFacePendingCallback(int fd, PARCEventType which_event,
   INFO(args->mapme, "Timeout during retransmission. Re-sending");
   mapme_setFacePending(args->mapme, args->name, args->fibEntry, args->conn_id,
                        args->send, args->is_producer, false, args->num_retx);
+  free(args);
 }
 
 /**
@@ -392,9 +415,6 @@ static bool mapme_setFacePending(const MapMe *mapme, const Name *name,
 
       for (size_t i = 0; i < numberSet_Length(conns); i++) {
         unsigned conn_id = numberSet_GetItem(conns, i);
-        PARCEventTimer *oldTimer = (PARCEventTimer *)mapmeTFIB_Get(TFIB(fibEntry), conn_id);
-        if (oldTimer)
-          parcEventTimer_Stop(oldTimer);
         mapmeTFIB_Remove(mapme, TFIB(fibEntry), conn_id);
       }
 
@@ -429,17 +449,18 @@ static bool mapme_setFacePending(const MapMe *mapme, const Name *name,
               name_str, params.seq, conn_id);
       free(name_str);
       connection_ReSend(conn, special_interest, NOT_A_NOTIFICATION);
-      parcMemory_Deallocate((void**)&special_interest);
     } else {
       INFO(mapme, "[MAP-Me] Stopped retransmissions as face went down");
     }
+    message_Release(&special_interest);
 
     if (num_retx < MAX_RETX) {
       INFO(mapme, "[MAP-Me]   - Scheduling retransmission\n");
       /* Schedule retransmission */
       struct setFacePendingArgs *args =
           malloc(sizeof(struct setFacePendingArgs));
-      if (!args) goto ERR_MALLOC;
+      if (!args)
+          goto ERR_MALLOC;
       args->mapme = mapme;
       args->name = name;
       args->fibEntry = fibEntry;
@@ -453,7 +474,10 @@ static bool mapme_setFacePending(const MapMe *mapme, const Name *name,
       struct timeval timeout = {mapme->retx / 1000,
                                 (mapme->retx % 1000) * 1000};
       rc = parcEventTimer_Start(timer, &timeout);
-      if (rc < 0) goto ERR_TIMER;
+      if (rc < 0) {
+          free(args);
+          goto ERR_TIMER;
+      }
     } else {
       INFO(mapme, "[MAP-Me] Last retransmission.");
       timer = NULL;
@@ -463,19 +487,14 @@ static bool mapme_setFacePending(const MapMe *mapme, const Name *name,
     timer = NULL;
   }
 
-  PARCEventTimer *oldTimer =
-      (PARCEventTimer *)mapmeTFIB_Get(TFIB(fibEntry), conn_id);
-  if (oldTimer) {
-    INFO(mapme, "[MAP-Me]   - Found old timer, would need to cancel !");
-    // parcEventTimer_Stop(oldTimer);
-  }
-  INFO(mapme, "[MAP-Me]   - Putting new timer in TFIB");
-  if (timer) mapmeTFIB_Put(TFIB(fibEntry), conn_id, timer);
+  mapmeTFIB_Remove(mapme, TFIB(fibEntry), conn_id);
+  mapmeTFIB_Put(TFIB(fibEntry), conn_id, timer);
 
   return true;
 
-ERR_MALLOC:
 ERR_TIMER:
+  dispatcher_DestroyTimerEvent(dispatcher, &timer);
+ERR_MALLOC:
   return false;
 }
 
@@ -511,7 +530,7 @@ void
 mapme_send_updates(const MapMe * mapme, FibEntry * fibEntry, const NumberSet * nexthops)
 {
   if (!TFIB(fibEntry)) /* Create TFIB associated to FIB entry */
-    mapme_CreateTFIB(fibEntry);
+    mapme_CreateTFIB(mapme, fibEntry);
   TFIB(fibEntry)->seq++;
 
   const Name *name = fibEntry_GetPrefix(fibEntry);
@@ -617,39 +636,6 @@ mapme_onConnectionEvent(const MapMe *mapme, const Connection *conn_added, connec
   INFO(mapme, "[MAP-Me] Done");
 }
 
-#if 0
-#ifdef WITH_POLICY
-void mapme_onPolicyUpdate(const MapMe *mapme, const Connection *conn_selected, FibEntry * fibEntry)
-{
-  /* Ignore local connections corresponding to applications for now */
-  if (connection_IsLocal(conn_selected))
-      return;
-
-  unsigned conn_selected_id = connection_GetConnectionId(conn_selected);
-  INFO(mapme, "[MAP-Me] New connection %d", conn_selected_id);
-
-  const Name *name = fibEntry_GetPrefix(fibEntry);
-
-  /* Skip entries that have no local connection as next hop */
-  if (!mapme_hasLocalNextHops(mapme, fibEntry))
-      return;
-
-  /* This entry corresponds to a locally served prefix, set
-   * Special Interest */
-  if (!TFIB(fibEntry)) /* Create TFIB associated to FIB entry */
-    mapme_CreateTFIB(fibEntry);
-  TFIB(fibEntry)->seq++;
-
-  char *name_str = name_ToString(name);
-  INFO(mapme, "[MAP-Me] sending IU/IN for name %s on connection %d", name_str,
-       conn_selected_id);
-  free(name_str);
-
-  mapme_setFacePending(mapme, name, fibEntry, conn_selected_id, true, true, true, 0);
-}
-#endif /* WITH_POLICY */
-#endif
-
 /*------------------------------------------------------------------------------
  * Special Interest handling
  *----------------------------------------------------------------------------*/
@@ -684,11 +670,18 @@ static bool mapme_onSpecialInterest(const MapMe *mapme,
    * We always ack, even duplicates.
    */
   Message *ack = mapme_createAckMessage(mapme, msgBuffer, params);
-  if (!ack) goto ERR_ACK_CREATE;
+  if (!ack) {
+    name_Release(&name);
+    return false;
+  }
   rv = connection_ReSend(conn_in, ack, NOT_A_NOTIFICATION);
-  if (!rv) goto ERR_ACK_SEND;
   message_Release(&ack);
 
+  if (!rv) {
+    name_Release(&name);
+    return false;
+  }
+
   /* EPM on FIB */
   /* only the processor has access to the FIB */
   FIB *fib = forwarder_getFib(mapme->forwarder);
@@ -722,7 +715,7 @@ static bool mapme_onSpecialInterest(const MapMe *mapme,
     fibEntry = fibEntry_Create(name, fwdStrategy);
 #endif /* WITH_POLICY */
     FibEntry *lpm = fib_MatchName(fib, name);
-    mapme_CreateTFIB(fibEntry);
+    mapme_CreateTFIB(mapme, fibEntry);
     fib_Add(fib, fibEntry);
     if (!lpm) {
       TFIB(fibEntry)->seq = seq;
@@ -745,7 +738,7 @@ static bool mapme_onSpecialInterest(const MapMe *mapme,
     /* Create TFIB associated to FIB entry */
     INFO(mapme,
          "[MAP-Me]   - Creating TFIB entry with default sequence number");
-    mapme_CreateTFIB(fibEntry);
+    mapme_CreateTFIB(mapme, fibEntry);
   }
 
   /*
@@ -816,15 +809,6 @@ static bool mapme_onSpecialInterest(const MapMe *mapme,
      */
 
     INFO(mapme, "[MAP-Me]   - (3/3) next hops ~~> prev hops");
-    PARCEventTimer *oldTimer =
-        (PARCEventTimer *)mapmeTFIB_Get(TFIB(fibEntry), conn_in_id);
-    if (oldTimer) {
-      /* This happens if we receive an IU while we are still sending
-       * one in the other direction
-       */
-      INFO(mapme, "[MAP-Me]   - Canceled pending timer");
-      parcEventTimer_Stop(oldTimer);
-    }
     mapmeTFIB_Remove(mapme, TFIB(fibEntry), conn_in_id);
 
     /* Remove all next hops */
@@ -898,11 +882,6 @@ static bool mapme_onSpecialInterest(const MapMe *mapme,
   }
 
   return true;
-
-ERR_ACK_SEND:
-  message_Release(&ack);
-ERR_ACK_CREATE:
-  return false;
 }
 
 void mapme_onSpecialInterestAck(const MapMe *mapme, const uint8_t *msgBuffer,
@@ -910,7 +889,7 @@ void mapme_onSpecialInterestAck(const MapMe *mapme, const uint8_t *msgBuffer,
                                 mapme_params_t *params) {
   INFO(mapme, "[MAP-Me] Receive IU/IN Ack on connection %d", conn_in_id);
 
-  const Name * name =
+  Name * name =
       name_CreateFromPacket(msgBuffer, MessagePacketType_ContentObject);
   name_setLen((Name*) name, prefix->len);
   char * name_str = name_ToString(name);
@@ -920,6 +899,9 @@ void mapme_onSpecialInterestAck(const MapMe *mapme, const uint8_t *msgBuffer,
 
   FIB *fib = forwarder_getFib(mapme->forwarder);
   FibEntry *fibEntry = fib_Contains(fib, name);
+
+  name_Release(&name);
+
   if (!fibEntry) {
     return;
   }
@@ -967,7 +949,6 @@ void mapme_onSpecialInterestAck(const MapMe *mapme, const uint8_t *msgBuffer,
   }
 
   /* Stop timer and remove entry from TFIB */
-  parcEventTimer_Stop(timer);
   mapmeTFIB_Remove(mapme, TFIB(fibEntry), conn_in_id);
 
   INFO(mapme, "[MAP-Me]   - Removing TFIB entry for ack on connection %d",
diff --git a/hicn-light/src/hicn/processor/fibEntry.c b/hicn-light/src/hicn/processor/fibEntry.c
index 7412b4ccf..30bcc1f9a 100644
--- a/hicn-light/src/hicn/processor/fibEntry.c
+++ b/hicn-light/src/hicn/processor/fibEntry.c
@@ -64,8 +64,9 @@ struct fib_entry {
 #endif /* WITH_POLICY */
 #ifdef WITH_MAPME
   NumberSet * previous_nexthops;
+  const void *userDataOwner;
   void *userData;
-  void (*userDataRelease)(void **userData);
+  void (*userDataRelease)(const void *owner, void **userData);
 #endif /* WITH_MAPME */
 };
 
@@ -101,6 +102,7 @@ FibEntry *fibEntry_Create(Name *name, strategy_type fwdStrategy) {
   fibEntry->refcount = 1;
 
 #ifdef WITH_MAPME
+  fibEntry->userDataOwner = NULL;
   fibEntry->userData = NULL;
   fibEntry->userDataRelease = NULL;
 #endif /* WITH_MAPME */
@@ -136,7 +138,7 @@ void fibEntry_Release(FibEntry **fibEntryPtr) {
     fibEntry->fwdStrategy->destroy(&(fibEntry->fwdStrategy));
 #ifdef WITH_MAPME
     if (fibEntry->userData) {
-      fibEntry->userDataRelease(&fibEntry->userData);
+      fibEntry->userDataRelease(fibEntry->userDataOwner, &fibEntry->userData);
     }
 #endif /* WITH_MAPME */
 #ifdef WITH_POLICY
@@ -881,9 +883,10 @@ void *fibEntry_getUserData(const FibEntry *fibEntry) {
   return fibEntry->userData;
 }
 
-void fibEntry_setUserData(FibEntry *fibEntry, const void *userData,
-                          void (*userDataRelease)(void **)) {
+void fibEntry_setUserData(FibEntry *fibEntry, const void *userDataOwner, const
+        void *userData, void (*userDataRelease)(const void *, void **)) {
   parcAssertNotNull(fibEntry, "Parameter fibEntry must be non-null");
+  fibEntry->userDataOwner = userDataOwner;
   fibEntry->userData = (void *)userData;
   fibEntry->userDataRelease = userDataRelease;
 }
diff --git a/hicn-light/src/hicn/processor/fibEntry.h b/hicn-light/src/hicn/processor/fibEntry.h
index 3f78f47dd..cf267d6a6 100644
--- a/hicn-light/src/hicn/processor/fibEntry.h
+++ b/hicn-light/src/hicn/processor/fibEntry.h
@@ -158,8 +158,8 @@ void *fibEntry_getUserData(const FibEntry *fibEntry);
  * @param [in@ userDataRelease - Callback used to release user data upon change
  *       of FIB entry removal.
  */
-void fibEntry_setUserData(FibEntry *fibEntry, const void *userData,
-                          void (*userDataRelease)(void **));
+void fibEntry_setUserData(FibEntry *fibEntry, const void *userDataOwner,
+        const void *userData, void (*userDataRelease)(const void *, void **));
 
 #endif /* WITH_MAPME */
 
diff --git a/hicn-light/src/hicn/strategies/loadBalancer.c b/hicn-light/src/hicn/strategies/loadBalancer.c
index bba398b02..82b5a6103 100644
--- a/hicn-light/src/hicn/strategies/loadBalancer.c
+++ b/hicn-light/src/hicn/strategies/loadBalancer.c
@@ -216,8 +216,8 @@ static NumberSet *_strategyLoadBalancer_LookupNexthop(
     PARCUnsigned *cid = parcUnsigned_Create(numberSet_GetItem(nexthops, i));
     const StrategyNexthopState *state =
         parcHashMap_Get(lb->strategy_state, cid);
-    if (!state){
-        parcUnsigned_Release(&cid);
+    if (!state) {
+       parcUnsigned_Release(&cid);
        continue;
     }
     distance -= strategyNexthopState_GetWeight(state);
@@ -227,6 +227,7 @@ static NumberSet *_strategyLoadBalancer_LookupNexthop(
       parcUnsigned_Release(&cid);
       break;
     }
+    parcUnsigned_Release(&cid);
   }
 #else
   unsigned in_connection = message_GetIngressConnectionId(interestMessage);