From 6d2391f4de0a1e6579abad30a764c0ba9fc4ef36 Mon Sep 17 00:00:00 2001 From: Dave Gosselin Date: Wed, 8 Jan 2025 15:18:03 -0500 Subject: [PATCH] MDEV-35510 ASAN build crashes during bootstrap Decouple a DML result's cleanup from the destructor semantics, allowing cleanup to occur without simultaneously deallocating memory. The MYSQL_DML_DONE function would access the select_result_interceptor implementation (in either the cases of UPDATE or DELETE) after that interceptor instance's destructor had been called. Most of the time this works because the instance was allocated on the statement memroot which continues to live on and no other allocations are made at this point. The interceptor instance's destructor would be called during either Sql_cmd_update::execute_inner or Sql_cmd_delete::execute_inner by an explicit call to delete. Since the result instance lives on the memroot associated with the current command, we can avoid calling the destructor via delete and instead invoke a cleanup method, deferring the memory deallocation to free_root when the command's processing has completed. --- sql/sql_class.h | 2 ++ sql/sql_delete.cc | 8 +++++++- sql/sql_update.cc | 16 ++++++++++++---- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/sql/sql_class.h b/sql/sql_class.h index 16ce6aa0c203f..9b55c7b4864f8 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -7435,6 +7435,7 @@ class multi_delete :public select_result_interceptor public: multi_delete(THD *thd_arg, TABLE_LIST *dt, uint num_of_tables); ~multi_delete(); + void cleanup(); int prepare(List &list, SELECT_LEX_UNIT *u) override; int send_data(List &items) override; bool initialize_tables (JOIN *join) override; @@ -7489,6 +7490,7 @@ class multi_update :public select_result_interceptor List *fields, List *values, enum_duplicates handle_duplicates, bool ignore); ~multi_update(); + void cleanup(); bool init(THD *thd); bool init_for_single_table(THD *thd); int prepare(List &list, SELECT_LEX_UNIT *u) override; diff --git a/sql/sql_delete.cc b/sql/sql_delete.cc index dd842f0f1c05c..abaf91820b5d3 100644 --- a/sql/sql_delete.cc +++ b/sql/sql_delete.cc @@ -1158,6 +1158,12 @@ multi_delete::initialize_tables(JOIN *join) multi_delete::~multi_delete() +{ + cleanup(); +} + + +void multi_delete::cleanup() { for (table_being_deleted= delete_tables; table_being_deleted; @@ -1854,7 +1860,7 @@ bool Sql_cmd_delete::execute_inner(THD *thd) if (result) { res= false; - delete result; + result->cleanup(); } status_var_add(thd->status_var.rows_sent, thd->get_sent_row_count()); diff --git a/sql/sql_update.cc b/sql/sql_update.cc index 1e425911de0ad..c451efc1e1987 100644 --- a/sql/sql_update.cc +++ b/sql/sql_update.cc @@ -2250,6 +2250,12 @@ int multi_update::prepare2(JOIN *join) multi_update::~multi_update() +{ + cleanup(); +} + + +void multi_update::cleanup() { TABLE_LIST *table; for (table= update_tables ; table; table= table->next_local) @@ -2270,10 +2276,12 @@ multi_update::~multi_update() } } } - if (copy_field) - delete [] copy_field; + if (copy_field) { + delete[] copy_field; + copy_field= nullptr; + } thd->count_cuted_fields= CHECK_FIELD_IGNORE; // Restore this setting - DBUG_ASSERT(trans_safe || !updated || + DBUG_ASSERT(trans_safe || !updated || thd->transaction->all.modified_non_trans_table); } @@ -3127,7 +3135,7 @@ bool Sql_cmd_update::execute_inner(THD *thd) if (result) { res= false; - delete result; + result->cleanup(); } status_var_add(thd->status_var.rows_sent, thd->get_sent_row_count());