MDEV-32466: Potential memory leak on executing of create view statement

This is the follow-up patch that removes explicit use of thd->stmt_arena for memory allocation and replaces it with call of the method THD::active_stmt_arena_to_use() Additionally, this patch adds extra DBUG_ASSERT to check that right query arena is in use.
MariaDB · Nov 24, 2023 · 85f2e4f · 85f2e4f
1 parent 5064750
commit 85f2e4f
Show file tree

Hide file tree

Showing 10 changed files with 49 additions and 15 deletions.
diff --git a/sql/item_func.cc b/sql/item_func.cc
@@ -2740,8 +2740,17 @@ bool Item_func_rand::fix_fields(THD *thd,Item **ref)
       No need to send a Rand log event if seed was given eg: RAND(seed),
       as it will be replicated in the query as such.
     */
+    DBUG_ASSERT((!rand &&
+                 (thd->active_stmt_arena_to_use()->
+                    is_stmt_prepare_or_first_stmt_execute() ||
+                  thd->active_stmt_arena_to_use()->
+                    is_conventional() ||
+                  thd->active_stmt_arena_to_use()->state ==
+                    Query_arena::STMT_SP_QUERY_ARGUMENTS
+                 )
+                ) || rand);
     if (!rand && !(rand= (struct my_rnd_struct*)
-                   thd->stmt_arena->alloc(sizeof(*rand))))
+                   thd->active_stmt_arena_to_use()->alloc(sizeof(*rand))))
       return TRUE;
   }
   else

diff --git a/sql/item_jsonfunc.cc b/sql/item_jsonfunc.cc
@@ -827,7 +827,7 @@ static int alloc_tmp_paths(THD *thd, uint n_paths,
   {
     if (*tmp_paths == 0)
     {
-      MEM_ROOT *root= thd->stmt_arena->mem_root;
+      MEM_ROOT *root= thd->active_stmt_arena_to_use()->mem_root;
 
       *paths= (json_path_with_flags *) alloc_root(root,
           sizeof(json_path_with_flags) * n_paths);

diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc
@@ -3200,8 +3200,12 @@ bool Item_exists_subselect::exists2in_processor(void *opt_arg)
           if (eqs.at(i).outer_exp->
               walk(&Item::find_item_processor, TRUE, upper->item))
             break;
+        DBUG_ASSERT(thd->stmt_arena->is_stmt_prepare_or_first_stmt_execute() ||
+                    thd->stmt_arena->is_conventional());
+        DBUG_ASSERT(thd->stmt_arena->mem_root == thd->mem_root);
         if (i == (uint)eqs.elements() &&
-            (in_subs->upper_refs.push_back(upper, thd->stmt_arena->mem_root)))
+            (in_subs->upper_refs.push_back(
+               upper, thd->mem_root)))
           goto out;
       }
     }

diff --git a/sql/item_sum.cc b/sql/item_sum.cc
@@ -4098,8 +4098,14 @@ Item_func_group_concat::fix_fields(THD *thd, Item **ref)
     char *buf;
     String *new_separator;
 
