From 82a95bfe2d04b48621d381a9c70505bdef805f74 Mon Sep 17 00:00:00 2001 From: Marko Date: Wed, 6 Jun 2018 09:59:35 +0300 Subject: [PATCH 1/3] MXS-1709 Fix memory leaks causing test_server to fail --- server/core/server.cc | 2 ++ server/core/test/test_server.cc | 2 ++ 2 files changed, 4 insertions(+) diff --git a/server/core/server.cc b/server/core/server.cc index 1b1b7115f..c3509ff10 100644 --- a/server/core/server.cc +++ b/server/core/server.cc @@ -195,6 +195,8 @@ server_free(SERVER *tofreeserver) /* Clean up session and free the memory */ MXS_FREE(tofreeserver->protocol); MXS_FREE(tofreeserver->unique_name); + MXS_FREE(tofreeserver->authenticator); + MXS_FREE(tofreeserver->persistent); server_parameter_free(tofreeserver->parameters); if (tofreeserver->persistent) diff --git a/server/core/test/test_server.cc b/server/core/test/test_server.cc index f05019deb..7a06cea7c 100644 --- a/server/core/test/test_server.cc +++ b/server/core/test/test_server.cc @@ -133,6 +133,8 @@ bool test_load_config(const char *input, SERVER *server) "Server authenticator options differ"); TEST(server->port == atoi(config_get_param(param, "port")->value), "Server ports differ"); TEST(create_new_server(obj) == 0, "Failed to create server from loaded config"); + duplicate_context_finish(&dcontext); + config_context_free(obj); } } From 3c0f301674e51897ed2ce6157c9647ec440cf291 Mon Sep 17 00:00:00 2001 From: Marko Date: Wed, 6 Jun 2018 22:32:52 +0300 Subject: [PATCH 2/3] MXS-1709 Fix to test_server --- server/core/server.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/server.cc b/server/core/server.cc index c3509ff10..6dd532503 100644 --- a/server/core/server.cc +++ b/server/core/server.cc @@ -196,7 +196,6 @@ server_free(SERVER *tofreeserver) MXS_FREE(tofreeserver->protocol); MXS_FREE(tofreeserver->unique_name); MXS_FREE(tofreeserver->authenticator); - MXS_FREE(tofreeserver->persistent); server_parameter_free(tofreeserver->parameters); if (tofreeserver->persistent) @@ -207,6 +206,7 @@ server_free(SERVER *tofreeserver) { dcb_persistent_clean_count(tofreeserver->persistent[i], i, true); } + MXS_FREE(tofreeserver->persistent); } MXS_FREE(tofreeserver); return 1; From 0dd74485868bf852e50eda47451832a050aa9fa8 Mon Sep 17 00:00:00 2001 From: Marko Date: Wed, 6 Jun 2018 22:59:52 +0300 Subject: [PATCH 3/3] MXS-1709 Fix memory leaks in unit tests --- .../filter/cache/test/testkeygeneration.cc | 5 +- .../routing/binlogrouter/test/testbinlog.c | 80 ++++++++++++++++--- 2 files changed, 72 insertions(+), 13 deletions(-) diff --git a/server/modules/filter/cache/test/testkeygeneration.cc b/server/modules/filter/cache/test/testkeygeneration.cc index 5a5a2d0f6..d6b470ca9 100644 --- a/server/modules/filter/cache/test/testkeygeneration.cc +++ b/server/modules/filter/cache/test/testkeygeneration.cc @@ -125,7 +125,7 @@ int test(StorageFactory& factory, istream& in) int main(int argc, char* argv[]) { int rv = EXIT_FAILURE; - + StorageFactory* pFactory; if ((argc == 2) || (argc == 3)) { char* libdir = MXS_STRDUP("../../../../../query_classifier/qc_sqlite/"); @@ -139,7 +139,7 @@ int main(int argc, char* argv[]) libdir = MXS_STRDUP("../storage/storage_inmemory/"); set_libdir(libdir); - StorageFactory* pFactory = StorageFactory::Open(zModule); + pFactory = StorageFactory::Open(zModule); if (pFactory) { @@ -182,6 +182,7 @@ int main(int argc, char* argv[]) // TODO: Remove this once globally allocated memory is freed MXS_FREE(libdir); + delete pFactory; } else { diff --git a/server/modules/routing/binlogrouter/test/testbinlog.c b/server/modules/routing/binlogrouter/test/testbinlog.c index a784e573a..14a0f9e71 100644 --- a/server/modules/routing/binlogrouter/test/testbinlog.c +++ b/server/modules/routing/binlogrouter/test/testbinlog.c @@ -54,6 +54,7 @@ static void printVersion(const char *progname); static void printUsage(const char *progname); +static void master_free_parsed_options(CHANGE_MASTER_OPTIONS *options); extern int blr_test_parse_change_master_command(char *input, char *error_string, CHANGE_MASTER_OPTIONS *config); extern char *blr_test_set_master_logfile(ROUTER_INSTANCE *router, char *filename, char *error); @@ -76,6 +77,12 @@ int main(int argc, char **argv) int rc; char error_string[BINLOG_ERROR_MSG_LEN + 1] = ""; CHANGE_MASTER_OPTIONS change_master; + change_master.host = NULL; + change_master.port = NULL; + change_master.user = NULL; + change_master.password = NULL; + change_master.binlog_file = NULL; + change_master.binlog_pos = NULL; char query[512 + 1] = ""; char saved_query[512 + 1] = ""; int command_offset = strlen("CHANGE MASTER TO"); @@ -329,7 +336,7 @@ int main(int argc, char **argv) } tests++; - + master_free_parsed_options(&change_master); /** * Test 9: 1 valid option with value * @@ -348,7 +355,7 @@ int main(int argc, char **argv) { printf("Test %d PASSED, valid options for [%s]\n", tests, saved_query); } - + master_free_parsed_options(&change_master); tests++; /** @@ -369,7 +376,7 @@ int main(int argc, char **argv) { printf("Test %d PASSED, valid / not valid options for [%s]\n", tests, saved_query); } - + master_free_parsed_options(&change_master); tests++; /** @@ -390,7 +397,7 @@ int main(int argc, char **argv) { printf("Test %d PASSED, valid / not valid options for [%s]\n", tests, saved_query); } - + master_free_parsed_options(&change_master); tests++; /** @@ -411,7 +418,7 @@ int main(int argc, char **argv) { printf("Test %d PASSED, valid options for [%s]\n", tests, saved_query); } - + master_free_parsed_options(&change_master); tests++; /** @@ -433,7 +440,7 @@ int main(int argc, char **argv) { printf("Test %d PASSED, valid / not valid options for [%s]\n", tests, saved_query); } - + master_free_parsed_options(&change_master); tests++; /** @@ -455,7 +462,7 @@ int main(int argc, char **argv) { printf("Test %d PASSED, valid options for [%s]\n", tests, saved_query); } - + master_free_parsed_options(&change_master); tests++; /** @@ -478,7 +485,7 @@ int main(int argc, char **argv) { printf("Test %d PASSED, valid / not valid options for [%s]\n", tests, saved_query); } - + master_free_parsed_options(&change_master); tests++; /** @@ -502,6 +509,16 @@ int main(int argc, char **argv) { printf("Test %d PASSED, valid options for [%s]\n", tests, saved_query); } + MXS_FREE(change_master.host); + change_master.host = NULL; + MXS_FREE(change_master.port); + change_master.port = NULL; + MXS_FREE(change_master.user); + change_master.user = NULL; + MXS_FREE(change_master.password); + change_master.password = NULL; + MXS_FREE(change_master.binlog_pos); + change_master.binlog_pos = NULL; tests++; @@ -509,7 +526,7 @@ int main(int argc, char **argv) /** * Test 17: use current binlog filename in master_state != BLRM_UNCONFIGURED - * and try to set a new filename with wront format from previous test + * and try to set a new filename with wrong format from previous test * * Expected master_log_file is NULL and error_string is not empty */ @@ -539,7 +556,8 @@ int main(int argc, char **argv) } tests++; - + MXS_FREE(change_master.binlog_file); + change_master.binlog_file = NULL; printf("--- MASTER_LOG_POS and MASTER_LOG_FILE rule/constraints checks ---\n"); /******************************************** @@ -845,8 +863,48 @@ int main(int argc, char **argv) mxs_log_flush_sync(); mxs_log_finish(); - + MXS_FREE(inst->user); + MXS_FREE(inst->password); + MXS_FREE(inst->fileroot); MXS_FREE(inst); MXS_FREE(roptions); return 0; } + +static void +master_free_parsed_options(CHANGE_MASTER_OPTIONS *options) +{ + if (options->host) + { + MXS_FREE(options->host); + options->host = NULL; + } + if (options->port) + { + MXS_FREE(options->port); + options->port = NULL; + } + if (options->user) + { + MXS_FREE(options->user); + options->user = NULL; + } + + if (options->password) + { + MXS_FREE(options->password); + options->password = NULL; + } + + if (options->binlog_file) + { + MXS_FREE(options->binlog_file); + options->binlog_file = NULL; + } + + if (options->binlog_pos) + { + MXS_FREE(options->binlog_pos); + options->binlog_pos = NULL; + } +}