From 022014f9ca14efacba28fd6fcaa60df3e59e294d Mon Sep 17 00:00:00 2001 From: mmichalek Date: Fri, 30 Oct 2015 15:53:59 -0400 Subject: [PATCH] Memory management improvements. --- symmetric-client-clib/inc/util/Map.h | 4 +-- symmetric-client-clib/src/db/model/Table.c | 3 +- .../src/db/platform/sqlite/SqliteDdlReader.c | 1 + symmetric-client-clib/src/db/sql/Row.c | 6 +++- .../src/db/sqlite/SqliteDialect.c | 1 + .../src/io/writer/DefaultDatabaseWriter.c | 5 ++- symmetric-client-clib/src/model/Trigger.c | 2 ++ .../src/model/TriggerHistory.c | 27 ++++++++++++++++ .../src/service/NodeService.c | 1 + .../src/service/ParameterService.c | 1 + .../src/service/RouterService.c | 3 +- .../src/service/SequenceService.c | 5 ++- .../src/service/TriggerRouterService.c | 31 ++++++++++++++++--- symmetric-client-clib/src/util/Map.c | 15 +++++---- symmetric-client-clib/src/util/Properties.c | 8 ++--- symmetric-client-clib/src/util/StringArray.c | 1 + symmetric-client-native/src/SymClientNative.c | 9 ++++-- 17 files changed, 91 insertions(+), 32 deletions(-) diff --git a/symmetric-client-clib/inc/util/Map.h b/symmetric-client-clib/inc/util/Map.h index ee87652b0f..f227a2278a 100644 --- a/symmetric-client-clib/inc/util/Map.h +++ b/symmetric-client-clib/inc/util/Map.h @@ -29,9 +29,9 @@ typedef struct SymMap { SymList * (*values)(struct SymMap *this); SymList * (*entries)(struct SymMap *this); void (*reset)(struct SymMap *this); - void (*resetAll)(struct SymMap *this, void (*destroyObject)(void *object), unsigned short shouldFreeKey); + void (*resetAll)(struct SymMap *this, void (*destroyObject)(void *object)); void (*destroy)(struct SymMap *this); - void (*destroyAll)(struct SymMap *this, void (*destroyObject)(void *object), unsigned short shouldFreeKey); + void (*destroyAll)(struct SymMap *this, void (*destroyObject)(void *object)); } SymMap; SymMap *SymMap_new(SymMap *this, int size); diff --git a/symmetric-client-clib/src/db/model/Table.c b/symmetric-client-clib/src/db/model/Table.c index 291704963b..6be24ccd40 100644 --- a/symmetric-client-clib/src/db/model/Table.c +++ b/symmetric-client-clib/src/db/model/Table.c @@ -27,6 +27,7 @@ char * SymTable_getFullyQualifiedTableName(char *catalogName, char *schemaName, } char *prefix = SymTable_getFullyQualifiedTablePrefix(catalogName, schemaName, quoteString, catalogSeparator, schemaSeparator); SymStringBuilder *sb = SymStringBuilder_newWithString(prefix); + free(prefix); sb->append(sb, quoteString)->append(sb, tableName)->append(sb, quoteString); return sb->destroyAndReturn(sb); } @@ -72,7 +73,7 @@ char * SymTable_getCommaDeliminatedColumns(SymList *cols) { return columns->destroyAndReturn(columns); } else { - return " "; + return SymStringUtils_format("%s", " "); } } diff --git a/symmetric-client-clib/src/db/platform/sqlite/SqliteDdlReader.c b/symmetric-client-clib/src/db/platform/sqlite/SqliteDdlReader.c index cac907f897..530b3fc5a3 100644 --- a/symmetric-client-clib/src/db/platform/sqlite/SqliteDdlReader.c +++ b/symmetric-client-clib/src/db/platform/sqlite/SqliteDdlReader.c @@ -61,6 +61,7 @@ static void * SymSqliteDdlReader_columnMapper(SymRow *row) { char *name = row->getStringNew(row, "name"); int isPrimaryKey = row->getInt(row, "pk"); SymColumn *column = SymColumn_new(NULL, name, isPrimaryKey); + free(name); column->isRequired = row->getInt(row, "notnull"); column->sqlType = SymSqliteDdlReader_toSqlType(row->getString(row, "type")); return column; diff --git a/symmetric-client-clib/src/db/sql/Row.c b/symmetric-client-clib/src/db/sql/Row.c index 6fe636057d..e96b780ff1 100644 --- a/symmetric-client-clib/src/db/sql/Row.c +++ b/symmetric-client-clib/src/db/sql/Row.c @@ -119,6 +119,7 @@ int SymRow_intValue(SymRow *this) { char *str = SymRow_stringValue(this); if (str != NULL) { value = atoi(str); + free(str); } return value; } @@ -128,6 +129,7 @@ long SymRow_longValue(SymRow *this) { char *str = SymRow_stringValue(this); if (str != NULL) { value = atol(str); + free(str); } return value; } @@ -137,6 +139,7 @@ unsigned short SymRow_booleanValue(SymRow *this) { char *str = SymRow_stringValue(this); if (str != NULL) { value = atoi(str); + free(str); } return value; } @@ -146,12 +149,13 @@ SymDate * SymRow_dateValue(SymRow *this) { char *str = SymRow_stringValue(this); if (str != NULL) { value = SymDate_newWithString(str); + free(str); } return value; } void SymRow_destroy(SymRow *this) { - this->map->destroyAll(this->map, (void *) &SymRowEntry_destroy, 0); + this->map->destroyAll(this->map, (void *) &SymRowEntry_destroy); free(this); } diff --git a/symmetric-client-clib/src/db/sqlite/SqliteDialect.c b/symmetric-client-clib/src/db/sqlite/SqliteDialect.c index c41ac0a9a7..6d813b0efe 100644 --- a/symmetric-client-clib/src/db/sqlite/SqliteDialect.c +++ b/symmetric-client-clib/src/db/sqlite/SqliteDialect.c @@ -115,6 +115,7 @@ int SymSqliteDialect_getInitialLoadSql(SymSqliteDialect *this) { void SymSqliteDialect_destroy(SymDialect *super) { SymSqliteDialect *this = (SymSqliteDialect *) super; + free(super->triggerTemplate); free(this); } diff --git a/symmetric-client-clib/src/io/writer/DefaultDatabaseWriter.c b/symmetric-client-clib/src/io/writer/DefaultDatabaseWriter.c index 2746d0125d..509cc3cb5c 100644 --- a/symmetric-client-clib/src/io/writer/DefaultDatabaseWriter.c +++ b/symmetric-client-clib/src/io/writer/DefaultDatabaseWriter.c @@ -201,9 +201,8 @@ unsigned short SymDefaultDatabaseWriter_write(SymDefaultDatabaseWriter *this, Sy if (!this->missingTables->get(this->missingTables, qualifiedName)) { SymLog_warn("Did not find the '%s' table in the target database", this->sourceTable->name); this->missingTables->put(this->missingTables, qualifiedName, qualifiedName); - } else { - free(qualifiedName); } + free(qualifiedName); return 1; } else { this->targetTable = this->sourceTable; @@ -285,7 +284,7 @@ void SymDefaultDatabaseWriter_close(SymDefaultDatabaseWriter *this) { void SymDefaultDatabaseWriter_destroy(SymDefaultDatabaseWriter *this) { this->targetTables->destroy(this->targetTables); - this->missingTables->destroyAll(this->missingTables, NULL, 1); + this->missingTables->destroyAll(this->missingTables, NULL); this->settings->destroy(this->settings); free(this); } diff --git a/symmetric-client-clib/src/model/Trigger.c b/symmetric-client-clib/src/model/Trigger.c index 5483d3a912..7dc3058435 100644 --- a/symmetric-client-clib/src/model/Trigger.c +++ b/symmetric-client-clib/src/model/Trigger.c @@ -46,6 +46,8 @@ SymList * SymTrigger_orderColumnsForTable(SymTrigger *this, SymTable *table) { orderedColumns->add(orderedColumns, cols->get(cols, i)); } } + + pks->destroy(pks); // TODO filterExcludedColumns } return orderedColumns; diff --git a/symmetric-client-clib/src/model/TriggerHistory.c b/symmetric-client-clib/src/model/TriggerHistory.c index 0b91639561..555dbb0335 100644 --- a/symmetric-client-clib/src/model/TriggerHistory.c +++ b/symmetric-client-clib/src/model/TriggerHistory.c @@ -62,6 +62,33 @@ 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); + } free(this); } diff --git a/symmetric-client-clib/src/service/NodeService.c b/symmetric-client-clib/src/service/NodeService.c index df4c211a7c..18f6dd1cc7 100644 --- a/symmetric-client-clib/src/service/NodeService.c +++ b/symmetric-client-clib/src/service/NodeService.c @@ -86,6 +86,7 @@ SymNode * SymNodeService_findIdentityWithCache(SymNodeService *this, unsigned sh SymList *nodes = sqlTemplate->query(sqlTemplate, sb->str, NULL, NULL, &error, (void *) SymNodeService_nodeMapper); this->cachedNodeIdentity = nodes->get(nodes, 0); + nodes->destroy(nodes); sb->destroy(sb); } return this->cachedNodeIdentity; diff --git a/symmetric-client-clib/src/service/ParameterService.c b/symmetric-client-clib/src/service/ParameterService.c index ad4b8e2979..d3731f84ae 100644 --- a/symmetric-client-clib/src/service/ParameterService.c +++ b/symmetric-client-clib/src/service/ParameterService.c @@ -42,6 +42,7 @@ static void getDatabaseParameters(SymParameterService *this, char *externalId, c args->add(args, externalId)->add(args, nodeGroupId); int error; SymList *list = sqlTemplate->queryWithUserData(sqlTemplate, SYM_SQL_SELECT_PARAMETERS, args, NULL, &error, (void *) ¶meterMapper, this); + args->destroy(args); list->destroy(list); } diff --git a/symmetric-client-clib/src/service/RouterService.c b/symmetric-client-clib/src/service/RouterService.c index 7841629298..76a98247fb 100644 --- a/symmetric-client-clib/src/service/RouterService.c +++ b/symmetric-client-clib/src/service/RouterService.c @@ -247,8 +247,7 @@ long SymRouterService_routeData(SymRouterService *this) { } void SymRouterService_destroy(SymRouterService *this) { - // TODO: clean up memory - // SymMap_destroyAll(this->routers, SymDataRouter_destroy); + SymMap_destroy(this->routers); free(this); } diff --git a/symmetric-client-clib/src/service/SequenceService.c b/symmetric-client-clib/src/service/SequenceService.c index 60060ab5ea..8191e9ef48 100644 --- a/symmetric-client-clib/src/service/SequenceService.c +++ b/symmetric-client-clib/src/service/SequenceService.c @@ -46,6 +46,7 @@ static SymMap * getAll(SymSequenceService *this) { map->put(map, sequence->sequenceName, sequence); } iter->destroy(iter); + sequences->destroy(sequences); return map; } @@ -173,6 +174,8 @@ void SymSequenceService_init(SymSequenceService *this) { initSequence(this, SYM_SEQUENCE_TRIGGER_HIST, maxTriggerHistId); } + sequences->destroyAll(sequences, (void *) SymSequence_destroy); + //if (sequences->get(sequences, SYM_SEQUENCE_EXTRACT_REQ) == NULL) { // long maxRequestId = sqlTemplate->queryForLong(sqlTemplate, SYM_SQL_MAX_EXTRACT_REQUEST, NULL, NULL, &error); // initSequence(this, SYM_SEQUENCE_EXTRACT_REQ, maxRequestId); @@ -180,7 +183,7 @@ void SymSequenceService_init(SymSequenceService *this) { } void SymSequenceService_destroy(SymSequenceService *this) { - this->sequenceDefinitionCache->destroyAll(this->sequenceDefinitionCache, (void *) &SymSequence_destroy, 0); + this->sequenceDefinitionCache->destroyAll(this->sequenceDefinitionCache, (void *) &SymSequence_destroy); free(this); } diff --git a/symmetric-client-clib/src/service/TriggerRouterService.c b/symmetric-client-clib/src/service/TriggerRouterService.c index 3d65496f45..ab923f5ee6 100644 --- a/symmetric-client-clib/src/service/TriggerRouterService.c +++ b/symmetric-client-clib/src/service/TriggerRouterService.c @@ -112,8 +112,6 @@ static SymRouter * SymTriggerRouterService_routerMapper(SymRow *row) { SymRouter *router = SymRouter_new(NULL); - SymNodeGroupLink_new(NULL); - router->syncOnInsert = row->getBoolean(row, "r_sync_on_insert"); router->syncOnUpdate = row->getBoolean(row, "r_sync_on_update"); router->syncOnDelete = row->getBoolean(row, "r_sync_on_delete"); @@ -307,6 +305,14 @@ static SymList * buildTriggersForSymmetricTables(SymTriggerRouterService *this, SymTrigger *trigger = buildTriggerForSymmetricTable(this, tableName); triggers->add(triggers, trigger); } + + if (definedTriggers->size > 0) { + definedTriggers->destroyAll(definedTriggers, (void *)((SymTrigger*)triggers->head)->destroy); + } else { + definedTriggers->destroy(definedTriggers); + } + + tables->destroy(tables); iter->destroy(iter); return triggers; } @@ -392,12 +398,13 @@ SymTriggerHistory * SymTriggerRouterService_getNewestTriggerHistoryForTrigger(Sy if (catalogMatches && schemaMatches) { result = triggerHistory; + triggerHistories->remove(triggerHistories, i); // remove so we can safely destroy the rest if the list below. break; } } args->destroy(args); - triggerHistories->destroy(triggerHistories); + triggerHistories->destroyAll(triggerHistories, (void *) result->destroy); return result; } @@ -422,6 +429,7 @@ SymList * SymTriggerRouterService_enhanceTriggerRouters(SymTriggerRouterService char *triggerId = SymStringUtils_toUpperCase(trimmed); triggersById->put(triggersById, triggerId, trigger); free(trimmed); +// free(triggerId); TODO this causes crash because the map needs this key around. } for (i = 0; i < triggerRouters->size; i++) { @@ -434,10 +442,13 @@ SymList * SymTriggerRouterService_enhanceTriggerRouters(SymTriggerRouterService triggerRouter->trigger = triggersById->get(triggersById, triggerId ); triggerRouter->router = routersById->get(routersById, routerId ); + free(triggerId); + free(routerId); free(triggerIdTrimmed); free(routerIdTrimmed); } + triggers->destroy(triggers); routers->destroy(routers); triggersById->destroy(triggersById); routersById->destroy(routersById); @@ -665,8 +676,14 @@ SymTriggerHistory * SymTriggerRouterService_rebuildTriggerIfNecessary(SymTrigger newTriggerHist->lastTriggerBuildReason = reason; newTriggerHist->sourceTableName = trigger->sourceTableName; // TODO trigger.isSourceTableNameWildCarded() - newTriggerHist->columnNames = SymTable_getCommaDeliminatedColumns(trigger->orderColumnsForTable(trigger, table)); - newTriggerHist->pkColumnNames = SymTable_getCommaDeliminatedColumns(trigger->getSyncKeysColumnsForTable(trigger, table)); + SymList *orderColumnsForTable = trigger->orderColumnsForTable(trigger, table); + newTriggerHist->columnNames = SymTable_getCommaDeliminatedColumns(orderColumnsForTable); + orderColumnsForTable->destroy(orderColumnsForTable); + + SymList *syncKeysColumnsForTable = trigger->getSyncKeysColumnsForTable(trigger, table); + newTriggerHist->pkColumnNames = SymTable_getCommaDeliminatedColumns(syncKeysColumnsForTable); + syncKeysColumnsForTable->destroy(syncKeysColumnsForTable); + newTriggerHist->triggerRowHash = trigger->toHashedValue(trigger); newTriggerHist->triggerTemplateHash = this->symmetricDialect->triggerTemplate-> toHashedValue(this->symmetricDialect->triggerTemplate); @@ -677,14 +694,17 @@ SymTriggerHistory * SymTriggerRouterService_rebuildTriggerIfNecessary(SymTrigger if (trigger->syncOnInsert) { char* triggerName = SymTriggerRouterService_getTriggerName(this, SYM_DATA_EVENT_INSERT, maxTriggerNameLength, trigger, table, activeTriggerHistories); newTriggerHist->nameForInsertTrigger = SymStringUtils_toUpperCase(triggerName); + free(triggerName); } if (trigger->syncOnUpdate) { char* triggerName = SymTriggerRouterService_getTriggerName(this, SYM_DATA_EVENT_UPDATE, maxTriggerNameLength, trigger, table, activeTriggerHistories); newTriggerHist->nameForUpdateTrigger = SymStringUtils_toUpperCase(triggerName); + free(triggerName); } if (trigger->syncOnDelete) { char* triggerName = SymTriggerRouterService_getTriggerName(this, SYM_DATA_EVENT_DELETE, maxTriggerNameLength, trigger, table, activeTriggerHistories); newTriggerHist->nameForDeleteTrigger = SymStringUtils_toUpperCase(triggerName); + free(triggerName); } char *oldTriggerName = NULL; @@ -840,6 +860,7 @@ void SymTriggerRouterService_syncTriggers(SymTriggerRouterService *this, unsigne } } + symmetricTableTriggers->destroy(symmetricTableTriggers); triggers->destroy(triggers); activeTriggerHistories->destroy(activeTriggerHistories); } diff --git a/symmetric-client-clib/src/util/Map.c b/symmetric-client-clib/src/util/Map.c index 9c46afdfd8..3b8ce3c6ae 100644 --- a/symmetric-client-clib/src/util/Map.c +++ b/symmetric-client-clib/src/util/Map.c @@ -38,7 +38,7 @@ static SymMapEntry * SymMap_newEntry(char *key, void *value) { return NULL; } - entry->key = key; + entry->key = SymStringBuilder_copy(key); entry->value = value; entry->next = NULL; return entry; @@ -158,6 +158,7 @@ void * SymMap_remove(SymMap *this, char *key) { while (entry != NULL) { if (entry->key != NULL && strcmp(key, entry->key) == 0) { result = entry->value; + free(entry->key); entry->key = NULL; entry->value = NULL; break; @@ -175,7 +176,7 @@ void * SymMap_removeByInt(SymMap *this, int key) { return result; } -void SymMap_resetAll(SymMap *this, void (*destroyObject)(void *object), unsigned short shouldFreeKey) { +void SymMap_resetAll(SymMap *this, void (*destroyObject)(void *object)) { SymMapEntry *entry = NULL; int index = 0; @@ -189,9 +190,7 @@ void SymMap_resetAll(SymMap *this, void (*destroyObject)(void *object), unsigned if (destroyObject) { destroyObject(currentEntry->value); } - if (shouldFreeKey) { - free(currentEntry->key); - } + free(currentEntry->key); currentEntry->key = NULL; currentEntry->value = NULL; free(currentEntry); @@ -203,7 +202,7 @@ void SymMap_resetAll(SymMap *this, void (*destroyObject)(void *object), unsigned } void SymMap_reset(SymMap *this) { - SymMap_resetAll(this, NULL, 0); + SymMap_resetAll(this, NULL); } void SymMap_destroy(SymMap *this) { @@ -212,8 +211,8 @@ void SymMap_destroy(SymMap *this) { free(this); } -void SymMap_destroyAll(SymMap *this, void (*destroyObject)(void *object), unsigned short shouldFreeKey) { - SymMap_resetAll(this, destroyObject, shouldFreeKey); +void SymMap_destroyAll(SymMap *this, void (*destroyObject)(void *object)) { + SymMap_resetAll(this, destroyObject); free(this->table); free(this); } diff --git a/symmetric-client-clib/src/util/Properties.c b/symmetric-client-clib/src/util/Properties.c index fc08605da8..0e5790d20c 100644 --- a/symmetric-client-clib/src/util/Properties.c +++ b/symmetric-client-clib/src/util/Properties.c @@ -92,6 +92,7 @@ SymProperties * SymProperties_newWithString(SymProperties *this, char *propertie char *trimmed = SymStringUtils_ltrim(propertyLine); if (trimmed[0] == '#' || trimmed[0] == '!' ) { + free(trimmed); continue; } @@ -122,10 +123,6 @@ SymProperties * SymProperties_newWithString(SymProperties *this, char *propertie } SymProperties * SymProperties_newWithFile(SymProperties *this, char *argPath) { - if (this == NULL) { - this = SymProperties_new(this); - } - FILE *file; int BUFFER_SIZE = 1024; char inputBuffer[BUFFER_SIZE]; @@ -141,10 +138,9 @@ SymProperties * SymProperties_newWithFile(SymProperties *this, char *argPath) { buff->append(buff, inputBuffer); } - this = SymProperties_newWithString(this, buff->destroyAndReturn(buff)); + this = SymProperties_newWithString(NULL, buff->destroyAndReturn(buff)); fclose(file); - return this; } diff --git a/symmetric-client-clib/src/util/StringArray.c b/symmetric-client-clib/src/util/StringArray.c index 7547bc7fbc..0bbaba5e1c 100644 --- a/symmetric-client-clib/src/util/StringArray.c +++ b/symmetric-client-clib/src/util/StringArray.c @@ -148,6 +148,7 @@ void SymStringArray_sort(SymStringArray *this) { void SymStringArray_destroy(SymStringArray *this) { this->reset(this); + free(this->array); free(this); } diff --git a/symmetric-client-native/src/SymClientNative.c b/symmetric-client-native/src/SymClientNative.c index 04e31759c1..be2826e425 100644 --- a/symmetric-client-native/src/SymClientNative.c +++ b/symmetric-client-native/src/SymClientNative.c @@ -28,7 +28,9 @@ SymStringArray * SymNativeClient_getPropertiesNames(int argCount, char **argValu propertiesNames->add(propertiesNames, argValues[1]); } else { - propertiesNames->add(propertiesNames, SymStringUtils_format("%s/symmetric.properties", getenv("HOME"))); + char *location = SymStringUtils_format("%s/symmetric.properties", getenv("HOME")); + propertiesNames->add(propertiesNames, location); + free(location); propertiesNames->add(propertiesNames, "./symmetric.properties"); } return propertiesNames; @@ -44,7 +46,6 @@ int SymNativeClient_runSymmetricEngine(SymProperties *properties) { jobManager->destroy(jobManager); engine->destroy(engine); - properties->destroy(properties); return 0; } @@ -77,7 +78,9 @@ int main(int argCount, char **argValues) { SymNativeClient_runSymmetricEngine(properties); - properties->destroy(properties); + if (properties) { + properties->destroy(properties); + } propertiesFiles->destroy(propertiesFiles); return 0; }