Bug #468, http://bugs.skysql.com/show_bug.cgi?id=468, Query classifier accessed freed thread context. If parsing fails thd doesn't need to be freed because it holds correct information about command type.

session.c:session_setup_filters : fixed memory leak
hintparser.c: added token_free for HINT_TOKENs and fixed a few memory leaks.
mysql_client_server_protocol.h: added mysql_protocol_done which frees memory blocks pointed to by protocol members. Those can't be freed in dcb.c because dcb.c doesn't know about protocol's members.
mysql_backend.c:gw_backend_close: fixed memory leak
mysql_client.c: gw_client_close: fixed memory leak
mysql_common.c: added implementation of mysql_protocol_done
	:protocol_archive_srv_command: tried to fix memory leak. Some memory is still leaking according to valgrind. Removed use of uninitialized local variable len.
readwritesplit.c:execute_sescmd_in_backend: fixed a memory leak - visible only in DEBUG=Y build.
This commit is contained in:
VilhoRaatikka
2014-08-05 09:31:10 +03:00
parent 6ef5cbc609
commit 6b3c7041e3
7 changed files with 52 additions and 19 deletions

View File

@ -149,17 +149,17 @@ skygw_query_type_t skygw_query_classifier_get_type(
thd = get_or_create_thd_for_parsing(mysql, query_str); thd = get_or_create_thd_for_parsing(mysql, query_str);
if (thd == NULL) if (thd == NULL)
{
skygw_query_classifier_free(mysql);
}
/** Create parse_tree inside thd */
failp = create_parse_tree(thd);
if (failp)
{ {
skygw_query_classifier_free(mysql); skygw_query_classifier_free(mysql);
*p_mysql = NULL; *p_mysql = NULL;
goto return_qtype;
} }
/**
* Create parse_tree inside thd.
* thd and even lex are readable even if parser failed so let it
* continue despite failure.
*/
failp = create_parse_tree(thd);
qtype = resolve_query_type(thd); qtype = resolve_query_type(thd);
if (p_mysql == NULL) if (p_mysql == NULL)
@ -464,7 +464,7 @@ static skygw_query_type_t resolve_query_type(
type |= QUERY_TYPE_DISABLE_AUTOCOMMIT; type |= QUERY_TYPE_DISABLE_AUTOCOMMIT;
type |= QUERY_TYPE_BEGIN_TRX; type |= QUERY_TYPE_BEGIN_TRX;
} }
/** /**
* REVOKE ALL, ASSIGN_TO_KEYCACHE, * REVOKE ALL, ASSIGN_TO_KEYCACHE,
* PRELOAD_KEYS, FLUSH, RESET, CREATE|ALTER|DROP SERVER * PRELOAD_KEYS, FLUSH, RESET, CREATE|ALTER|DROP SERVER
*/ */

View File

@ -667,6 +667,7 @@ int i;
session->filters[i].session = head->session; session->filters[i].session = head->session;
session->filters[i].instance = head->instance; session->filters[i].instance = head->instance;
session->head = *head; session->head = *head;
free(head);
} }
for (i = 0; i < service->n_filters; i++) for (i = 0; i < service->n_filters; i++)

View File

