diff --git a/mysql-test/main/ps_mem_leaks.result b/mysql-test/main/ps_mem_leaks.result index ebbccb71c6b..e934c24288c 100644 --- a/mysql-test/main/ps_mem_leaks.result +++ b/mysql-test/main/ps_mem_leaks.result @@ -148,3 +148,32 @@ CALL p1(2); DROP TABLE t1; DROP PROCEDURE p1; # End of 10.11 tests +# +# MDEV-34517: Memory leak on re-compilation of a failing statement inside a stored routine +# +CREATE TABLE t1 (a INT); +CREATE PROCEDURE p1() +SELECT * FROM t1; +SET @save_dbug = @@debug_dbug; +CALL p1(); +a +DROP TABLE t1; +CREATE TABLE t1 (a INT); +SET @@debug_dbug='+d,check_sp_cache_not_invalidated,sp_instr_reparsing_1st_time'; +# Recomplilation of the statement 'SELECT * FORM t1' on +# the second run of the procedure p1 shouldn't result in memory leaks +CALL p1(); +a +SET @@debug_dbug='-d,sp_instr_reparsing_1st_time'; +DROP TABLE t1; +CREATE TABLE t1 (a INT); +# Recompilation is requested the second time +SET @@debug_dbug='+d,sp_instr_reparsing_2nd_time'; +CALL p1(); +a +SET @@debug_dbug='-d,sp_instr_reparsing_2nd_time'; +# Clean up +SET @@debug_dbug=@save_dbug; +DROP TABLE t1; +DROP PROCEDURE p1; +# End of 11.2 tests diff --git a/mysql-test/main/ps_mem_leaks.test b/mysql-test/main/ps_mem_leaks.test index 75381e13116..17baf122d16 100644 --- a/mysql-test/main/ps_mem_leaks.test +++ b/mysql-test/main/ps_mem_leaks.test @@ -167,3 +167,36 @@ DROP TABLE t1; DROP PROCEDURE p1; --echo # End of 10.11 tests + +--echo # +--echo # MDEV-34517: Memory leak on re-compilation of a failing statement inside a stored routine +--echo # +CREATE TABLE t1 (a INT); + +CREATE PROCEDURE p1() + SELECT * FROM t1; + +SET @save_dbug = @@debug_dbug; + +CALL p1(); +DROP TABLE t1; +CREATE TABLE t1 (a INT); +SET @@debug_dbug='+d,check_sp_cache_not_invalidated,sp_instr_reparsing_1st_time'; +--echo # Recomplilation of the statement 'SELECT * FORM t1' on +--echo # the second run of the procedure p1 shouldn't result in memory leaks +CALL p1(); +SET @@debug_dbug='-d,sp_instr_reparsing_1st_time'; + +DROP TABLE t1; +CREATE TABLE t1 (a INT); +--echo # Recompilation is requested the second time +SET @@debug_dbug='+d,sp_instr_reparsing_2nd_time'; +CALL p1(); +SET @@debug_dbug='-d,sp_instr_reparsing_2nd_time'; + +--echo # Clean up +SET @@debug_dbug=@save_dbug; +DROP TABLE t1; +DROP PROCEDURE p1; + +--echo # End of 11.2 tests diff --git a/sql/sp_instr.cc b/sql/sp_instr.cc index 4ffb49a8cf2..457be739926 100644 --- a/sql/sp_instr.cc +++ b/sql/sp_instr.cc @@ -653,6 +653,84 @@ bool sp_lex_instr::setup_table_fields_for_trigger( return result; } + +/** + Initialize a new memory root for re-parsing a failed SP instruction's + statement or free a memory allocated on re-parsing of the failed statement + and re-initialize it again so to avoid memory leaks on repeating a statement + re-parsing. + + @param sphead The stored program. + + @return false on success, true on error (OOM) +*/ + +bool sp_lex_instr::setup_memroot_for_reparsing(sp_head *sphead) +{ + if (!m_mem_root_for_reparsing) + { + DBUG_EXECUTE_IF("sp_instr_reparsing_2nd_time", DBUG_ASSERT(0);); + /* + Allocate a memory for SP-instruction's mem_root on a mem_root of sp_head. + Since the method sp_lex_instr::setup_memroot_for_reparsing() is called + on failing execution of SP-instruction by the reason of changes in data + dictionary objects metadata, the sp_head mem_root protection flag could + has been already set on first execution of the stored routine. Therefore, + clear the flag + ROOT_FLAG_READ_ONLY + in case it is set before allocating a memory for SP instruction's + mem_root on sp_head's mem_root and restore its original value once + the memory for the SP-instruction's new_root allocated. Read only + property for the stored routine's mem_root can be not set after first + invocation of a stored routine in case it was completed with error. + So, check the flag is set before resetting its value and restoring its + original value on return. + */ + MEM_ROOT *sphead_mem_root= sphead->get_main_mem_root(); + +#ifdef PROTECT_STATEMENT_MEMROOT + const bool read_only_mem_root= + (sphead_mem_root->flags & ROOT_FLAG_READ_ONLY); + + if (read_only_mem_root) + sphead_mem_root->flags&= ~ROOT_FLAG_READ_ONLY; +#endif + + m_mem_root_for_reparsing= + (MEM_ROOT*)alloc_root(sphead_mem_root, sizeof(MEM_ROOT)); + +#ifdef PROTECT_STATEMENT_MEMROOT + if (read_only_mem_root) + /* + Restore original read only property of sp_head' s mem_root + in case it was set + */ + sphead_mem_root->flags|= ROOT_FLAG_READ_ONLY; +#endif + + if (!m_mem_root_for_reparsing) + return true; + } + else + { + DBUG_EXECUTE_IF("sp_instr_reparsing_1st_time", DBUG_ASSERT(0);); + /* + Free a memory allocated on SP-instruction's mem_root to avoid + memory leaks could take place on recompilation of SP-instruction's + statement. + */ + free_root(m_mem_root_for_reparsing, MYF(0)); + } + + init_sql_alloc(key_memory_sp_head_main_root, m_mem_root_for_reparsing, + MEM_ROOT_BLOCK_SIZE, MEM_ROOT_PREALLOC, MYF(0)); + + mem_root= m_mem_root_for_reparsing; + + return false; +} + + LEX* sp_lex_instr::parse_expr(THD *thd, sp_head *sp, LEX *sp_instr_lex) { String sql_query; @@ -704,6 +782,18 @@ LEX* sp_lex_instr::parse_expr(THD *thd, sp_head *sp, LEX *sp_instr_lex) SP arena's state to STMT_INITIALIZED_FOR_SP as its initial state. */ state= STMT_INITIALIZED_FOR_SP; + + /* + First, set up a men_root for the statement is going to re-compile. + */ + if (setup_memroot_for_reparsing(sp)) + return nullptr; + + /* + and then set it as the current mem_root. Any memory allocations can take + place on re-parsing the SP-instruction's statement will be performed on + this mem_root. + */ thd->set_n_backup_active_arena(this, &backup); thd->free_list= nullptr; diff --git a/sql/sp_instr.h b/sql/sp_instr.h index ae466608663..def5206d250 100644 --- a/sql/sp_instr.h +++ b/sql/sp_instr.h @@ -278,6 +278,7 @@ public: { if (m_lex_resp) { + m_lex_resp= false; /* Prevent endless recursion. */ m_lex->sphead= nullptr; lex_end(m_lex); @@ -397,9 +398,30 @@ class sp_lex_instr : public sp_instr public: sp_lex_instr(uint ip, sp_pcontext *ctx, LEX *lex, bool is_lex_owner) : sp_instr(ip, ctx), - m_lex_keeper(lex, is_lex_owner) + m_lex_keeper(lex, is_lex_owner), + m_mem_root_for_reparsing(nullptr) {} + ~sp_lex_instr() override + { + if (m_mem_root_for_reparsing) + { + /* + Free items owned by an instance of sp_lex_instr and call m_lex_keeper's + destructor explicitly to avoid referencing a deallocated memory + owned by the memory root m_mem_root_for_reparsing that else would take + place in case their implicit invocations (in that case, m_lex_keeper's + destructor and the method free_items() called by ~sp_instr are invoked + after the memory owned by the memory root m_mem_root_for_reparsing + be freed, that would result in abnormal server termination) + */ + free_items(); + m_lex_keeper.~sp_lex_keeper(); + free_root(m_mem_root_for_reparsing, MYF(0)); + m_mem_root_for_reparsing= nullptr; + } + } + virtual bool is_invalid() const = 0; virtual void invalidate() = 0; @@ -469,6 +491,12 @@ private: */ SQL_I_List m_cur_trigger_stmt_items; + /** + MEM_ROOT used for allocation of memory on re-parsing of a statement + caused failure of SP-instruction execution + */ + MEM_ROOT *m_mem_root_for_reparsing; + /** Clean up items previously created on behalf of the current instruction. */ @@ -492,6 +520,8 @@ private: bool setup_table_fields_for_trigger( THD *thd, sp_head *sp, SQL_I_List *next_trig_items_list); + + bool setup_memroot_for_reparsing(sp_head *sphead); };