From de6c35cb8a78d7e05864316743dc1d204f65631b Mon Sep 17 00:00:00 2001 From: Jordan Augé Date: Tue, 5 Jan 2021 18:05:48 +0100 Subject: [HICN-668] Fix various leaks across codebase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: I114e4e5d3e5bf4feb3d367b3f442c165ee193755 Signed-off-by: Jordan Augé --- .../src/hicn/config/configurationListeners.c | 4 +- hicn-light/src/hicn/core/mapme.c | 61 ++++++++++++++++------ hicn-light/src/hicn/core/name.c | 5 +- hicn-light/src/hicn/core/nameBitvector.c | 4 +- hicn-light/src/hicn/io/hicnListener.c | 5 +- hicn-light/src/hicn/processor/fib.c | 3 ++ hicn-light/src/hicn/strategies/loadBalancer.c | 12 ++--- 7 files changed, 68 insertions(+), 26 deletions(-) diff --git a/hicn-light/src/hicn/config/configurationListeners.c b/hicn-light/src/hicn/config/configurationListeners.c index 21bfe7640..b862b98f4 100644 --- a/hicn-light/src/hicn/config/configurationListeners.c +++ b/hicn-light/src/hicn/config/configurationListeners.c @@ -393,10 +393,12 @@ bool _addHicn(Configuration *config, add_listener_command *control, if (success == true && localAddress != NULL) { if (logger_IsLoggable(configuration_GetLogger(config), LoggerFacility_Config, PARCLogLevel_Info)) { + char * str = addressToString(localAddress); logger_Log(configuration_GetLogger(config), LoggerFacility_Config, PARCLogLevel_Info, __func__, "Setup hicn listener on address %s", - addressToString(localAddress)); + str); + parcMemory_Deallocate((void **)&str); } } diff --git a/hicn-light/src/hicn/core/mapme.c b/hicn-light/src/hicn/core/mapme.c index a22d01ae7..397723a79 100644 --- a/hicn-light/src/hicn/core/mapme.c +++ b/hicn-light/src/hicn/core/mapme.c @@ -146,7 +146,7 @@ ERR_MALLOC: void mapmeTFIB_Release(MapMeTFIB **tfibPtr) { MapMeTFIB *tfib = *tfibPtr; - /* TODO; Release all timers */ + /* TODO; Release all timers: see mapmeTFIB_Remove */ parcHashMap_Release(&tfib->nexthops); free(tfib); *tfibPtr = NULL; @@ -177,9 +177,13 @@ static const PARCEventTimer *mapmeTFIB_Get(const MapMeTFIB *tfib, const PARCBuffer *buffer; PARCUnsigned *cid = parcUnsigned_Create(conn_id); buffer = parcHashMap_Get(tfib->nexthops, cid); - if (!buffer) return NULL; + if (!buffer) { + timer = NULL; + goto END; + } PARCByteArray *array = parcBuffer_Array(buffer); timer = *((PARCEventTimer **)parcByteArray_Array(array)); +END: parcUnsigned_Release(&cid); return timer; } @@ -200,14 +204,37 @@ static void mapmeTFIB_Put(MapMeTFIB *tfib, unsigned conn_id, parcBuffer_Release(&buffer); } -static void mapmeTFIB_Remove(MapMeTFIB *tfib, unsigned conn_id) { - // Who releases the timer ? +static void mapmeTFIB_Remove(const MapMe * mapme, MapMeTFIB *tfib, unsigned conn_id) { PARCUnsigned *cid = parcUnsigned_Create(conn_id); + + /* Release timer */ + const PARCBuffer *buffer = parcHashMap_Get(tfib->nexthops, cid); + if (buffer) { + PARCByteArray *array = parcBuffer_Array(buffer); + PARCEventTimer * timer = *((PARCEventTimer **)parcByteArray_Array(array)); + if (timer) { + Dispatcher *dispatcher = forwarder_GetDispatcher(mapme->forwarder); + dispatcher_DestroyTimerEvent(dispatcher, &timer); + } + } + parcHashMap_Remove(tfib->nexthops, cid); 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); } @@ -368,7 +395,7 @@ static bool mapme_setFacePending(const MapMe *mapme, const Name *name, PARCEventTimer *oldTimer = (PARCEventTimer *)mapmeTFIB_Get(TFIB(fibEntry), conn_id); if (oldTimer) parcEventTimer_Stop(oldTimer); - mapmeTFIB_Remove(TFIB(fibEntry), conn_id); + mapmeTFIB_Remove(mapme, TFIB(fibEntry), conn_id); } numberSet_Release(&conns); @@ -402,6 +429,7 @@ 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"); } @@ -743,15 +771,18 @@ static bool mapme_onSpecialInterest(const MapMe *mapme, INFO(mapme, "[MAP-Me] - (1/3) processing prev hops"); if (params->type == UPDATE) { PARCIterator *iterator = mapmeTFIB_CreateKeyIterator(TFIB(fibEntry)); - while (parcIterator_HasNext(iterator)) { - PARCUnsigned *cid = parcIterator_Next(iterator); - unsigned conn_id = parcUnsigned_GetUnsigned(cid); - INFO(mapme, "[MAP-Me] - Re-sending IU to pending connection %d", - conn_id); - mapme_setFacePending(mapme, fibEntry_GetPrefix(fibEntry), fibEntry, - conn_id, false, false, false, 0); + 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); + INFO(mapme, "[MAP-Me] - Re-sending IU to pending connection %d", + conn_id); + mapme_setFacePending(mapme, fibEntry_GetPrefix(fibEntry), fibEntry, + conn_id, false, false, false, 0); + } + parcIterator_Release(&iterator); } - parcIterator_Release(&iterator); } /* nextHops -> prevHops @@ -791,7 +822,7 @@ static bool mapme_onSpecialInterest(const MapMe *mapme, INFO(mapme, "[MAP-Me] - Canceled pending timer"); parcEventTimer_Stop(oldTimer); } - mapmeTFIB_Remove(TFIB(fibEntry), conn_in_id); + mapmeTFIB_Remove(mapme, TFIB(fibEntry), conn_in_id); /* Remove all next hops */ for (size_t k = 0; k < numberSet_Length(nexthops); k++) { @@ -934,7 +965,7 @@ void mapme_onSpecialInterestAck(const MapMe *mapme, const uint8_t *msgBuffer, /* Stop timer and remove entry from TFIB */ parcEventTimer_Stop(timer); - mapmeTFIB_Remove(TFIB(fibEntry), conn_in_id); + mapmeTFIB_Remove(mapme, TFIB(fibEntry), conn_in_id); INFO(mapme, "[MAP-Me] - Removing TFIB entry for ack on connection %d", conn_in_id); diff --git a/hicn-light/src/hicn/core/name.c b/hicn-light/src/hicn/core/name.c index 805e7bfae..b4a5e8d1b 100644 --- a/hicn-light/src/hicn/core/name.c +++ b/hicn-light/src/hicn/core/name.c @@ -239,8 +239,9 @@ char *name_ToString(const Name *name) { Address *packetAddr = nameBitvector_ToAddress(name_GetContentName(name)); - sprintf(output, "name: %s seq: %u", addressToString(packetAddr), - name->segment); + char * address_str = addressToString(packetAddr); + sprintf(output, "name: %s seq: %u", address_str, name->segment); + parcMemory_Deallocate((void **)&address_str); addressDestroy(&packetAddr); diff --git a/hicn-light/src/hicn/core/nameBitvector.c b/hicn-light/src/hicn/core/nameBitvector.c index 6ad623b14..cc13bf610 100644 --- a/hicn-light/src/hicn/core/nameBitvector.c +++ b/hicn-light/src/hicn/core/nameBitvector.c @@ -354,7 +354,9 @@ char *nameBitvector_ToString(const NameBitvector *name) { Address *packetAddr = nameBitvector_ToAddress(name); - sprintf(output, "prefix: %s len: %u", addressToString(packetAddr), name->len); + char * str = addressToString(packetAddr); + sprintf(output, "prefix: %s len: %u", str, name->len); + parcMemory_Deallocate((void **)&str); addressDestroy(&packetAddr); diff --git a/hicn-light/src/hicn/io/hicnListener.c b/hicn-light/src/hicn/io/hicnListener.c index 7d911b18a..8647a4d54 100644 --- a/hicn-light/src/hicn/io/hicnListener.c +++ b/hicn-light/src/hicn/io/hicnListener.c @@ -216,7 +216,10 @@ static void _hicnListener_readcb(int fd, PARCEventType what, void *listener_void static bool _isEmptyAddressIPv4(Address *address) { bool res = false; - if (strcmp("inet4://0.0.0.0:1234", addressToString(address)) == 0) res = true; + char * str = addressToString(address); + if (strcmp("inet4://0.0.0.0:1234", str) == 0) res = true; + parcMemory_Deallocate((void**)&str); + return res; } diff --git a/hicn-light/src/hicn/processor/fib.c b/hicn-light/src/hicn/processor/fib.c index 8822134fe..2f4dca9a3 100644 --- a/hicn-light/src/hicn/processor/fib.c +++ b/hicn-light/src/hicn/processor/fib.c @@ -431,6 +431,9 @@ void fib_Remove(FIB *fib, const Name *name, unsigned connId) { _removeNode(fib, name); #endif /* WITH_MAPME */ + // XXX We never release the FIB entry here it seems, including the inner + // prefix + } void _removeConnectionId(FibNode *n, unsigned connectionId, diff --git a/hicn-light/src/hicn/strategies/loadBalancer.c b/hicn-light/src/hicn/strategies/loadBalancer.c index 878a58515..bba398b02 100644 --- a/hicn-light/src/hicn/strategies/loadBalancer.c +++ b/hicn-light/src/hicn/strategies/loadBalancer.c @@ -355,14 +355,14 @@ static void _strategyLoadBalancer_ImplDestroy(StrategyImpl **strategyPtr) { parcObject_Release((void **) &state); } parcIterator_Release(&it); + } - parcHashMap_Release(&(strategy->strategy_state)); + parcHashMap_Release(&(strategy->strategy_state)); #ifndef WITH_POLICY - numberSet_Release(&(strategy->nexthops)); + numberSet_Release(&(strategy->nexthops)); #endif /* ! WITH_POLICY */ - parcMemory_Deallocate((void **) &strategy); - parcMemory_Deallocate((void **) &impl); - *strategyPtr = NULL; - } + parcMemory_Deallocate((void **) &strategy); + parcMemory_Deallocate((void **) &impl); + *strategyPtr = NULL; } -- cgit 1.2.3-korg