@ -299,6 +299,7 @@ typedef struct {
void gw_mysql_close(MySQLProtocol **ptr); void gw_mysql_close(MySQLProtocol **ptr);
MySQLProtocol* mysql_protocol_init(DCB* dcb, int fd); MySQLProtocol* mysql_protocol_init(DCB* dcb, int fd);
void mysql_protocol_done (DCB* dcb);
MySQLProtocol *gw_mysql_init(MySQLProtocol *data); MySQLProtocol *gw_mysql_init(MySQLProtocol *data);
void gw_mysql_close(MySQLProtocol **ptr); void gw_mysql_close(MySQLProtocol **ptr);
int gw_receive_backend_auth(MySQLProtocol *protocol); int gw_receive_backend_auth(MySQLProtocol *protocol);

View File

@ -996,6 +996,8 @@ gw_backend_close(DCB *dcb)
/** Send COM_QUIT to the backend being closed */ /** Send COM_QUIT to the backend being closed */
mysql_send_com_quit(dcb, 0, quitbuf); mysql_send_com_quit(dcb, 0, quitbuf);
mysql_protocol_done(dcb);
if (session != NULL && session->state == SESSION_STATE_STOPPING) if (session != NULL && session->state == SESSION_STATE_STOPPING)
{ {

View File

@ -1325,6 +1325,7 @@ gw_client_close(DCB *dcb)
CHK_PROTOCOL(protocol); CHK_PROTOCOL(protocol);
} }
#endif #endif
mysql_protocol_done(dcb);
session = dcb->session; session = dcb->session;
/** /**

View File

@ -96,6 +96,31 @@ return_p:
} }
/**
* mysql_protocol_done
*
* free protocol allocations.
*
* @param dcb owner DCB
*
*/
void mysql_protocol_done (
DCB* dcb)
{
server_command_t* scmd = ((MySQLProtocol *)dcb->protocol)->protocol_cmd_history;
server_command_t* scmd2;
while (scmd != NULL)
{
scmd2 = scmd->scom_next;
free(scmd);
scmd = scmd2;
}
}
/** /**
* gw_mysql_close * gw_mysql_close
* *
@ -1601,7 +1626,7 @@ void protocol_archive_srv_command(
{ {
server_command_t* s1; server_command_t* s1;
server_command_t** s2; server_command_t** s2;
int len; int len = 0;
spinlock_acquire(&p->protocol_lock); spinlock_acquire(&p->protocol_lock);
@ -1617,9 +1642,7 @@ void protocol_archive_srv_command(
s2 = &p->protocol_cmd_history; s2 = &p->protocol_cmd_history;
if (*s2 != NULL) if (*s2 != NULL)
{ {
len = 0;
while ((*s2)->scom_next != NULL) while ((*s2)->scom_next != NULL)
{ {
*s2 = (*s2)->scom_next; *s2 = (*s2)->scom_next;

View File

@ -1132,6 +1132,8 @@ static int routeQuery(
break; break;
case MYSQL_COM_STMT_EXECUTE: case MYSQL_COM_STMT_EXECUTE:
/** Parsing is not needed for this type of packet */
#if defined(NOT_USED)
plainsqlbuf = gwbuf_clone_transform(querybuf, plainsqlbuf = gwbuf_clone_transform(querybuf,
GWBUF_TYPE_PLAINSQL); GWBUF_TYPE_PLAINSQL);
len = GWBUF_LENGTH(plainsqlbuf); len = GWBUF_LENGTH(plainsqlbuf);
@ -1140,7 +1142,8 @@ static int routeQuery(
memcpy(querystr, startpos, len); memcpy(querystr, startpos, len);
memset(&querystr[len], 0, 1); memset(&querystr[len], 0, 1);
qtype = skygw_query_classifier_get_type(querystr, 0, &mysql); qtype = skygw_query_classifier_get_type(querystr, 0, &mysql);
qtype |= QUERY_TYPE_EXEC_STMT; #endif
qtype = QUERY_TYPE_EXEC_STMT;
break; break;
case MYSQL_COM_SHUTDOWN: /**< 8 where should shutdown be routed ? */ case MYSQL_COM_SHUTDOWN: /**< 8 where should shutdown be routed ? */
@ -1923,7 +1926,8 @@ static bool select_connect_backend_servers(
} }
} }
} }
} } /*< log only */
/** /**
* Choose at least 1+min_nslaves (master and slave) and at most 1+max_nslaves * Choose at least 1+min_nslaves (master and slave) and at most 1+max_nslaves
* servers from the sorted list. First master found is selected. * servers from the sorted list. First master found is selected.
@ -2666,8 +2670,9 @@ static bool execute_sescmd_in_backend(
pthread_self(), pthread_self(),
dcb->fd, dcb->fd,
STRPACKETTYPE(cmd)))); STRPACKETTYPE(cmd))));
gwbuf_free(tmpbuf);
} }
#endif #endif /*< SS_DEBUG */
switch (scur->scmd_cur_cmd->my_sescmd_packet_type) { switch (scur->scmd_cur_cmd->my_sescmd_packet_type) {
case MYSQL_COM_CHANGE_USER: case MYSQL_COM_CHANGE_USER:
rc = dcb->func.auth( rc = dcb->func.auth(
@ -3065,7 +3070,7 @@ static bool router_option_configured(
} }
return succp; return succp;
} }
#endif #endif /*< NOT_USED */
static void rwsplit_process_router_options( static void rwsplit_process_router_options(
ROUTER_INSTANCE* router, ROUTER_INSTANCE* router,
@ -3346,7 +3351,7 @@ static void print_error_packet(
{ {
while ((buf = gwbuf_consume(buf, GWBUF_LENGTH(buf))) != NULL); while ((buf = gwbuf_consume(buf, GWBUF_LENGTH(buf))) != NULL);
} }
#endif #endif /*< SS_DEBUG */
} }
static int router_get_servercount( static int router_get_servercount(
@ -3568,10 +3573,10 @@ static prep_stmt_t* prep_stmt_init(
if (pstmt != NULL) if (pstmt != NULL)
{ {
#if defined(SS_DEBUG) #if defined(SS_DEBUG)
pstmt->pstmt_chk_top = CHK_NUM_PREP_STMT; pstmt->pstmt_chk_top = CHK_NUM_PREP_STMT;
pstmt->pstmt_chk_tail = CHK_NUM_PREP_STMT; pstmt->pstmt_chk_tail = CHK_NUM_PREP_STMT;
#endif #endif
pstmt->pstmt_state = PREP_STMT_ALLOC; pstmt->pstmt_state = PREP_STMT_ALLOC;
pstmt->pstmt_type = type; pstmt->pstmt_type = type;