MDEV-34517: Memory leak on re-compilation of a failing statement inside a stored routine

SP instructions, consisting a body of a stored routine, had the same memory
root as an instance of the class sp_head, representing abstraction for stored
routine itself. It resulted in memory leaks on re-parsing a failed statement
of a stored routine in case the statement re-compilation has to be performed
by the reason of changes in metadata of tables, triggers, etc. the stored
routine depends on.

To fix this kind of memory leaks, every SP instruction requiring access to
a LEX object must do re-parsing of a failed statement on its own memory root.
These memory roots are allocated on sp_head's memory root and every instance of
the sp_lex_instr class has a pointer to allocated memory root in case re-parsing
of the correspondiong SP instruction was requested. On every subsequent
re-parsing of the failed statement, a memory allocated on SP instruction's
memory root is released and the memory root re-initialized. Following memory
allocations taken place on re-parsing the SP instruction's statement
is performed on the dedicated memory root. So, no memory leaks will happen on
SP statement re-parsing.

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

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

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;
 

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<Item_trigger_field> 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<Item_trigger_field> *next_trig_items_list);
+
+  bool setup_memroot_for_reparsing(sp_head *sphead);
 };