Skip to content

Commit

Permalink
Memory improvements, fix for StringArray when splitting large string.
Browse files Browse the repository at this point in the history
  • Loading branch information
mmichalek committed Nov 10, 2015
1 parent 898147d commit 281632b
Show file tree
Hide file tree
Showing 12 changed files with 70 additions and 35 deletions.
20 changes: 20 additions & 0 deletions symmetric-client-clib-test/src/util/MapTest.c
Expand Up @@ -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) {
Expand All @@ -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

) {
Expand Down
18 changes: 17 additions & 1 deletion symmetric-client-clib-test/src/util/StringArrayTest.c
Expand Up @@ -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);
Expand All @@ -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;
Expand Down
1 change: 0 additions & 1 deletion symmetric-client-clib/src/core/SymEngine.c
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions symmetric-client-clib/src/model/Channel.c
Expand Up @@ -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);
}
Expand Down
21 changes: 0 additions & 21 deletions symmetric-client-clib/src/model/TriggerHistory.c
Expand Up @@ -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);
}
Expand Down
1 change: 1 addition & 0 deletions symmetric-client-clib/src/service/DataService.c
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion symmetric-client-clib/src/service/RouterService.c
Expand Up @@ -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",
Expand Down Expand Up @@ -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;
}
Expand Down
21 changes: 17 additions & 4 deletions symmetric-client-clib/src/service/TriggerRouterService.c
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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;
}
Expand Down Expand Up @@ -1012,6 +1017,7 @@ static void SymTriggerRouterService_mergeInConfigurationTablesTriggerRoutersForC
configuredInDatabase->add(configuredInDatabase, trigger);
}
}

virtualConfigTriggers->destroy(virtualConfigTriggers);
iter->destroy(iter);
}
Expand Down Expand Up @@ -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);
Expand All @@ -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);
}

Expand Down
Expand Up @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions symmetric-client-clib/src/util/Date.c
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 5 additions & 0 deletions symmetric-client-clib/src/util/Map.c
Expand Up @@ -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) {
Expand Down
6 changes: 4 additions & 2 deletions symmetric-client-clib/src/util/StringArray.c
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit 281632b

Please sign in to comment.