From 281632b8702e22f59e19e57e7be5315ea198559d Mon Sep 17 00:00:00 2001 From: mmichalek Date: Tue, 10 Nov 2015 13:03:10 -0500 Subject: [PATCH] Memory improvements, fix for StringArray when splitting large string. --- symmetric-client-clib-test/src/util/MapTest.c | 20 ++++++++++++++++++ .../src/util/StringArrayTest.c | 18 +++++++++++++++- symmetric-client-clib/src/core/SymEngine.c | 1 - symmetric-client-clib/src/model/Channel.c | 4 ++-- .../src/model/TriggerHistory.c | 21 ------------------- .../src/service/DataService.c | 1 + .../src/service/RouterService.c | 2 +- .../src/service/TriggerRouterService.c | 21 +++++++++++++++---- .../src/transport/http/HttpTransportManager.c | 2 +- symmetric-client-clib/src/util/Date.c | 4 ++-- symmetric-client-clib/src/util/Map.c | 5 +++++ symmetric-client-clib/src/util/StringArray.c | 6 ++++-- 12 files changed, 70 insertions(+), 35 deletions(-) diff --git a/symmetric-client-clib-test/src/util/MapTest.c b/symmetric-client-clib-test/src/util/MapTest.c index fcea6953ca..be048a18b0 100644 --- a/symmetric-client-clib-test/src/util/MapTest.c +++ b/symmetric-client-clib-test/src/util/MapTest.c @@ -302,6 +302,25 @@ void SymMapTest_testRemove() { map->remove(map, "1"); } +void SymMapTest_testResetRemove() { + int mapSize = 16; + SymMap *map = SymMap_new(NULL, mapSize); + int i; + for (i = 0; i < mapSize; i++) { + map->put(map, SymStringUtils_format("%d", i), SymStringUtils_format("value %d", i)); + } + CU_ASSERT(SymStringUtils_equals(map->get(map, "1"), "value 1")); + map->reset(map); + CU_ASSERT(map->get(map, "1") == NULL); + map->remove(map, "1"); + CU_ASSERT(map->get(map, "1") == NULL); + + map->put(map, "1", "value 1"); + CU_ASSERT(SymStringUtils_equals(map->get(map, "1"), "value 1")); + map->remove(map, "1"); + CU_ASSERT(map->get(map, "1") == NULL); +} + int SymMapTest_CUnit() { CU_pSuite suite = CU_add_suite("SymMapTest", NULL, NULL); if (suite == NULL) { @@ -321,6 +340,7 @@ int SymMapTest_CUnit() { CU_add_test(suite, "SymMapTest_testKeysSingleValue", SymMapTest_testKeysSingleValue) == NULL || CU_add_test(suite, "SymMapTest_testKeysMultiValue", SymMapTest_testKeysMultiValue) == NULL || CU_add_test(suite, "SymMapTest_testRemove", SymMapTest_testRemove) == NULL || + CU_add_test(suite, "SymMapTest_testResetRemove", SymMapTest_testResetRemove) == NULL || 1==0 ) { diff --git a/symmetric-client-clib-test/src/util/StringArrayTest.c b/symmetric-client-clib-test/src/util/StringArrayTest.c index ce295d0ebd..58163bcfd3 100644 --- a/symmetric-client-clib-test/src/util/StringArrayTest.c +++ b/symmetric-client-clib-test/src/util/StringArrayTest.c @@ -82,6 +82,19 @@ void SymStringArrayTest_test4() { array->destroy(array); } +void SymStringArrayTest_testSplitLong() { + // This long string was triggering a bug because it splits longer than the initial + // length of the array. + SymStringArray *array = SymStringArray_split("batch-11=ok&nodeId-11=000&network-11=0&filter-11=0&database-11=2&byteCount-11=203&batch-12=ok&nodeId-12=000&network-12=0&filter-12=0&database-12=0&byteCount-12=8&batch-14=ok&nodeId-14=000&network-14=0&filter-14=0&database-14=0&byteCount-14=8&batch-16=ok&nodeId-16=000&network-16=0&filter-16=0&database-16=0&byteCount-16=8&batch-17=ok&nodeId-17=000&network-17=0&filter-17=0&database-17=0&byteCount-17=8&batch-18=ok&nodeId-18=000&network-18=0&filter-18=0&database-18=0&byteCount-18=8&batch-19=ok&nodeId-19=000&network-19=0&filter-19=0&database-19=0&byteCount-19=8&batch-21=ok&nodeId-21=000&network-21=0&filter-21=0&database-21=0&byteCount-21=8&batch-22=ok&nodeId-22=000&network-22=0&filter-22=0&database-22=0&byteCount-22=8&batch-23=ok&nodeId-23=000&network-23=0&filter-23=0&database-23=0&byteCount-23=8&batch-24=ok&nodeId-24=000&network-24=0&filter-24=0&database-24=0&byteCount-24=8&batch-25=ok&nodeId-25=000&network-25=0&filter-25=0&database-25=0&byteCount-25=8&batch-26=ok&nodeId-26=000&network-26=0&filter-26=0&database-26=0&byteCount-26=8&batch-27=ok&nodeId-27=000&network-27=0&filter-27=0&database-27=0&byteCount-27=8&batch-28=ok&nodeId-28=000&network-28=0&filter-28=0&database-28=0&byteCount-28=8&batch-29=ok&nodeId-29=000&network-29=0&filter-29=0&database-29=0&byteCount-29=8&batch-30=ok&nodeId-30=000&network-30=0&filter-30=0&database-30=0&byteCount-30=8&batch-31=ok&nodeId-31=000&network-31=0&filter-31=0&database-31=0&byteCount-31=8&batch-32=ok&nodeId-32=000&network-32=0&filter-32=0&database-32=0&byteCount-32=8&batch-33=ok&nodeId-33=000&network-33=0&filter-33=0&database-33=0&byteCount-33=8", "&"); + + CU_ASSERT(array->size == 120); + + CU_ASSERT(strcmp(array->get(array, 0), "batch-11=ok") == 0); + CU_ASSERT(strcmp(array->get(array, 1), "nodeId-11=000") == 0); + CU_ASSERT(strcmp(array->get(array, 2), "network-11=0") == 0); + + array->destroy(array); +} int SymStringArrayTest_CUnit() { CU_pSuite suite = CU_add_suite("SymStringArrayTest", NULL, NULL); @@ -92,7 +105,10 @@ int SymStringArrayTest_CUnit() { if (CU_add_test(suite, "SymStringArrayTest_test1", SymStringArrayTest_test1) == NULL || CU_add_test(suite, "SymStringArrayTest_test2", SymStringArrayTest_test2) == NULL || CU_add_test(suite, "SymStringArrayTest_test3", SymStringArrayTest_test3) == NULL || - CU_add_test(suite, "SymStringArrayTest_test4", SymStringArrayTest_test4) == NULL) { + CU_add_test(suite, "SymStringArrayTest_test4", SymStringArrayTest_test4) == NULL || + CU_add_test(suite, "SymStringArrayTest_testSplitLong", SymStringArrayTest_testSplitLong) == NULL || + 1==0 + ) { return CUE_NOTEST; } return CUE_SUCCESS; diff --git a/symmetric-client-clib/src/core/SymEngine.c b/symmetric-client-clib/src/core/SymEngine.c index bb3c94c28c..5489afd100 100644 --- a/symmetric-client-clib/src/core/SymEngine.c +++ b/symmetric-client-clib/src/core/SymEngine.c @@ -129,7 +129,6 @@ unsigned short SymEngine_uninstall(SymEngine *this) { void SymEngine_destroy(SymEngine *this) { this->triggerRouterService->destroy(this->triggerRouterService); this->dialect->destroy(this->dialect); - this->platform->destroy(this->platform); this->purgeService->destroy(this->purgeService); this->pushService->destroy(this->pushService); this->pullService->destroy(this->pullService); diff --git a/symmetric-client-clib/src/model/Channel.c b/symmetric-client-clib/src/model/Channel.c index 86db633936..160f5b1b7c 100644 --- a/symmetric-client-clib/src/model/Channel.c +++ b/symmetric-client-clib/src/model/Channel.c @@ -22,10 +22,10 @@ void SymChannel_destroy(SymChannel *this) { if (this->createTime) { - free(this->createTime); + this->createTime->destroy(this->createTime); } if (this->lastUpdateTime) { - free(this->lastUpdateTime); + this->lastUpdateTime->destroy(this->lastUpdateTime); } free(this); } diff --git a/symmetric-client-clib/src/model/TriggerHistory.c b/symmetric-client-clib/src/model/TriggerHistory.c index dec2e58c51..e82054e36e 100644 --- a/symmetric-client-clib/src/model/TriggerHistory.c +++ b/symmetric-client-clib/src/model/TriggerHistory.c @@ -62,30 +62,9 @@ char * SymTriggerHistory_getTriggerNameForDmlType(SymTriggerHistory *this, SymDa } void SymTriggerHistory_destroy(SymTriggerHistory *this) { - // Being selective here based on Valgrind reports and testing that - // shows other fields are static memory or really owned by someone else. - if (this->sourceTableName) { - // free(this->sourceTableName); - } - if (this->nameForInsertTrigger) { - free(this->nameForInsertTrigger); - } - if (this->nameForUpdateTrigger) { - free(this->nameForUpdateTrigger); - } - if (this->nameForDeleteTrigger) { - free(this->nameForDeleteTrigger); - } - if (this->createTime) { this->createTime->destroy(this->createTime); } - if (this->columnNames) { - free(this->columnNames); - } - if (this->pkColumnNames) { - free(this->pkColumnNames); - } if (this->inactiveTime) { this->inactiveTime->destroy(this->inactiveTime); } diff --git a/symmetric-client-clib/src/service/DataService.c b/symmetric-client-clib/src/service/DataService.c index ae546beb04..14ccd7613a 100644 --- a/symmetric-client-clib/src/service/DataService.c +++ b/symmetric-client-clib/src/service/DataService.c @@ -143,6 +143,7 @@ void SymDataService_insertDataEvents(SymDataService *this, SymSqlTransaction *tr args->add(args, dataEvent->routerId); int error; transaction->addRow(transaction, args, NULL, &error); + args->destroy(args); } iter->destroy(iter); } diff --git a/symmetric-client-clib/src/service/RouterService.c b/symmetric-client-clib/src/service/RouterService.c index 30efb6701e..f2e7794ba5 100644 --- a/symmetric-client-clib/src/service/RouterService.c +++ b/symmetric-client-clib/src/service/RouterService.c @@ -161,6 +161,7 @@ static int SymRouterService_routeDataToNodes(SymRouterService *this, SymData *da } } numberOfDataEventsInserted += SymRouterService_insertDataEvents(this, context, dataMetaData, nodeIds); + dataMetaData->destroy(dataMetaData); } } else { SymLog_warn("Could not find trigger routers for trigger history id of %d. There is a good chance that data was captured and the trigger router link was removed before the data could be routed", @@ -237,7 +238,6 @@ static int SymRouterService_routeDataForEachChannel(SymRouterService *this) { } iter->destroy(iter); channelList->destroy(channelList); -// channels->destroy(channels); channels->destroyAll(channels, (void *)SymChannel_destroy); return dataCount; } diff --git a/symmetric-client-clib/src/service/TriggerRouterService.c b/symmetric-client-clib/src/service/TriggerRouterService.c index c5c01fdc28..63e3666a87 100644 --- a/symmetric-client-clib/src/service/TriggerRouterService.c +++ b/symmetric-client-clib/src/service/TriggerRouterService.c @@ -532,6 +532,7 @@ void SymTriggerRouterService_destroyTriggerRouters(SymTriggerRouterService *this } if (shouldFreeRouter && triggerRouter->router) { + free(triggerRouter->router->routerId); triggerRouter->router->destroy(triggerRouter->router); freedRouters->add(freedRouters, triggerRouter->router); triggerRouter->router = NULL; @@ -946,8 +947,10 @@ static unsigned short SymTriggerRouterService_doesTriggerRouterExistInList(SymLi static char * SymTriggerRouterService_buildSymmetricTableRouterId(SymTriggerRouterService *this, char *triggerId, char *sourceNodeGroupId, char *targetNodeGroupId) { - return SymTriggerRouterService_replaceCharsToShortenName(this, SymStringUtils_format("%s_%s_2_%s", - triggerId, sourceNodeGroupId, targetNodeGroupId)); + char *format = SymStringUtils_format("%s_%s_2_%s", triggerId, sourceNodeGroupId, targetNodeGroupId); + char *symmetricTableRouterId = SymTriggerRouterService_replaceCharsToShortenName(this, format); + free(format); + return symmetricTableRouterId; } static SymTriggerRouter * SymTriggerRouterService_buildTriggerRoutersForSymmetricTablesWithTrigger(SymTriggerRouterService *this, SymTrigger *trigger, @@ -961,9 +964,11 @@ static SymTriggerRouter * SymTriggerRouterService_buildTriggerRoutersForSymmetri nodeGroupLink->targetNodeGroupId); router->routerType = SYM_CONFIGURATION_CHANGED_DATA_ROUTER_ROUTER_TYPE; router->nodeGroupLink = nodeGroupLink; - router->lastUpdateTime = trigger->lastUpdateTime; + if (trigger->lastUpdateTime) { + router->lastUpdateTime = SymDate_newWithTime(trigger->lastUpdateTime->time); // Everyone needs their own copy to avoid attempts to free the same memory later. + triggerRouter->lastUpdateTime = SymDate_newWithTime(trigger->lastUpdateTime->time); + } - triggerRouter->lastUpdateTime = trigger->lastUpdateTime; return triggerRouter; } @@ -1012,6 +1017,7 @@ static void SymTriggerRouterService_mergeInConfigurationTablesTriggerRoutersForC configuredInDatabase->add(configuredInDatabase, trigger); } } + virtualConfigTriggers->destroy(virtualConfigTriggers); iter->destroy(iter); } @@ -1062,11 +1068,15 @@ static SymTriggerRoutersCache * SymTriggerRouterService_getTriggerRoutersCacheFo } } iter->destroy(iter); + triggerRouters->destroy(triggerRouters); cache = (SymTriggerRoutersCache *) calloc(1, sizeof(SymTriggerRoutersCache)); cache->routersByRouterId = routers; cache->triggerRoutersByTriggerId = triggerRoutersByTriggerId; newTriggerRouterCacheByNodeGroupId->put(newTriggerRouterCacheByNodeGroupId, myNodeGroupId, cache); + if (this->triggerRouterCacheByNodeGroupId) { + this->triggerRouterCacheByNodeGroupId->destroy(this->triggerRouterCacheByNodeGroupId); + } this->triggerRouterCacheByNodeGroupId = newTriggerRouterCacheByNodeGroupId; cache = this->triggerRouterCacheByNodeGroupId == NULL ? NULL: this->triggerRouterCacheByNodeGroupId->get(this->triggerRouterCacheByNodeGroupId, myNodeGroupId); @@ -1087,6 +1097,9 @@ void SymTriggerRouterService_destroy(SymTriggerRouterService * this) { if (this->triggersCache) { this->triggersCache->destroy(this->triggersCache); } + if (this->triggerRouterCacheByNodeGroupId) { + this->triggerRouterCacheByNodeGroupId->destroy(this->triggerRouterCacheByNodeGroupId); + } free(this); } diff --git a/symmetric-client-clib/src/transport/http/HttpTransportManager.c b/symmetric-client-clib/src/transport/http/HttpTransportManager.c index 3b85d820a0..aa522497ad 100644 --- a/symmetric-client-clib/src/transport/http/HttpTransportManager.c +++ b/symmetric-client-clib/src/transport/http/HttpTransportManager.c @@ -110,7 +110,7 @@ static int sendMessage(SymHttpTransportManager *this, char *url, char *postData) } curl_easy_cleanup(curl); } else { - fprintf(stderr, "Error cannot initialize curl\n"); + SymLog_error("Cannot initialize curl."); } return httpResponseCode; } diff --git a/symmetric-client-clib/src/util/Date.c b/symmetric-client-clib/src/util/Date.c index f941959bb3..a2940fee6d 100644 --- a/symmetric-client-clib/src/util/Date.c +++ b/symmetric-client-clib/src/util/Date.c @@ -26,7 +26,7 @@ void SymDate_destroy(SymDate *this) { } SymDate * SymDate_newWithString(char *dateTimeString) { - SymDate *this = (SymDate *) calloc(1, sizeof(SymDate)); + SymDate *this = SymDate_new(); this->dateTimeString = SymStringBuilder_copy(dateTimeString); SymStringBuilder *sb = SymStringBuilder_newWithString(dateTimeString); char *year = sb->substring(sb, 0, 4); @@ -69,10 +69,10 @@ unsigned short SymDate_equals(SymDate *this, SymDate *otherDate) { SymDate * SymDate_newWithTime(time_t time) { SymDate *this = (SymDate *) calloc(1, sizeof(SymDate)); + this->destroy = (void *) &SymDate_destroy; this->before = (void *) &SymDate_before; this->after = (void *) &SymDate_after; this->equals = (void *) &SymDate_equals; - this->destroy = (void *) &SymDate_destroy; this->dateTimeString = calloc(24, sizeof(char)); this->time = time; struct tm *timeInfo = localtime(&time); diff --git a/symmetric-client-clib/src/util/Map.c b/symmetric-client-clib/src/util/Map.c index 3b556e3102..dbdd2c5631 100644 --- a/symmetric-client-clib/src/util/Map.c +++ b/symmetric-client-clib/src/util/Map.c @@ -199,6 +199,11 @@ void SymMap_resetAll(SymMap *this, void (*destroyObject)(void *object)) { } index++; } + + int i; + for (i = 0; i < this->size; i++) { + this->table[i] = NULL; + } } void SymMap_reset(SymMap *this) { diff --git a/symmetric-client-clib/src/util/StringArray.c b/symmetric-client-clib/src/util/StringArray.c index 0bbaba5e1c..dd69706362 100644 --- a/symmetric-client-clib/src/util/StringArray.c +++ b/symmetric-client-clib/src/util/StringArray.c @@ -29,7 +29,7 @@ int SymStringArray_compareFunction(const void * a, const void * b) { SymStringArray * SymStringArray_addn(SymStringArray *this, char *src, int size) { if (this->size == this->sizeAllocated) { this->sizeAllocated += this->sizeIncrement; - this->array = realloc(this->array, this->sizeAllocated); + this->array = (char **) realloc(this->array, this->sizeAllocated * sizeof(char *)); } char *str = NULL; if (size > 0) { @@ -126,7 +126,9 @@ void SymStringArray_print(SymStringArray *this) { void SymStringArray_reset(SymStringArray *this) { int i; for (i = 0; i < this->size; i++) { - free(this->array[i]); + if (this->array[i]) { + free(this->array[i]); + } } this->size = 0; }