From f4c1b298d6e051786cc5ac54e4696310dd8c90d0 Mon Sep 17 00:00:00 2001 From: Mark Riddoch Date: Mon, 24 Jun 2013 16:43:17 +0200 Subject: [PATCH] Fixes for close and cleanup of sessions, dcb, router sessions etc. Fix memory leak in config Fix for debug command execution without second argument --- core/config.c | 36 ++++++++++++++++++++++++++++++++- core/dcb.c | 26 ++++++++++++++++++++++++ core/depend.mk | 2 +- include/spinlock.h | 2 +- modules/protocol/mysql_client.c | 8 ++------ modules/routing/debugcli.c | 4 ---- modules/routing/debugcmd.c | 2 +- 7 files changed, 66 insertions(+), 14 deletions(-) diff --git a/core/config.c b/core/config.c index d1d00a9e9..9450ed5b7 100644 --- a/core/config.c +++ b/core/config.c @@ -38,6 +38,7 @@ static int process_config_context(CONFIG_CONTEXT *); +static void free_config_context(CONFIG_CONTEXT *); static char *config_get_value(CONFIG_PARAMETER *, const char *); @@ -91,6 +92,7 @@ int load_config(char *file) { CONFIG_CONTEXT config; +int rval; config.object = ""; config.next = NULL; @@ -98,7 +100,10 @@ CONFIG_CONTEXT config; if (ini_parse(file, handler, &config) < 0) return 0; - return process_config_context(config.next); + rval = process_config_context(config.next); + free_config_context(config.next); + + return rval; } /** @@ -217,3 +222,32 @@ config_get_value(CONFIG_PARAMETER *params, const char *name) } return NULL; } + +/** + * Free a config tree + * + * @param context The configuration data + */ +static void +free_config_context(CONFIG_CONTEXT *context) +{ +CONFIG_CONTEXT *obj; +CONFIG_PARAMETER *p1, *p2; + + while (context) + { + free(context->object); + p1 = context->parameters; + while (p1) + { + free(p1->name); + free(p1->value); + p2 = p1->next; + free(p1); + p1 = p2; + } + obj = context->next; + free(context); + context = obj; + } +} diff --git a/core/dcb.c b/core/dcb.c index df2f23edf..6bdbb2f4d 100644 --- a/core/dcb.c +++ b/core/dcb.c @@ -43,6 +43,7 @@ #include #include #include +#include #include #include #include @@ -79,6 +80,8 @@ DCB *rval; rval->remote = NULL; rval->state = DCB_STATE_ALLOC; rval->next = NULL; + rval->data = NULL; + rval->protocol = NULL; memset(&rval->stats, 0, sizeof(DCBSTATS)); // Zero the statistics spinlock_acquire(dcbspin); @@ -119,6 +122,10 @@ dcb_free(DCB *dcb) } spinlock_release(dcbspin); + if (dcb->protocol) + free(dcb->protocol); + if (dcb->data) + free(dcb->data); if (dcb->remote) free(dcb->remote); free(dcb); @@ -352,8 +359,27 @@ int saved_errno = 0; void dcb_close(DCB *dcb) { + poll_remove_dcb(dcb); close(dcb->fd); dcb->state = DCB_STATE_DISCONNECTED; + + if (dcb_isclient(dcb)) + { + /* + * If the DCB we are closing is a client side DCB then shutdown the + * router session. This will close any backend connections. + */ + SERVICE *service = dcb->session->service; + + if (service && service->router) + { + service->router->closeSession(service->router_instance, + dcb->session->router_session); + } + + session_free(dcb->session); + } + dcb_free(dcb); } /** diff --git a/core/depend.mk b/core/depend.mk index 8c19ccd8e..97aa3ede4 100644 --- a/core/depend.mk +++ b/core/depend.mk @@ -234,7 +234,7 @@ dcb.o: dcb.c /usr/include/stdio.h /usr/include/features.h \ /usr/include/sched.h /usr/include/bits/sched.h \ /usr/include/bits/setjmp.h ../include/buffer.h ../include/server.h \ ../include/session.h ../include/service.h ../include/modules.h \ - /usr/include/errno.h /usr/include/bits/errno.h \ + ../include/router.h /usr/include/errno.h /usr/include/bits/errno.h \ /usr/include/linux/errno.h /usr/include/asm/errno.h \ /usr/include/asm-generic/errno.h /usr/include/asm-generic/errno-base.h \ ../include/gw.h /usr/include/ctype.h /usr/include/netdb.h \ diff --git a/include/spinlock.h b/include/spinlock.h index 5957a0f47..66706d3fd 100644 --- a/include/spinlock.h +++ b/include/spinlock.h @@ -31,7 +31,7 @@ #include typedef struct spinlock { - volatile int lock; + int lock; #if DEBUG int spins; int acquired; diff --git a/modules/protocol/mysql_client.c b/modules/protocol/mysql_client.c index 7e7c76fbd..d5d2ca4b8 100644 --- a/modules/protocol/mysql_client.c +++ b/modules/protocol/mysql_client.c @@ -783,13 +783,9 @@ int gw_read_client_event(DCB* dcb) { // dcb still to be freed // this will propagate COM_QUIT to backends - //router->routeQuery(router_instance, rsession, queue); + router->routeQuery(router_instance, rsession, queue); // close client - //(dcb->func).close(dcb); - // remove dcb - //poll_remove_dcb(dcb); - // close the session - //router->closeSession(router_instance, rsession); + (dcb->func).close(dcb); // call errors, it will be removed after tests //(dcb->func).error(dcb); diff --git a/modules/routing/debugcli.c b/modules/routing/debugcli.c index 3911992b8..aaea4cf3d 100644 --- a/modules/routing/debugcli.c +++ b/modules/routing/debugcli.c @@ -170,10 +170,6 @@ closeSession(ROUTER *instance, void *router_session) CLI_INSTANCE *inst = (CLI_INSTANCE *)instance; CLI_SESSION *session = (CLI_SESSION *)router_session; - /* - * Close the connection to the backend - */ - session->session->client->func.close(session->session->client); spinlock_acquire(&inst->lock); if (inst->sessions == session) diff --git a/modules/routing/debugcmd.c b/modules/routing/debugcmd.c index cd472e967..f90cf0e00 100644 --- a/modules/routing/debugcmd.c +++ b/modules/routing/debugcmd.c @@ -180,7 +180,7 @@ char *saveptr, *delim = " \t\r\n"; { return 0; } - else + else if (argc >= 0) { for (i = 0; cmds[i].cmd; i++) {