From 9937d5f0a91bb8fdceb7d2110e22c03878242a29 Mon Sep 17 00:00:00 2001 From: Andrei Elkin Date: Tue, 29 Nov 2011 20:17:02 +0200 Subject: [PATCH] Bug#13437900 - VALGRIND REPORTS A LEAK FOR REPL_IGNORE_SERVER_IDS There was memory leak when running some tests on PB2. The reason of the failure is an early return from change_master() that was supposed to deallocate a dyn-array. Fixed with relocating the dyn-array's destructor at ~LEX() that is the end of the session, per Gleb's patch idea. Two optimizations were done: the static buffer for the dyn-array to base on, and the array initialization is called precisely when it's necessary rather than per each CHANGE-MASTER as before. --- .../suite/rpl/t/rpl_empty_master_host.test | 1 + sql/sql_lex.cc | 1 + sql/sql_lex.h | 7 +++++++ sql/sql_repl.cc | 1 - sql/sql_yacc.yy | 16 ++++++++++------ 5 files changed, 19 insertions(+), 7 deletions(-) diff --git a/mysql-test/suite/rpl/t/rpl_empty_master_host.test b/mysql-test/suite/rpl/t/rpl_empty_master_host.test index df0c85ad7ec..66d30375a59 100644 --- a/mysql-test/suite/rpl/t/rpl_empty_master_host.test +++ b/mysql-test/suite/rpl/t/rpl_empty_master_host.test @@ -17,6 +17,7 @@ # working when expected. --source include/master-slave.inc +--source include/have_binlog_format_mixed.inc connection slave; STOP SLAVE; diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index 869a5916339..00a67e2c134 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -2352,6 +2352,7 @@ LEX::LEX() INITIAL_LEX_PLUGIN_LIST_SIZE, INITIAL_LEX_PLUGIN_LIST_SIZE); reset_query_tables_list(TRUE); + repl_ignore_server_ids_inited= false; } diff --git a/sql/sql_lex.h b/sql/sql_lex.h index 542e8b42ae2..474fa87495b 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -291,7 +291,9 @@ typedef struct st_lex_master_info char *ssl_key, *ssl_cert, *ssl_ca, *ssl_capath, *ssl_cipher; char *relay_log_name; ulong relay_log_pos; + bool repl_ignore_server_ids_inited; DYNAMIC_ARRAY repl_ignore_server_ids; + typeof(::server_id) repl_ignore_server_ids_static_buffer[4]; } LEX_MASTER_INFO; typedef struct st_lex_reset_slave @@ -2455,6 +2457,11 @@ struct LEX: public Query_tables_list destroy_query_tables_list(); plugin_unlock_list(NULL, (plugin_ref *)plugins.buffer, plugins.elements); delete_dynamic(&plugins); + if (mi.repl_ignore_server_ids_inited) + { + delete_dynamic(&mi.repl_ignore_server_ids); + mi.repl_ignore_server_ids_inited= false; + } } inline bool is_ps_or_view_context_analysis() diff --git a/sql/sql_repl.cc b/sql/sql_repl.cc index 00c85d8eb43..5300a327029 100644 --- a/sql/sql_repl.cc +++ b/sql/sql_repl.cc @@ -1689,7 +1689,6 @@ err: thd_proc_info(thd, 0); if (ret == FALSE) my_ok(thd); - delete_dynamic(&lex_mi->repl_ignore_server_ids); //freeing of parser-time alloc DBUG_RETURN(ret); } diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 87c3ae5b129..209ec1ff0fc 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -1863,12 +1863,7 @@ change: LEX *lex = Lex; lex->sql_command = SQLCOM_CHANGE_MASTER; bzero((char*) &lex->mi, sizeof(lex->mi)); - /* - resetting flags that can left from the previous CHANGE MASTER - */ lex->mi.repl_ignore_server_ids_opt= LEX_MASTER_INFO::LEX_MI_UNCHANGED; - my_init_dynamic_array(&Lex->mi.repl_ignore_server_ids, - sizeof(::server_id), 16, 16); } master_defs {} @@ -1965,7 +1960,7 @@ master_def: | IGNORE_SERVER_IDS_SYM EQ '(' ignore_server_id_list ')' { Lex->mi.repl_ignore_server_ids_opt= LEX_MASTER_INFO::LEX_MI_ENABLE; - } + } | master_file_def ; @@ -1979,6 +1974,15 @@ ignore_server_id_list: ignore_server_id: ulong_num { + if (!Lex->mi.repl_ignore_server_ids_inited) + { + my_init_dynamic_array2(&Lex->mi.repl_ignore_server_ids, + sizeof(::server_id), + Lex->mi.repl_ignore_server_ids_static_buffer, + array_elements(Lex->mi.repl_ignore_server_ids_static_buffer), + 16); + Lex->mi.repl_ignore_server_ids_inited= true; + } insert_dynamic(&Lex->mi.repl_ignore_server_ids, (uchar*) &($1)); }