Moving a change_list related methods from THD to Item_change_list

1. Moving the following methods from THD to Item_change_list:
   nocheck_register_item_tree_change()
   check_and_register_item_tree_change()
   rollback_item_tree_changes()
   as they work only with the "change_list" member and don't
   require anything else from THD.
2. Deriving THD from Item_change_list

This change will help to fix "MDEV-14603 signal 11 with short stacktrace" easier.
This commit is contained in:
Alexander Barkov 2018-01-16 16:09:51 +04:00
parent be85c2dc88
commit 81378b3947
5 changed files with 41 additions and 28 deletions

View File

@ -1249,7 +1249,7 @@ sp_head::execute(THD *thd, bool merge_da_on_success)
We should also save Item tree change list to avoid rollback something We should also save Item tree change list to avoid rollback something
too early in the calling query. too early in the calling query.
*/ */
thd->change_list.move_elements_to(&old_change_list); thd->Item_change_list::move_elements_to(&old_change_list);
/* /*
Cursors will use thd->packet, so they may corrupt data which was prepared Cursors will use thd->packet, so they may corrupt data which was prepared
for sending by upper level. OTOH cursors in the same routine can share this for sending by upper level. OTOH cursors in the same routine can share this
@ -1389,8 +1389,8 @@ sp_head::execute(THD *thd, bool merge_da_on_success)
/* Restore all saved */ /* Restore all saved */
thd->server_status= (thd->server_status & ~status_backup_mask) | old_server_status; thd->server_status= (thd->server_status & ~status_backup_mask) | old_server_status;
old_packet.swap(thd->packet); old_packet.swap(thd->packet);
DBUG_ASSERT(thd->change_list.is_empty()); DBUG_ASSERT(thd->Item_change_list::is_empty());
old_change_list.move_elements_to(&thd->change_list); old_change_list.move_elements_to(thd);
thd->lex= old_lex; thd->lex= old_lex;
thd->set_query_id(old_query_id); thd->set_query_id(old_query_id);
DBUG_ASSERT(!thd->derived_tables); DBUG_ASSERT(!thd->derived_tables);
@ -2976,7 +2976,7 @@ sp_lex_keeper::reset_lex_and_exec_core(THD *thd, uint *nextp,
bool parent_modified_non_trans_table= thd->transaction.stmt.modified_non_trans_table; bool parent_modified_non_trans_table= thd->transaction.stmt.modified_non_trans_table;
thd->transaction.stmt.modified_non_trans_table= FALSE; thd->transaction.stmt.modified_non_trans_table= FALSE;
DBUG_ASSERT(!thd->derived_tables); DBUG_ASSERT(!thd->derived_tables);
DBUG_ASSERT(thd->change_list.is_empty()); DBUG_ASSERT(thd->Item_change_list::is_empty());
/* /*
Use our own lex. Use our own lex.
We should not save old value since it is saved/restored in We should not save old value since it is saved/restored in

View File

@ -2605,8 +2605,10 @@ struct Item_change_record: public ilink
thd->mem_root (due to possible set_n_backup_active_arena called for thd). thd->mem_root (due to possible set_n_backup_active_arena called for thd).
*/ */
void THD::nocheck_register_item_tree_change(Item **place, Item *old_value, void
MEM_ROOT *runtime_memroot) Item_change_list::nocheck_register_item_tree_change(Item **place,
Item *old_value,
MEM_ROOT *runtime_memroot)
{ {
Item_change_record *change; Item_change_record *change;
DBUG_ENTER("THD::nocheck_register_item_tree_change"); DBUG_ENTER("THD::nocheck_register_item_tree_change");
@ -2647,8 +2649,10 @@ void THD::nocheck_register_item_tree_change(Item **place, Item *old_value,
changes to substitute the same reference at both locations L1 and L2. changes to substitute the same reference at both locations L1 and L2.
*/ */
void THD::check_and_register_item_tree_change(Item **place, Item **new_value, void
MEM_ROOT *runtime_memroot) Item_change_list::check_and_register_item_tree_change(Item **place,
Item **new_value,
MEM_ROOT *runtime_memroot)
{ {
Item_change_record *change; Item_change_record *change;
I_List_iterator<Item_change_record> it(change_list); I_List_iterator<Item_change_record> it(change_list);
@ -2663,7 +2667,7 @@ void THD::check_and_register_item_tree_change(Item **place, Item **new_value,
} }
void THD::rollback_item_tree_changes() void Item_change_list::rollback_item_tree_changes()
{ {
I_List_iterator<Item_change_record> it(change_list); I_List_iterator<Item_change_record> it(change_list);
Item_change_record *change; Item_change_record *change;

View File

@ -1279,7 +1279,21 @@ public:
*/ */
struct Item_change_record; struct Item_change_record;
typedef I_List<Item_change_record> Item_change_list; class Item_change_list
{
I_List<Item_change_record> change_list;
public:
void nocheck_register_item_tree_change(Item **place, Item *old_value,
MEM_ROOT *runtime_memroot);
void check_and_register_item_tree_change(Item **place, Item **new_value,
MEM_ROOT *runtime_memroot);
void rollback_item_tree_changes();
void move_elements_to(Item_change_list *to)
{
change_list.move_elements_to(&to->change_list);
}
bool is_empty() { return change_list.is_empty(); }
};
/** /**
@ -2040,6 +2054,14 @@ void dbug_serve_apcs(THD *thd, int n_calls);
*/ */
class THD :public Statement, class THD :public Statement,
/*
This is to track items changed during execution of a prepared
statement/stored procedure. It's created by
nocheck_register_item_tree_change() in memory root of THD,
and freed in rollback_item_tree_changes().
For conventional execution it's always empty.
*/
public Item_change_list,
public MDL_context_owner, public MDL_context_owner,
public Open_tables_state public Open_tables_state
{ {
@ -2510,14 +2532,6 @@ public:
#ifdef SIGNAL_WITH_VIO_CLOSE #ifdef SIGNAL_WITH_VIO_CLOSE
Vio* active_vio; Vio* active_vio;
#endif #endif
/*
This is to track items changed during execution of a prepared
statement/stored procedure. It's created by
nocheck_register_item_tree_change() in memory root of THD, and freed in
rollback_item_tree_changes(). For conventional execution it's always
empty.
*/
Item_change_list change_list;
/* /*
A permanent memory area of the statement. For conventional A permanent memory area of the statement. For conventional
@ -3599,11 +3613,6 @@ public:
*/ */
memcpy((char*) place, new_value, sizeof(*new_value)); memcpy((char*) place, new_value, sizeof(*new_value));
} }
void nocheck_register_item_tree_change(Item **place, Item *old_value,
MEM_ROOT *runtime_memroot);
void check_and_register_item_tree_change(Item **place, Item **new_value,
MEM_ROOT *runtime_memroot);
void rollback_item_tree_changes();
/* /*
Cleanup statement parse state (parse tree, lex) and execution Cleanup statement parse state (parse tree, lex) and execution

View File

@ -7910,7 +7910,7 @@ void mysql_parse(THD *thd, char *rawbuf, uint length,
sp_cache_enforce_limit(thd->sp_func_cache, stored_program_cache_size); sp_cache_enforce_limit(thd->sp_func_cache, stored_program_cache_size);
thd->end_statement(); thd->end_statement();
thd->cleanup_after_query(); thd->cleanup_after_query();
DBUG_ASSERT(thd->change_list.is_empty()); DBUG_ASSERT(thd->Item_change_list::is_empty());
} }
else else
{ {

View File

@ -3930,7 +3930,7 @@ bool Prepared_statement::prepare(const char *packet, uint packet_len)
If called from a stored procedure, ensure that we won't rollback If called from a stored procedure, ensure that we won't rollback
external changes when cleaning up after validation. external changes when cleaning up after validation.
*/ */
DBUG_ASSERT(thd->change_list.is_empty()); DBUG_ASSERT(thd->Item_change_list::is_empty());
/* /*
Marker used to release metadata locks acquired while the prepared Marker used to release metadata locks acquired while the prepared
@ -4407,7 +4407,7 @@ Prepared_statement::execute_server_runnable(Server_runnable *server_runnable)
bool error; bool error;
Query_arena *save_stmt_arena= thd->stmt_arena; Query_arena *save_stmt_arena= thd->stmt_arena;
Item_change_list save_change_list; Item_change_list save_change_list;
thd->change_list.move_elements_to(&save_change_list); thd->Item_change_list::move_elements_to(&save_change_list);
state= STMT_CONVENTIONAL_EXECUTION; state= STMT_CONVENTIONAL_EXECUTION;
@ -4426,7 +4426,7 @@ Prepared_statement::execute_server_runnable(Server_runnable *server_runnable)
thd->restore_backup_statement(this, &stmt_backup); thd->restore_backup_statement(this, &stmt_backup);
thd->stmt_arena= save_stmt_arena; thd->stmt_arena= save_stmt_arena;
save_change_list.move_elements_to(&thd->change_list); save_change_list.move_elements_to(thd);
/* Items and memory will freed in destructor */ /* Items and memory will freed in destructor */
@ -4654,7 +4654,7 @@ bool Prepared_statement::execute(String *expanded_query, bool open_cursor)
If the free_list is not empty, we'll wrongly free some externally If the free_list is not empty, we'll wrongly free some externally
allocated items when cleaning up after execution of this statement. allocated items when cleaning up after execution of this statement.
*/ */
DBUG_ASSERT(thd->change_list.is_empty()); DBUG_ASSERT(thd->Item_change_list::is_empty());
/* /*
The only case where we should have items in the thd->free_list is The only case where we should have items in the thd->free_list is