From 05303f5b7e7ad411bc0d1599b652cffbde78eca4 Mon Sep 17 00:00:00 2001 From: ekorh475 Date: Fri, 25 Nov 2016 12:53:15 +0200 Subject: [PATCH] Fix memory leaks in maxinfo (modified for develop-branch) MXS-1009. This commit adds a gwbuf_free after maxinfo_execute() to free a buffer with an sql-query after it has been processed. Also, the parse tree in maxinfo_execute_query() is now freed. The tree_free- function was renamed to maxinfo_tree_free, since it is now globally available. This commit has additional changes (in relation to the 1.4.4 branch) to remove errors caused by differences in the html and sql-sides of MaxInfo. --- server/modules/routing/maxinfo/maxinfo.c | 21 ++++++++---- server/modules/routing/maxinfo/maxinfo.h | 1 + server/modules/routing/maxinfo/maxinfo_exec.c | 32 +++++++++++++------ .../modules/routing/maxinfo/maxinfo_parse.c | 24 +++++++------- 4 files changed, 49 insertions(+), 29 deletions(-) diff --git a/server/modules/routing/maxinfo/maxinfo.c b/server/modules/routing/maxinfo/maxinfo.c index 3115899d8..2e4f75a3c 100644 --- a/server/modules/routing/maxinfo/maxinfo.c +++ b/server/modules/routing/maxinfo/maxinfo.c @@ -359,29 +359,35 @@ execute(ROUTER *rinstance, void *router_session, GWBUF *queue) return 1; } - // We have a complete request in a signle buffer + int rc = 1; + // We have a complete request in a single buffer if (modutil_MySQL_Query(queue, &sql, &len, &residual)) { sql = strndup(sql, len); - int rc = maxinfo_execute_query(instance, session, sql); + rc = maxinfo_execute_query(instance, session, sql); MXS_FREE(sql); - return rc; } else { switch (MYSQL_COMMAND(queue)) { case COM_PING: - return maxinfo_ping(instance, session, queue); + rc = maxinfo_ping(instance, session, queue); + break; case COM_STATISTICS: - return maxinfo_statistics(instance, session, queue); + rc = maxinfo_statistics(instance, session, queue); + break; + case COM_QUIT: + break; default: MXS_ERROR("maxinfo: Unexpected MySQL command 0x%x", MYSQL_COMMAND(queue)); + break; } } - - return 1; + // MaxInfo doesn't route the data forward so it should be freed. + gwbuf_free(queue); + return rc; } /** @@ -652,6 +658,7 @@ maxinfo_execute_query(INFO_INSTANCE *instance, INFO_SESSION *session, char *sql) else { maxinfo_execute(session->dcb, tree); + maxinfo_free_tree(tree); } return 1; } diff --git a/server/modules/routing/maxinfo/maxinfo.h b/server/modules/routing/maxinfo/maxinfo.h index 4e42292ae..f736b3f0e 100644 --- a/server/modules/routing/maxinfo/maxinfo.h +++ b/server/modules/routing/maxinfo/maxinfo.h @@ -138,6 +138,7 @@ typedef enum extern MAXINFO_TREE *maxinfo_parse(char *, PARSE_ERROR *); +extern void maxinfo_free_tree(MAXINFO_TREE *); extern void maxinfo_execute(DCB *, MAXINFO_TREE *); extern void maxinfo_send_error(DCB *, int, char *); extern void maxinfo_send_parse_error(DCB *, char *, PARSE_ERROR); diff --git a/server/modules/routing/maxinfo/maxinfo_exec.c b/server/modules/routing/maxinfo/maxinfo_exec.c index a82433b1b..3ede0d05b 100644 --- a/server/modules/routing/maxinfo/maxinfo_exec.c +++ b/server/modules/routing/maxinfo/maxinfo_exec.c @@ -880,21 +880,27 @@ static void exec_show_variables(DCB *dcb, MAXINFO_TREE *filter) { RESULTSET *result; - VARCONTEXT context; + VARCONTEXT *context; + + if ((context = MXS_MALLOC(sizeof(VARCONTEXT))) == NULL) + { + return; + } if (filter) { - context.like = filter->value; + context->like = filter->value; } else { - context.like = NULL; + context->like = NULL; } - context.index = 0; + context->index = 0; - if ((result = resultset_create(variable_row, &context)) == NULL) + if ((result = resultset_create(variable_row, context)) == NULL) { maxinfo_send_error(dcb, 0, "No resources available"); + MXS_FREE(context); return; } resultset_add_column(result, "Variable_name", 40, COL_TYPE_VARCHAR); @@ -1166,21 +1172,27 @@ static void exec_show_status(DCB *dcb, MAXINFO_TREE *filter) { RESULTSET *result; - VARCONTEXT context; + VARCONTEXT *context; + + if ((context = MXS_MALLOC(sizeof(VARCONTEXT))) == NULL) + { + return; + } if (filter) { - context.like = filter->value; + context->like = filter->value; } else { - context.like = NULL; + context->like = NULL; } - context.index = 0; + context->index = 0; - if ((result = resultset_create(status_row, &context)) == NULL) + if ((result = resultset_create(status_row, context)) == NULL) { maxinfo_send_error(dcb, 0, "No resources available"); + MXS_FREE(context); return; } resultset_add_column(result, "Variable_name", 40, COL_TYPE_VARCHAR); diff --git a/server/modules/routing/maxinfo/maxinfo_parse.c b/server/modules/routing/maxinfo/maxinfo_parse.c index e75d65e92..95b4cdfbf 100644 --- a/server/modules/routing/maxinfo/maxinfo_parse.c +++ b/server/modules/routing/maxinfo/maxinfo_parse.c @@ -43,7 +43,7 @@ #include static MAXINFO_TREE *make_tree_node(MAXINFO_OPERATOR, char *, MAXINFO_TREE *, MAXINFO_TREE *); -static void free_tree(MAXINFO_TREE *); +void maxinfo_free_tree(MAXINFO_TREE *); // This function is needed by maxinfo.c static char *fetch_token(char *, int *, char **); static MAXINFO_TREE *parse_column_list(char **sql); static MAXINFO_TREE *parse_table_name(char **sql); @@ -95,13 +95,13 @@ maxinfo_parse(char *sql, PARSE_ERROR *parse_error) { // Expected expression *parse_error = PARSE_EXPECTED_LIKE; - free_tree(tree); + maxinfo_free_tree(tree); return NULL; } } // Malformed show MXS_FREE(text); - free_tree(tree); + maxinfo_free_tree(tree); *parse_error = PARSE_MALFORMED_SHOW; return NULL; #if 0 @@ -132,7 +132,7 @@ maxinfo_parse(char *sql, PARSE_ERROR *parse_error) { /** Unknown token after SHUTDOWN MONITOR|SERVICE */ *parse_error = PARSE_SYNTAX_ERROR; - free_tree(tree); + maxinfo_free_tree(tree); return NULL; } return tree; @@ -146,7 +146,7 @@ maxinfo_parse(char *sql, PARSE_ERROR *parse_error) { /** Missing token for RESTART MONITOR|SERVICE */ *parse_error = PARSE_SYNTAX_ERROR; - free_tree(tree); + maxinfo_free_tree(tree); return NULL; } tree->right = make_tree_node(MAXOP_LITERAL, text, NULL, NULL); @@ -156,7 +156,7 @@ maxinfo_parse(char *sql, PARSE_ERROR *parse_error) /** Unknown token after RESTART MONITOR|SERVICE */ *parse_error = PARSE_SYNTAX_ERROR; MXS_FREE(text); - free_tree(tree); + maxinfo_free_tree(tree); return NULL; } return tree; @@ -278,20 +278,20 @@ make_tree_node(MAXINFO_OPERATOR op, char *value, MAXINFO_TREE *left, MAXINFO_TRE } /** - * Recusrsively free the storage associated with a parse tree + * Recursively free the storage associated with a parse tree * * @param tree The parse tree to free */ -static void -free_tree(MAXINFO_TREE *tree) +void +maxinfo_free_tree(MAXINFO_TREE *tree) { if (tree->left) { - free_tree(tree->left); + maxinfo_free_tree(tree->left); } if (tree->right) { - free_tree(tree->right); + maxinfo_free_tree(tree->right); } if (tree->value) { @@ -431,7 +431,7 @@ MAXINFO_TREE* maxinfo_parse_literals(MAXINFO_TREE *tree, int min_args, char *ptr (node->right = make_tree_node(MAXOP_LITERAL, text, NULL, NULL)) == NULL) { *parse_error = PARSE_SYNTAX_ERROR; - free_tree(tree); + maxinfo_free_tree(tree); if (ptr) { MXS_FREE(text);