diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 2595717572a..2321991d99f 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -1357,12 +1357,15 @@ void THD::change_user(void) /* Do operations that may take a long time */ -void THD::cleanup(void) +void THD::cleanup(bool have_mutex) { DBUG_ENTER("THD::cleanup"); DBUG_ASSERT(cleanup_done == 0); - set_killed(KILL_CONNECTION); + if (have_mutex) + set_killed_no_mutex(KILL_CONNECTION,0,0); + else + set_killed(KILL_CONNECTION); #ifdef ENABLE_WHEN_BINLOG_WILL_BE_ABLE_TO_PREPARE if (transaction.xid_state.xa_state == XA_PREPARED) { @@ -1437,6 +1440,28 @@ void THD::cleanup(void) void THD::free_connection() { DBUG_ASSERT(free_connection_done == 0); + /* Check that we have already called thd->unlink() */ + DBUG_ASSERT(prev == 0 && next == 0); + + /* + Other threads may have a lock on THD::LOCK_thd_data or + THD::LOCK_thd_kill to ensure that this THD is not deleted + while they access it. The following mutex_lock ensures + that no one else is using this THD and it's now safe to + continue. + + For example consider KILL-statement execution on + sql_parse.cc kill_one_thread() that will use + THD::LOCK_thd_data to protect victim thread during + THD::awake(). + */ + mysql_mutex_lock(&LOCK_thd_data); + mysql_mutex_lock(&LOCK_thd_kill); + +#ifdef WITH_WSREP + delete wsrep_rgi; + wsrep_rgi= 0; +#endif /* WITH_WSREP */ my_free(db); db= NULL; #ifndef EMBEDDED_LIBRARY @@ -1445,8 +1470,8 @@ void THD::free_connection() net.vio= 0; net_end(&net); #endif - if (!cleanup_done) - cleanup(); + if (!cleanup_done) + cleanup(true); // We have locked THD::LOCK_thd_kill ha_close_connection(this); plugin_thdvar_cleanup(this); mysql_audit_free_thd(this); @@ -1457,6 +1482,8 @@ void THD::free_connection() #if defined(ENABLED_PROFILING) profiling.restart(); // Reset profiling #endif + mysql_mutex_unlock(&LOCK_thd_kill); + mysql_mutex_unlock(&LOCK_thd_data); } /* @@ -1512,9 +1539,6 @@ THD::~THD() mysql_mutex_lock(&LOCK_thd_data); mysql_mutex_unlock(&LOCK_thd_data); -#ifdef WITH_WSREP - delete wsrep_rgi; -#endif if (!free_connection_done) free_connection(); diff --git a/sql/sql_class.h b/sql/sql_class.h index b68e3553a2d..a97f64c13b3 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -3194,7 +3194,7 @@ public: void update_all_stats(); void update_stats(void); void change_user(void); - void cleanup(void); + void cleanup(bool have_mutex=false); void cleanup_after_query(); void free_connection(); void reset_for_reuse(); diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 7cc4401d0ff..c79b3423b63 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -5176,38 +5176,15 @@ static void innobase_kill_query(handlerton*, THD* thd, enum thd_kill_levels) if (trx_t* trx= thd_to_trx(thd)) { + ut_ad(trx->mysql_thd == thd); lock_mutex_enter(); - trx_sys_mutex_enter(); - trx_mutex_enter(trx); - /* It is possible that innobase_close_connection() is concurrently - being executed on our victim. In that case, trx->mysql_thd would - be reset before invoking trx_t::free(). Even if the trx object is later - reused for another client connection or a background transaction, - its trx->mysql_thd will differ from our thd. - - If trx never performed any changes, nothing is really protecting - the trx_t::free() call or the changes of trx_t::state when the - transaction is being rolled back and trx_commit_low() is being - executed. - - The function trx_allocate_for_mysql() acquires - trx_sys_t::mutex, but trx_allocate_for_background() will not. - Luckily, background transactions cannot be read-only, because - for read-only transactions, trx_start_low() will avoid acquiring - any of the trx_sys_t::mutex, lock_sys_t::mutex, trx_t::mutex before - assigning trx_t::state. - - At this point, trx may have been reallocated for another client - connection, or for a background operation. In that case, either - trx_t::state or trx_t::mysql_thd should not match our expectations. */ - bool cancel= trx->mysql_thd == thd && trx->state == TRX_STATE_ACTIVE && - !trx->lock.was_chosen_as_deadlock_victim; - trx_sys_mutex_exit(); - if (!cancel); - else if (lock_t *lock= trx->lock.wait_lock) + if (lock_t *lock= trx->lock.wait_lock) + { + trx_mutex_enter(trx); lock_cancel_waiting_and_release(lock); + trx_mutex_exit(trx); + } lock_mutex_exit(); - trx_mutex_exit(trx); } DBUG_VOID_RETURN; diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc index ec37f9029f7..c95506abc39 100644 --- a/storage/innobase/lock/lock0lock.cc +++ b/storage/innobase/lock/lock0lock.cc @@ -6453,6 +6453,7 @@ lock_cancel_waiting_and_release( ut_ad(lock_mutex_own()); ut_ad(trx_mutex_own(lock->trx)); + ut_ad(lock->trx->state == TRX_STATE_ACTIVE); lock->trx->lock.cancel = true; diff --git a/storage/innobase/trx/trx0trx.cc b/storage/innobase/trx/trx0trx.cc index 245758cecd5..60e534e0f43 100644 --- a/storage/innobase/trx/trx0trx.cc +++ b/storage/innobase/trx/trx0trx.cc @@ -393,7 +393,7 @@ inline void trx_t::free() /* do not poison mutex */ MEM_NOACCESS(&id, sizeof id); MEM_NOACCESS(&no, sizeof no); - /* state is accessed by innobase_kill_connection() */ + MEM_NOACCESS(&state, sizeof state); MEM_NOACCESS(&is_recovered, sizeof is_recovered); #ifdef WITH_WSREP MEM_NOACCESS(&wsrep, sizeof wsrep); @@ -419,7 +419,7 @@ inline void trx_t::free() MEM_NOACCESS(&start_time_micro, sizeof start_time_micro); MEM_NOACCESS(&commit_lsn, sizeof commit_lsn); MEM_NOACCESS(&table_id, sizeof table_id); - /* mysql_thd is accessed by innobase_kill_connection() */ + MEM_NOACCESS(&mysql_thd, sizeof mysql_thd); MEM_NOACCESS(&mysql_log_file_name, sizeof mysql_log_file_name); MEM_NOACCESS(&mysql_log_offset, sizeof mysql_log_offset); MEM_NOACCESS(&n_mysql_tables_in_use, sizeof n_mysql_tables_in_use);