diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index ac196f58416b2..8638ed665387d 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -5201,13 +5201,43 @@ static void innobase_kill_query(handlerton*, THD* thd, enum thd_kill_levels) } #endif /* WITH_WSREP */ - if (trx_t* trx = thd_to_trx(thd)) { - ut_ad(trx->mysql_thd == thd); - /* Cancel a pending lock request if there are any */ - lock_trx_handle_wait(trx); - } + if (trx_t* trx= thd_to_trx(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_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_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) + lock_cancel_waiting_and_release(lock); + lock_mutex_exit(); + trx_mutex_exit(trx); + } - DBUG_VOID_RETURN; + DBUG_VOID_RETURN; } diff --git a/storage/innobase/include/ut0pool.h b/storage/innobase/include/ut0pool.h index f7bc4c5ebdf11..a7efc4e56a22d 100644 --- a/storage/innobase/include/ut0pool.h +++ b/storage/innobase/include/ut0pool.h @@ -1,6 +1,7 @@ /***************************************************************************** Copyright (c) 2013, 2014, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2018, 2020, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -86,11 +87,15 @@ struct Pool { for (Element* elem = m_start; elem != m_last; ++elem) { ut_ad(elem->m_pool == this); +#ifdef __SANITIZE_ADDRESS__ /* Unpoison the memory for AddressSanitizer */ MEM_UNDEFINED(&elem->m_type, sizeof elem->m_type); +#endif +#ifdef HAVE_valgrind /* Declare the contents as initialized for Valgrind; we checked this in mem_free(). */ UNIV_MEM_VALID(&elem->m_type, sizeof elem->m_type); +#endif Factory::destroy(&elem->m_type); } @@ -127,13 +132,18 @@ struct Pool { #if defined HAVE_valgrind || defined __SANITIZE_ADDRESS__ if (elem) { +# ifdef __SANITIZE_ADDRESS__ /* Unpoison the memory for AddressSanitizer */ MEM_UNDEFINED(&elem->m_type, sizeof elem->m_type); +# endif +# ifdef HAVE_valgrind + /* Declare the memory initialized for Valgrind. The trx_t that are released to the pool are actually initialized; we checked that by UNIV_MEM_ASSERT_RW() in mem_free() below. */ UNIV_MEM_VALID(&elem->m_type, sizeof elem->m_type); +# endif } #endif diff --git a/storage/innobase/trx/trx0trx.cc b/storage/innobase/trx/trx0trx.cc index 85667e91d1360..5768f0fc89068 100644 --- a/storage/innobase/trx/trx0trx.cc +++ b/storage/innobase/trx/trx0trx.cc @@ -408,14 +408,26 @@ trx_free(trx_t*& trx) ut_ad(trx->will_lock == 0); trx_pools->mem_free(trx); +#ifdef __SANITIZE_ADDRESS__ /* Unpoison the memory for innodb_monitor_set_option; it is operating also on the freed transaction objects. */ MEM_UNDEFINED(&trx->mutex, sizeof trx->mutex); MEM_UNDEFINED(&trx->undo_mutex, sizeof trx->undo_mutex); - /* Declare the contents as initialized for Valgrind; - we checked that it was initialized in trx_pools->mem_free(trx). */ + /* For innobase_kill_connection() */ + MEM_UNDEFINED(&trx->state, sizeof trx->state); + MEM_UNDEFINED(&trx->mysql_thd, sizeof trx->mysql_thd); +#endif +#ifdef HAVE_valgrind + /* Unpoison the memory for innodb_monitor_set_option; + it is operating also on the freed transaction objects. + We checked that these were initialized in + trx_pools->mem_free(trx). */ UNIV_MEM_VALID(&trx->mutex, sizeof trx->mutex); UNIV_MEM_VALID(&trx->undo_mutex, sizeof trx->undo_mutex); + /* For innobase_kill_connection() */ + UNIV_MEM_VALID(&trx->state, sizeof trx->state); + UNIV_MEM_VALID(&trx->mysql_thd, sizeof trx->mysql_thd); +#endif trx = NULL; }