From eadcf20081b18b28f344552a9902fea2404d55bd Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 1 Jun 2006 17:06:42 +0500 Subject: [PATCH 1/2] bug #16017 (memory leaks in embedded server) There actually was 3 different problems - hash_user_connections wasn't cleaned one strdupped database name wasn't freed and stmt->mem_root wasn't cleaned as it was replased with mysql->field_alloc for result For the last one - i made the library using stmt's fields to store result if it's the case. include/mysql.h: statement pointer added to the advanced_command to be checked in embedded server include/sql_common.h: stmt added to the cli_advanced_command interface libmysql/libmysql.c: stmt pointer now sent to advanced_command libmysqld/embedded_priv.h: it's enough to send database name to check_embedded_connection libmysqld/lib_sql.cc: Now we store result directly in the MYSQL_STMT structure to avoid extra copying libmysqld/libmysqld.c: it's enough to only send database pointer to check_embedded_connection sql-common/client.c: stmt fake attribute added to cli_advanced_command sql/sql_parse.cc: hash_user_connections isn't used if no access checks compiled --- include/mysql.h | 17 ++++++++++--- include/sql_common.h | 3 ++- libmysql/libmysql.c | 13 +++++----- libmysqld/embedded_priv.h | 6 ++--- libmysqld/lib_sql.cc | 53 +++++++++++++++++++++++---------------- libmysqld/libmysqld.c | 9 +++---- sql-common/client.c | 3 ++- sql/sql_parse.cc | 18 ++++++++----- 8 files changed, 74 insertions(+), 48 deletions(-) diff --git a/include/mysql.h b/include/mysql.h index 0949937814c..143f6752c46 100644 --- a/include/mysql.h +++ b/include/mysql.h @@ -216,6 +216,7 @@ enum mysql_rpl_type }; struct st_mysql_methods; +struct st_mysql_stmt; typedef struct st_mysql { @@ -269,6 +270,12 @@ typedef struct st_mysql from mysql_stmt_close if close had to cancel result set of this object. */ my_bool *unbuffered_fetch_owner; + /* + In embedded server it points to the statement that is processed + in the current query. We store some results directly in statement + fields then. + */ + struct st_mysql_stmt *current_stmt; } MYSQL; typedef struct st_mysql_res { @@ -636,7 +643,8 @@ typedef struct st_mysql_methods unsigned long header_length, const char *arg, unsigned long arg_length, - my_bool skip_check); + my_bool skip_check, + MYSQL_STMT *stmt); MYSQL_DATA *(*read_rows)(MYSQL *mysql,MYSQL_FIELD *mysql_fields, unsigned int fields); MYSQL_RES * (*use_result)(MYSQL *mysql); @@ -724,8 +732,11 @@ int STDCALL mysql_drop_db(MYSQL *mysql, const char *DB); */ #define simple_command(mysql, command, arg, length, skip_check) \ - (*(mysql)->methods->advanced_command)(mysql, command, \ - NullS, 0, arg, length, skip_check) + (*(mysql)->methods->advanced_command)(mysql, command, NullS, \ + 0, arg, length, skip_check, NULL) +#define stmt_command(mysql, command, arg, length, stmt) \ + (*(mysql)->methods->advanced_command)(mysql, command, NullS, \ + 0, arg, length, 1, stmt) unsigned long net_safe_read(MYSQL* mysql); #ifdef __NETWARE__ diff --git a/include/sql_common.h b/include/sql_common.h index c07a4a831bb..52d766d77d2 100644 --- a/include/sql_common.h +++ b/include/sql_common.h @@ -33,7 +33,8 @@ void mysql_read_default_options(struct st_mysql_options *options, my_bool cli_advanced_command(MYSQL *mysql, enum enum_server_command command, const char *header, ulong header_length, - const char *arg, ulong arg_length, my_bool skip_check); + const char *arg, ulong arg_length, my_bool skip_check, + MYSQL_STMT *stmt); void set_stmt_errmsg(MYSQL_STMT * stmt, const char *err, int errcode, const char *sqlstate); diff --git a/libmysql/libmysql.c b/libmysql/libmysql.c index 4b8f1c9da1f..0f246b3030e 100644 --- a/libmysql/libmysql.c +++ b/libmysql/libmysql.c @@ -2085,7 +2085,7 @@ mysql_stmt_prepare(MYSQL_STMT *stmt, const char *query, ulong length) mysql_use_result it won't be freed in mysql_stmt_free_result and we should get 'Commands out of sync' here. */ - if (simple_command(mysql, COM_CLOSE_STMT, buff, 4, 1)) + if (stmt_command(mysql, COM_CLOSE_STMT, buff, 4, stmt)) { set_stmt_errmsg(stmt, mysql->net.last_error, mysql->net.last_errno, mysql->net.sqlstate); @@ -2094,7 +2094,7 @@ mysql_stmt_prepare(MYSQL_STMT *stmt, const char *query, ulong length) stmt->state= MYSQL_STMT_INIT_DONE; } - if (simple_command(mysql, COM_PREPARE, query, length, 1)) + if (stmt_command(mysql, COM_PREPARE, query, length, stmt)) { set_stmt_errmsg(stmt, mysql->net.last_error, mysql->net.last_errno, mysql->net.sqlstate); @@ -2505,7 +2505,7 @@ static my_bool execute(MYSQL_STMT *stmt, char *packet, ulong length) buff[4]= (char) 0; /* no flags */ int4store(buff+5, 1); /* iteration count */ if (cli_advanced_command(mysql, COM_EXECUTE, buff, sizeof(buff), - packet, length, 1) || + packet, length, 1, NULL) || (*mysql->methods->read_query_result)(mysql)) { set_stmt_errmsg(stmt, net->last_error, net->last_errno, net->sqlstate); @@ -3279,7 +3279,8 @@ mysql_stmt_send_long_data(MYSQL_STMT *stmt, uint param_number, This is intentional to save bandwidth. */ if ((*mysql->methods->advanced_command)(mysql, COM_LONG_DATA, buff, - sizeof(buff), data, length, 1)) + sizeof(buff), data, length, 1, + NULL)) { set_stmt_errmsg(stmt, mysql->net.last_error, mysql->net.last_errno, mysql->net.sqlstate); @@ -4603,7 +4604,7 @@ my_bool STDCALL mysql_stmt_close(MYSQL_STMT *stmt) mysql->status= MYSQL_STATUS_READY; } int4store(buff, stmt->stmt_id); - if ((rc= simple_command(mysql, COM_CLOSE_STMT, buff, 4, 1))) + if ((rc= stmt_command(mysql, COM_CLOSE_STMT, buff, 4, stmt))) { set_stmt_errmsg(stmt, mysql->net.last_error, mysql->net.last_errno, mysql->net.sqlstate); @@ -4641,7 +4642,7 @@ my_bool STDCALL mysql_stmt_reset(MYSQL_STMT *stmt) mysql= stmt->mysql->last_used_con; int4store(buff, stmt->stmt_id); /* Send stmt id to server */ if ((*mysql->methods->advanced_command)(mysql, COM_RESET_STMT, buff, - sizeof(buff), 0, 0, 0)) + sizeof(buff), 0, 0, 0, 0)) { set_stmt_errmsg(stmt, mysql->net.last_error, mysql->net.last_errno, mysql->net.sqlstate); diff --git a/libmysqld/embedded_priv.h b/libmysqld/embedded_priv.h index d4316dff63f..1e1db071b33 100644 --- a/libmysqld/embedded_priv.h +++ b/libmysqld/embedded_priv.h @@ -24,9 +24,9 @@ C_MODE_START void lib_connection_phase(NET *net, int phase); -void init_embedded_mysql(MYSQL *mysql, int client_flag, char *db); -void *create_embedded_thd(int client_flag, char *db); -int check_embedded_connection(MYSQL *mysql); +void init_embedded_mysql(MYSQL *mysql, int client_flag); +void *create_embedded_thd(int client_flag); +int check_embedded_connection(MYSQL *mysql, const char *db); void free_old_query(MYSQL *mysql); void embedded_get_error(MYSQL *mysql); extern MYSQL_METHODS embedded_methods; diff --git a/libmysqld/lib_sql.cc b/libmysqld/lib_sql.cc index bf8c17a71af..1f5d749cbbf 100644 --- a/libmysqld/lib_sql.cc +++ b/libmysqld/lib_sql.cc @@ -47,6 +47,7 @@ C_MODE_START #include "errmsg.h" #include + void embedded_get_error(MYSQL *mysql) { THD *thd=(THD *) mysql->thd; @@ -66,7 +67,8 @@ void embedded_get_error(MYSQL *mysql) static my_bool emb_advanced_command(MYSQL *mysql, enum enum_server_command command, const char *header, ulong header_length, - const char *arg, ulong arg_length, my_bool skip_check) + const char *arg, ulong arg_length, my_bool skip_check, + MYSQL_STMT *stmt) { my_bool result= 1; THD *thd=(THD *) mysql->thd; @@ -90,6 +92,7 @@ emb_advanced_command(MYSQL *mysql, enum enum_server_command command, mysql->affected_rows= ~(my_ulonglong) 0; mysql->field_count= 0; net->last_errno= 0; + mysql->current_stmt= stmt; thd->store_globals(); // Fix if more than one connect /* @@ -183,7 +186,6 @@ static my_bool emb_read_prepare_result(MYSQL *mysql, MYSQL_STMT *stmt) mysql->server_status|= SERVER_STATUS_IN_TRANS; stmt->fields= mysql->fields; - stmt->mem_root= mysql->field_alloc; mysql->fields= NULL; } @@ -225,7 +227,7 @@ static int emb_stmt_execute(MYSQL_STMT *stmt) thd->client_param_count= stmt->param_count; thd->client_params= stmt->params; if (emb_advanced_command(stmt->mysql, COM_EXECUTE,0,0, - header, sizeof(header), 1) || + header, sizeof(header), 1, stmt) || emb_mysql_read_query_result(stmt->mysql)) { NET *net= &stmt->mysql->net; @@ -242,8 +244,6 @@ int emb_read_binary_rows(MYSQL_STMT *stmt) MYSQL_DATA *data; if (!(data= emb_read_rows(stmt->mysql, 0, 0))) return 1; - stmt->result= *data; - my_free((char *) data, MYF(0)); return 0; } @@ -298,7 +298,8 @@ my_bool emb_next_result(MYSQL *mysql) DBUG_ENTER("emb_next_result"); if (emb_advanced_command(mysql, COM_QUERY,0,0, - thd->query_rest.ptr(),thd->query_rest.length(),1) || + thd->query_rest.ptr(), + thd->query_rest.length(),1, 0) || emb_mysql_read_query_result(mysql)) DBUG_RETURN(1); @@ -482,14 +483,14 @@ void end_embedded_server() } /* extern "C" */ C_MODE_START -void init_embedded_mysql(MYSQL *mysql, int client_flag, char *db) +void init_embedded_mysql(MYSQL *mysql, int client_flag) { THD *thd = (THD *)mysql->thd; thd->mysql= mysql; mysql->server_version= server_version; } -void *create_embedded_thd(int client_flag, char *db) +void *create_embedded_thd(int client_flag) { THD * thd= new THD; thd->thread_id= thread_id++; @@ -515,8 +516,8 @@ void *create_embedded_thd(int client_flag, char *db) thd->init_for_queries(); thd->client_capabilities= client_flag; - thd->db= db; - thd->db_length= db ? strip_sp(db) : 0; + thd->db= NULL; + thd->db_length= 0; #ifndef NO_EMBEDDED_ACCESS_CHECKS thd->db_access= DB_ACLS; thd->master_access= ~NO_ACCESS; @@ -533,18 +534,18 @@ err: } #ifdef NO_EMBEDDED_ACCESS_CHECKS -int check_embedded_connection(MYSQL *mysql) +int check_embedded_connection(MYSQL *mysql, const char *db) { THD *thd= (THD*)mysql->thd; thd->host= (char*)my_localhost; thd->host_or_ip= thd->host; thd->user= my_strdup(mysql->user, MYF(0)); thd->priv_user= thd->user; - return check_user(thd, COM_CONNECT, NULL, 0, thd->db, true); + return check_user(thd, COM_CONNECT, NULL, 0, db, true); } #else -int check_embedded_connection(MYSQL *mysql) +int check_embedded_connection(MYSQL *mysql, const char *db) { THD *thd= (THD*)mysql->thd; int result; @@ -578,7 +579,7 @@ int check_embedded_connection(MYSQL *mysql) passwd_len= 0; if((result= check_user(thd, COM_CONNECT, - scramble_buff, passwd_len, thd->db, true))) + scramble_buff, passwd_len, db, true))) goto err; return 0; @@ -636,8 +637,9 @@ bool Protocol::send_fields(List *list, uint flag) DBUG_RETURN(0); field_count= list->elements; - field_alloc= &mysql->field_alloc; - if (!(client_field= thd->mysql->fields= + field_alloc= mysql->current_stmt ? &mysql->current_stmt->mem_root : + &mysql->field_alloc; + if (!(client_field= mysql->fields= (MYSQL_FIELD *)alloc_root(field_alloc, sizeof(MYSQL_FIELD) * field_count))) goto err; @@ -707,7 +709,7 @@ bool Protocol::send_fields(List *list, uint flag) client_field->max_length= 0; ++client_field; } - thd->mysql->field_count= field_count; + mysql->field_count= field_count; DBUG_RETURN(prepare_for_send(list)); err: @@ -736,13 +738,20 @@ bool Protocol_prep::write() if (!data) { - if (!(data= (MYSQL_DATA*) my_malloc(sizeof(MYSQL_DATA), - MYF(MY_WME | MY_ZEROFILL)))) - return true; + MYSQL *mysql= thd->mysql; + + if (mysql->current_stmt) + data= &mysql->current_stmt->result; + else + { + if (!(data= (MYSQL_DATA*) my_malloc(sizeof(MYSQL_DATA), + MYF(MY_WME | MY_ZEROFILL)))) + return true; + init_alloc_root(&data->alloc,8192,0); /* Assume rowlength < 8192 */ + data->alloc.min_malloc=sizeof(MYSQL_ROWS); + } alloc= &data->alloc; - init_alloc_root(alloc,8192,0); /* Assume rowlength < 8192 */ - alloc->min_malloc=sizeof(MYSQL_ROWS); data->rows=0; data->fields=field_count; data->prev_ptr= &data->data; diff --git a/libmysqld/libmysqld.c b/libmysqld/libmysqld.c index 6fa41fb3fd0..5505049e0e0 100644 --- a/libmysqld/libmysqld.c +++ b/libmysqld/libmysqld.c @@ -134,7 +134,6 @@ mysql_real_connect(MYSQL *mysql,const char *host, const char *user, const char *passwd, const char *db, uint port, const char *unix_socket,ulong client_flag) { - char *db_name; char name_buff[USERNAME_LENGTH]; DBUG_ENTER("mysql_real_connect"); @@ -197,13 +196,11 @@ mysql_real_connect(MYSQL *mysql,const char *host, const char *user, port=0; unix_socket=0; - db_name = db ? my_strdup(db,MYF(MY_WME)) : NULL; + mysql->thd= create_embedded_thd(client_flag); - mysql->thd= create_embedded_thd(client_flag, db_name); + init_embedded_mysql(mysql, client_flag); - init_embedded_mysql(mysql, client_flag, db_name); - - if (check_embedded_connection(mysql)) + if (check_embedded_connection(mysql, db)) goto error; if (mysql_init_charset(mysql)) diff --git a/sql-common/client.c b/sql-common/client.c index 3a598832253..ce49048ce79 100644 --- a/sql-common/client.c +++ b/sql-common/client.c @@ -648,7 +648,8 @@ void free_rows(MYSQL_DATA *cur) my_bool cli_advanced_command(MYSQL *mysql, enum enum_server_command command, const char *header, ulong header_length, - const char *arg, ulong arg_length, my_bool skip_check) + const char *arg, ulong arg_length, my_bool skip_check, + MYSQL_STMT *stmt __attribute__((unused))) { NET *net= &mysql->net; my_bool result= 1; diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 51ef3f31b26..37fce192aa1 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -54,8 +54,8 @@ extern "C" int gethostname(char *name, int namelen); static void time_out_user_resource_limits(THD *thd, USER_CONN *uc); #ifndef NO_EMBEDDED_ACCESS_CHECKS static int check_for_max_user_connections(THD *thd, USER_CONN *uc); -#endif static void decrease_user_connections(USER_CONN *uc); +#endif /* NO_EMBEDDED_ACCESS_CHECKS */ static bool check_db_used(THD *thd,TABLE_LIST *tables); static bool check_multi_update_lock(THD *thd, TABLE_LIST *tables, List *fields, SELECT_LEX *select_lex); @@ -137,6 +137,7 @@ inline bool all_tables_not_ok(THD *thd, TABLE_LIST *tables) #endif +#ifndef NO_EMBEDDED_ACCESS_CHECKS static HASH hash_user_connections; static int get_or_create_user_conn(THD *thd, const char *user, @@ -190,6 +191,7 @@ end: return return_val; } +#endif /* !NO_EMBEDDED_ACCESS_CHECKS */ /* @@ -231,11 +233,7 @@ int check_user(THD *thd, enum enum_server_command command, thd->db= 0; thd->db_length= 0; if (mysql_change_db(thd, db)) - { - if (thd->user_connect) - decrease_user_connections(thd->user_connect); DBUG_RETURN(-1); - } } else send_ok(thd); @@ -409,10 +407,12 @@ extern "C" void free_user(struct user_conn *uc) void init_max_user_conn(void) { +#ifndef NO_EMBEDDED_ACCESS_CHECKS (void) hash_init(&hash_user_connections,system_charset_info,max_connections, 0,0, (hash_get_key) get_key_conn, (hash_free_key) free_user, 0); +#endif } @@ -466,7 +466,6 @@ static int check_for_max_user_connections(THD *thd, USER_CONN *uc) (void) pthread_mutex_unlock(&LOCK_user_conn); DBUG_RETURN(error); } -#endif /* NO_EMBEDDED_ACCESS_CHECKS */ /* Decrease user connection count @@ -500,13 +499,18 @@ static void decrease_user_connections(USER_CONN *uc) DBUG_VOID_RETURN; } +#endif /* NO_EMBEDDED_ACCESS_CHECKS */ + void free_max_user_conn(void) { +#ifndef NO_EMBEDDED_ACCESS_CHECKS hash_free(&hash_user_connections); +#endif /* NO_EMBEDDED_ACCESS_CHECKS */ } + /* Mark all commands that somehow changes a table This is used to check number of updates / hour @@ -1476,9 +1480,11 @@ bool dispatch_command(enum enum_server_command command, THD *thd, } else { +#ifndef NO_EMBEDDED_ACCESS_CHECKS /* we've authenticated new user */ if (save_user_connect) decrease_user_connections(save_user_connect); +#endif /* NO_EMBEDDED_ACCESS_CHECKS */ x_free((gptr) save_db); x_free((gptr) save_user); } From b33085b49a2af96618769f1268dbd19574a4043e Mon Sep 17 00:00:00 2001 From: unknown Date: Tue, 18 Jul 2006 16:43:39 +0500 Subject: [PATCH 2/2] merging --- libmysql/libmysql.c | 5 +++-- libmysqld/lib_sql.cc | 14 -------------- libmysqld/libmysqld.c | 3 +-- sql/sql_parse.cc | 1 + tests/mysql_client_test.c | 2 +- 5 files changed, 6 insertions(+), 19 deletions(-) diff --git a/libmysql/libmysql.c b/libmysql/libmysql.c index b1fc99ad60e..4efc3c2fd1c 100644 --- a/libmysql/libmysql.c +++ b/libmysql/libmysql.c @@ -2699,7 +2699,8 @@ stmt_read_row_from_cursor(MYSQL_STMT *stmt, unsigned char **row) int4store(buff, stmt->stmt_id); int4store(buff + 4, stmt->prefetch_rows); /* number of rows to fetch */ if ((*mysql->methods->advanced_command)(mysql, COM_STMT_FETCH, - buff, sizeof(buff), NullS, 0, 1)) + buff, sizeof(buff), NullS, 0, + 1, NULL)) { set_stmt_errmsg(stmt, net->last_error, net->last_errno, net->sqlstate); return 1; @@ -4948,7 +4949,7 @@ static my_bool reset_stmt_handle(MYSQL_STMT *stmt, uint flags) char buff[MYSQL_STMT_HEADER]; /* packet header: 4 bytes for stmt id */ int4store(buff, stmt->stmt_id); if ((*mysql->methods->advanced_command)(mysql, COM_STMT_RESET, buff, - sizeof(buff), 0, 0, 0)) + sizeof(buff), 0, 0, 0, NULL)) { set_stmt_errmsg(stmt, mysql->net.last_error, mysql->net.last_errno, mysql->net.sqlstate); diff --git a/libmysqld/lib_sql.cc b/libmysqld/lib_sql.cc index 5fc0a007921..2640910990e 100644 --- a/libmysqld/lib_sql.cc +++ b/libmysqld/lib_sql.cc @@ -387,20 +387,6 @@ static MYSQL_RES * emb_store_result(MYSQL *mysql) return mysql_store_result(mysql); } -my_bool emb_next_result(MYSQL *mysql) -{ - THD *thd= (THD*)mysql->thd; - DBUG_ENTER("emb_next_result"); - - if (emb_advanced_command(mysql, COM_QUERY,0,0, - thd->query_rest.ptr(),thd->query_rest.length(), - 1, NULL) || - emb_mysql_read_query_result(mysql)) - DBUG_RETURN(1); - - DBUG_RETURN(0); /* No more results */ -} - int emb_read_change_user_result(MYSQL *mysql, char *buff __attribute__((unused)), const char *passwd __attribute__((unused))) diff --git a/libmysqld/libmysqld.c b/libmysqld/libmysqld.c index 1cb37e8252a..cb4fa104b4c 100644 --- a/libmysqld/libmysqld.c +++ b/libmysqld/libmysqld.c @@ -164,7 +164,6 @@ mysql_real_connect(MYSQL *mysql,const char *host, const char *user, port=0; unix_socket=0; - db_name = db ? my_strdup(db,MYF(MY_WME)) : NULL; /* Send client information for access check */ client_flag|=CLIENT_CAPABILITIES; @@ -175,7 +174,7 @@ mysql_real_connect(MYSQL *mysql,const char *host, const char *user, client_flag|=CLIENT_CONNECT_WITH_DB; mysql->info_buffer= my_malloc(MYSQL_ERRMSG_SIZE, MYF(0)); - mysql->thd= create_embedded_thd(client_flag, db_name); + mysql->thd= create_embedded_thd(client_flag); init_embedded_mysql(mysql, client_flag); diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 94e2d8e4c37..e37bc7cc3f7 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -306,6 +306,7 @@ int check_user(THD *thd, enum enum_server_command command, /* Send the error to the client */ net_send_error(thd); DBUG_RETURN(-1); + } } send_ok(thd); DBUG_RETURN(0); diff --git a/tests/mysql_client_test.c b/tests/mysql_client_test.c index 04eb427b35f..b81c0aef506 100644 --- a/tests/mysql_client_test.c +++ b/tests/mysql_client_test.c @@ -13100,7 +13100,7 @@ static void test_bug9478() int4store(buff, stmt->stmt_id); buff[4]= 1; /* prefetch rows */ rc= ((*mysql->methods->advanced_command)(mysql, COM_STMT_FETCH, buff, - sizeof(buff), 0,0,1) || + sizeof(buff), 0,0,1,NULL) || (*mysql->methods->read_query_result)(mysql)); DIE_UNLESS(rc); if (!opt_silent && i == 0)