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: Fix to bug #469, http://bugs.skysql.com/show_bug.cgi?id=469,  rwsplit counts every connection twice in master - counnection counts leak
	execute_sescmd_in_backend: fixed a memory leak - visible only in DEBUG=Y build.
readwritesplit/test/makefile: added target for hints tests
This commit is contained in:
VilhoRaatikka 2014-08-05 18:14:06 +03:00
parent 76d7f3d836
commit 08b99c121e
14 changed files with 170 additions and 22 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);
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);
*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);
if (p_mysql == NULL)

View File

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

View File

@ -77,9 +77,20 @@ static HINT *lookup_named_hint(HINT_SESSION *, char *);
static void create_named_hint(HINT_SESSION *, char *, HINT *);
static void hint_push(HINT_SESSION *, HINT *);
static const char* token_get_keyword (TOKEN_VALUE token);
static void token_free(HINT_TOKEN* token);
typedef enum { HM_EXECUTE, HM_START, HM_PREPARE } HINT_MODE;
void token_free(HINT_TOKEN* token)
{
if (token->value != NULL)
{
free(token->value);
}
free(token);
}
static const char* token_get_keyword (
TOKEN_VALUE token)
{
@ -224,11 +235,13 @@ HINT_MODE mode = HM_EXECUTE;
"with keyword '%s'",
token_get_keyword(tok->token),
token_get_keyword(TOK_MAXSCALE))));
free(tok);
token_free(tok);
goto retblock;
}
token_free(tok);
state = HS_INIT;
while ((tok = hint_next_token(&buf, &ptr)) != NULL
&& tok->token != TOK_EOL)
{
@ -333,8 +346,13 @@ HINT_MODE mode = HM_EXECUTE;
}
break;
}
free(tok);
}
token_free(tok);
} /*< while */
if (tok->token == TOK_EOL)
{
token_free(tok);
}
switch (mode)
{

View File

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

View File

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

View File

@ -1325,6 +1325,7 @@ gw_client_close(DCB *dcb)
CHK_PROTOCOL(protocol);
}
#endif
mysql_protocol_done(dcb);
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
*
@ -1601,7 +1626,7 @@ void protocol_archive_srv_command(
{
server_command_t* s1;
server_command_t** s2;
int len;
int len = 0;
spinlock_acquire(&p->protocol_lock);
@ -1617,9 +1642,7 @@ void protocol_archive_srv_command(
s2 = &p->protocol_cmd_history;
if (*s2 != NULL)
{
len = 0;
{
while ((*s2)->scom_next != NULL)
{
*s2 = (*s2)->scom_next;

View File

@ -1272,6 +1272,8 @@ static int routeQuery(
break;
case MYSQL_COM_STMT_EXECUTE:
/** Parsing is not needed for this type of packet */
#if defined(NOT_USED)
plainsqlbuf = gwbuf_clone_transform(querybuf,
GWBUF_TYPE_PLAINSQL);
len = GWBUF_LENGTH(plainsqlbuf);
@ -1280,7 +1282,8 @@ static int routeQuery(
memcpy(querystr, startpos, len);
memset(&querystr[len], 0, 1);
qtype = skygw_query_classifier_get_type(querystr, 0, &mysql);
qtype |= QUERY_TYPE_EXEC_STMT;
#endif
qtype = QUERY_TYPE_EXEC_STMT;
break;
case MYSQL_COM_SHUTDOWN: /**< 8 where should shutdown be routed ? */
@ -2146,7 +2149,7 @@ static bool select_connect_backend_servers(
backend_ref[i].bref_state = 0;
bref_set_state(&backend_ref[i],
BREF_IN_USE);
/**
/**
* Increase backend connection counter.
* Server's stats are _increased_ in
* dcb.c:dcb_alloc !
@ -2184,7 +2187,7 @@ static bool select_connect_backend_servers(
session,
b->backend_server->protocol);
if (backend_ref[i].bref_dcb != NULL)
if (backend_ref[i].bref_dcb != NULL)
{
master_connected = true;
/**
@ -2201,8 +2204,6 @@ static bool select_connect_backend_servers(
bref_set_state(&backend_ref[i],
BREF_IN_USE);
/** Increase backend connection counters */
atomic_add(&b->backend_server->stats.n_current, 1);
atomic_add(&b->backend_server->stats.n_connections, 1);
atomic_add(&b->backend_conn_count, 1);
}
else
@ -2832,6 +2833,7 @@ static bool execute_sescmd_in_backend(
pthread_self(),
dcb->fd,
STRPACKETTYPE(cmd))));
gwbuf_free(tmpbuf);
}
#endif /*< SS_DEBUG */
switch (scur->scmd_cur_cmd->my_sescmd_packet_type) {

View File

@ -14,6 +14,7 @@ RET := -1
cleantests:
- $(DEL) *.o
- $(DEL) *~
-$(MAKE) -C test_hints cleantests
testall:
@ -32,4 +33,5 @@ runtests:
@echo "-------------------------------" >> $(TESTLOG)
./rwsplit.sh $(TESTLOG) $(THOST) $(TPORT_RW) $(TMASTER_ID) $(TUSER) $(TPWD)
@echo "" >> $(TESTLOG)
@cat $(TESTLOG) >> $(TEST_MAXSCALE_LOG)
@cat $(TESTLOG) >> $(TEST_MAXSCALE_LOG)
-$(MAKE) -C test_hints runtests

View File

@ -0,0 +1,37 @@
# cleantests - clean local and subdirectories' tests
# buildtests - build all local and subdirectories' tests
# runtests - run all local tests
# testall - clean, build and run local and subdirectories' tests
include ../../../../../../build_gateway.inc
include $(ROOT_PATH)/makefile.inc
include $(ROOT_PATH)/test.inc
ARGS=6
CC=cc
TESTLOG := $(shell pwd)/testrwsplit_hints.log
RET := -1
cleantests:
- $(DEL) *.o
- $(DEL) *~
testall:
-$(MAKE) cleantests
-$(MAKE) DEBUG=Y buildtests
-$(MAKE) runtests
buildtests:
runtests:
@echo "" > $(TESTLOG)
@echo "-------------------------------" >> $(TESTLOG)
@echo $(shell date) >> $(TESTLOG)
@echo "Test Read/Write split router - hint routing" >> $(TESTLOG)
@echo "-------------------------------" >> $(TESTLOG)
./rwsplit_hints.sh $(TESTLOG) $(THOST) $(TPORT_RW) $(TMASTER_ID) $(TUSER) $(TPWD)
@echo "" >> $(TESTLOG)
@cat $(TESTLOG) >> $(TEST_MAXSCALE_LOG)

View File

@ -0,0 +1,9 @@
-- maxscale route to master:3000
-- maxscale route to server server1:3000
-- maxscale route to server server2:3001
-- maxscale route to server server3:3002
-- maxscale route to server server4:3003
-- maxscale max_slave_replication_lag = 100:
-- maxscale max_slave_replication_lag= 100:
-- maxscale max_slave_replication_lag =100:
-- maxscale max_slave_replication_lag=100:

View File

@ -0,0 +1,36 @@
#!/bin/bash
NARGS=6
TLOG=$1
THOST=$2
TPORT=$3
TMASTER_ID=$4
TUSER=$5
TPWD=$6
if [ $# != $NARGS ] ;
then
echo""
echo "Wrong number of arguments, gave "$#" but "$NARGS" is required"
echo ""
echo "Usage :"
echo " rwsplit.sh <log filename> <host> <port> <master id> <user> <password>"
echo ""
exit 1
fi
TESTINPUT=hints.txt
QUERY="select @@server_id;"
RUNCMD=mysql\ --host=$THOST\ -P$TPORT\ -u$TUSER\ -p$TPWD\ --unbuffered=true\ --disable-reconnect\ --silent\
while read -r LINE
do
TINPUT=`echo "$LINE"|awk '{split($0,a,":");print a[1]}'`
TRETVAL=`echo "$LINE"|awk '{split($0,a,":");print a[2]}'`
a=`$RUNCMD -e"$QUERY$TINPUT"`
if [ "$a" != "$TRETVAL" ]; then
echo "$TINPUT FAILED, return value $a when $TRETVAL was expected">>$TLOG;
else
echo "$TINPUT PASSED">>$TLOG ;
fi
done < $TESTINPUT

View File

@ -0,0 +1,15 @@
-------------------------------
Tue Aug 5 13:31:41 EEST 2014
Test Read/Write split router - hint routing
-------------------------------
-- maxscale route to master FAILED, return value 3003 when 3000 was expected
-- maxscale route to server server1 FAILED, return value 3003 when 3000 was expected
-- maxscale route to server server2 FAILED, return value 3003 when 3001 was expected
-- maxscale route to server server3 FAILED, return value 3003 when 3002 was expected
-- maxscale route to server server4 PASSED
-- maxscale max_slave_replication_lag = 100 FAILED, return value 3003 when was expected
-- maxscale max_slave_replication_lag= 100 FAILED, return value 3003 when was expected
-- maxscale max_slave_replication_lag =100 FAILED, return value 3003 when was expected
-- maxscale max_slave_replication_lag=100 FAILED, return value 3003 when was expected

View File

@ -133,7 +133,8 @@ typedef enum skygw_chk_t {
((t) == QUERY_TYPE_SESSION_WRITE ? "QUERY_TYPE_SESSION_WRITE" : \
((t) == QUERY_TYPE_UNKNOWN ? "QUERY_TYPE_UNKNOWN" : \
((t) == QUERY_TYPE_LOCAL_READ ? "QUERY_TYPE_LOCAL_READ" : \
"Unknown query type")))))
((t) == QUERY_TYPE_EXEC_STMT ? "QUERY_TYPE_EXEC_STMT" : \
"Unknown query type"))))))
#define STRLOGID(i) ((i) == LOGFILE_TRACE ? "LOGFILE_TRACE" : \
((i) == LOGFILE_MESSAGE ? "LOGFILE_MESSAGE" : \