-    if (!(buf= (char*) thd->stmt_arena->alloc(buflen)) ||
-        !(new_separator= new(thd->stmt_arena->mem_root)
+    DBUG_ASSERT(thd->active_stmt_arena_to_use()->
+                  is_stmt_prepare_or_first_sp_execute() ||
+                thd->active_stmt_arena_to_use()->
+                  is_conventional() ||
+                thd->active_stmt_arena_to_use()->state ==
+                  Query_arena::STMT_SP_QUERY_ARGUMENTS);
+    if (!(buf= (char*) thd->active_stmt_arena_to_use()->alloc(buflen)) ||
+        !(new_separator= new(thd->active_stmt_arena_to_use()->mem_root)
                            String(buf, buflen, collation.collation)))
       return TRUE;
 

diff --git a/sql/sp.cc b/sql/sp.cc
@@ -2694,7 +2694,13 @@ sp_update_stmt_used_routines(THD *thd, Query_tables_list *prelocking_ctx,
   for (uint i=0 ; i < src->records ; i++)
   {
     Sroutine_hash_entry *rt= (Sroutine_hash_entry *)my_hash_element(src, i);
-    (void)sp_add_used_routine(prelocking_ctx, thd->stmt_arena,
+    DBUG_ASSERT(thd->active_stmt_arena_to_use()->
+                  is_stmt_prepare_or_first_stmt_execute() ||
+                thd->active_stmt_arena_to_use()->
+                  is_conventional() ||
+                thd->active_stmt_arena_to_use()->state ==
+                  Query_arena::STMT_SP_QUERY_ARGUMENTS);
+    (void)sp_add_used_routine(prelocking_ctx, thd->active_stmt_arena_to_use(),
                               &rt->mdl_request.key, rt->m_handler,
                               belong_to_view);
   }
@@ -2720,7 +2726,6 @@ void sp_update_stmt_used_routines(THD *thd, Query_tables_list *prelocking_ctx,
                                   TABLE_LIST *belong_to_view)
 {
   for (Sroutine_hash_entry *rt= src->first; rt; rt= rt->next)
-
     (void)sp_add_used_routine(prelocking_ctx, thd->active_stmt_arena_to_use(),
                               &rt->mdl_request.key, rt->m_handler,
                               belong_to_view);

diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc
@@ -4137,18 +4137,24 @@ static void fix_prepare_info_in_table_list(THD *thd, TABLE_LIST *tbl)
 void st_select_lex::fix_prepare_information(THD *thd, Item **conds, 
                                             Item **having_conds)
 {
+  Query_arena *active_arena= thd->active_stmt_arena_to_use();
+
   DBUG_ENTER("st_select_lex::fix_prepare_information");
-  if (!thd->stmt_arena->is_conventional() &&
+
+  if (!active_arena->is_conventional() &&
       !(changed_elements & TOUCHED_SEL_COND))
   {
     Query_arena_stmt on_stmt_arena(thd);
     changed_elements|= TOUCHED_SEL_COND;
+    DBUG_ASSERT(
+      active_arena->is_stmt_prepare_or_first_stmt_execute() ||
+      active_arena->state == Query_arena::STMT_SP_QUERY_ARGUMENTS);
     if (group_list.first)
     {
       if (!group_list_ptrs)
       {
-        void *mem= thd->stmt_arena->alloc(sizeof(Group_list_ptrs));
-        group_list_ptrs= new (mem) Group_list_ptrs(thd->stmt_arena->mem_root);
+        void *mem= active_arena->alloc(sizeof(Group_list_ptrs));
+        group_list_ptrs= new (mem) Group_list_ptrs(active_arena->mem_root);
       }
       group_list_ptrs->reserve(group_list.elements);
       for (ORDER *order= group_list.first; order; order= order->next)

diff --git a/sql/sql_show.cc b/sql/sql_show.cc
@@ -8842,9 +8842,9 @@ int mysql_schema_table(THD *thd, LEX *lex, TABLE_LIST *table_list)
     }
     List_iterator_fast<Item> it(sel->item_list);
     if (!(transl=
-          (Field_translator*)(thd->stmt_arena->
+          (Field_translator*)(thd->active_stmt_arena_to_use()->
                               alloc(sel->item_list.elements *
-                                    sizeof(Field_translator)))))
+                                    sizeof(Field_translator))))) // ???
     {
       DBUG_RETURN(1);
     }

diff --git a/sql/sql_trigger.cc b/sql/sql_trigger.cc
@@ -2298,7 +2298,8 @@ add_tables_and_routines_for_triggers(THD *thd,
 
           MDL_key key(MDL_key::TRIGGER, trigger->m_db.str, trigger->m_name.str);
 
-          if (sp_add_used_routine(prelocking_ctx, thd->stmt_arena,
+          if (sp_add_used_routine(prelocking_ctx,
+                                  thd->active_stmt_arena_to_use(),
                                   &key, &sp_handler_trigger,
                                   table_list->belong_to_view))
           {

diff --git a/sql/sql_tvc.cc b/sql/sql_tvc.cc
@@ -240,7 +240,10 @@ bool table_value_constr::prepare(THD *thd, SELECT_LEX *sl,
 
   if (!holders)
   {
-    holders= type_holders= new (thd->stmt_arena->mem_root) Type_holder[cnt];
+    DBUG_ASSERT(thd->stmt_arena->is_stmt_prepare_or_first_stmt_execute() ||
+                thd->stmt_arena->is_conventional());
+    holders= type_holders=
+      new (thd->active_stmt_arena_to_use()->mem_root) Type_holder[cnt];
     if (!holders ||
          join_type_handlers_for_tvc(thd, li, holders, cnt) ||
          get_type_attributes_for_tvc(thd, li, holders,

diff --git a/sql/table.cc b/sql/table.cc
@@ -5548,7 +5548,7 @@ bool TABLE_LIST::create_field_translation(THD *thd)
   /* Create view fields translation table */
 
   if (!(transl=
-        (Field_translator*)(thd->stmt_arena->
+        (Field_translator*)(thd->
                             alloc(select->item_list.elements *
                                   sizeof(Field_translator)))))
   {