From 85f2e4f8e8c82978bd9cc0af9bfd2b549ea04d65 Mon Sep 17 00:00:00 2001 From: Dmitry Shulga Date: Fri, 24 Nov 2023 16:28:31 +0700 Subject: [PATCH] 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. --- sql/item_func.cc | 11 ++++++++++- sql/item_jsonfunc.cc | 2 +- sql/item_subselect.cc | 6 +++++- sql/item_sum.cc | 10 ++++++++-- sql/sp.cc | 9 +++++++-- sql/sql_lex.cc | 12 +++++++++--- sql/sql_show.cc | 4 ++-- sql/sql_trigger.cc | 3 ++- sql/sql_tvc.cc | 5 ++++- sql/table.cc | 2 +- 10 files changed, 49 insertions(+), 15 deletions(-) diff --git a/sql/item_func.cc b/sql/item_func.cc index 31c81eb7463..612fe7a4b9e 100644 --- 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 index ffa43f542ed..13eb9955fbd 100644 --- 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 index 6400bf518ce..c632c9fc94b 100644 --- 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 index 9a0f08e2a18..048307fdeb2 100644 --- 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 index 266dbbe9d71..d25c7353ca4 100644 --- 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 index a4859bcc1d2..127691997b0 100644 --- 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 index 3d74cf333ce..d93dea50972 100644 --- 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 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 index 1398e8f9a55..08532030505 100644 --- 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 index a351e3fa308..0f940ca5206 100644 --- 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 index 96a97b0e950..88154986805 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -5548,7 +5548,7 @@ allocate: /* Create view fields translation table */ if (!(transl= - (Field_translator*)(thd->stmt_arena-> + (Field_translator*)(thd-> alloc(select->item_list.elements * sizeof(Field_translator))))) {