From 3bfe305c5cd678a8563f7a76d6ed59095129007e Mon Sep 17 00:00:00 2001 From: Vladislav Vaintroub Date: Thu, 14 May 2020 15:46:17 +0200 Subject: [PATCH 1/4] MDEV-22555 Windows, packaging: binaries depend on vcruntime140_1.dll, which is not in MSI When server is compiled with recent VS2019, then executables, have dependency on vcruntime140_1.dll While we include the VC redistributable merge modules into our MSI package, those merge modules were stale (taken from older VS version, 2017) Since VS2019 brough new DLL dependency by introducing new exception handling https://devblogs.microsoft.com/cppblog/making-cpp-exception-handling-smaller-x64 thus the old MSMs were not enough. The fix is to change logic in win/packaging/CMakeLists.txt to look up for the correct, new MSMs. The bug only affects 10.4,as we compile with static CRT before 10.4, and partly-statically(just vcruntime stub is statically linked, but not UCRT) after 10.4 For the fix to work, it required also some changes on the build machine (vs_installer, modify VS2019 installation, add Individual Component "C++ 2019 Redistributable MSMs") --- win/packaging/CMakeLists.txt | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/win/packaging/CMakeLists.txt b/win/packaging/CMakeLists.txt index 280be75c6d7..a7168cbe85d 100644 --- a/win/packaging/CMakeLists.txt +++ b/win/packaging/CMakeLists.txt @@ -189,7 +189,7 @@ IF(MSVC_CRT_TYPE MATCHES "/MD") "$ENV{${ProgramFilesX86}}/Common Files/Merge Modules" "$ENV{ProgramFiles}/Common Files/Merge Modules" ) - ELSEIF(MSVC_VERSION LESS 2000) + ELSEIF(MSVC_VERSION LESS 1920) # VS2017 SET(VCREDIST_MSM_FILENAME Microsoft_VC141_CRT_${WIX_ARCH_SUFFIX}.msm) FILE(GLOB MSM_LIST "C:/Program Files*/Microsoft Visual Studio/2017/*/VC/Redist/MSVC/*/MergeModules/${VCREDIST_MSM_FILENAME}") @@ -198,9 +198,13 @@ IF(MSVC_CRT_TYPE MATCHES "/MD") LIST(GET MSM_LIST 0 VCRedist_MSM) ENDIF() ELSE() - # Post-VS2017. Needs to be ported when new VS is out - MESSAGE(WARNING - "Name of redistributable merge module not known for this version of MSVC") + # VS2019 + SET(VCREDIST_MSM_FILENAME Microsoft_VC142_CRT_${WIX_ARCH_SUFFIX}.msm) + FILE(GLOB MSM_LIST "C:/Program Files*/Microsoft Visual Studio/2019/*/VC/Redist/MSVC/*/MergeModules/${VCREDIST_MSM_FILENAME}") + LIST(LENGTH MSM_LIST LEN) + IF(LEN GREATER 0) + LIST(GET MSM_LIST 0 VCRedist_MSM) + ENDIF() ENDIF() IF (NOT VCRedist_MSM) MESSAGE(WARNING "Can't find merge module ${VCREDIST_MSM_FILENAME}") From 523d67a272ca37bf6c1972bae09f59d72a78d99c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Lindstr=C3=B6m?= Date: Thu, 14 May 2020 09:17:14 +0300 Subject: [PATCH 2/4] MDEV-22494 : Galera assertion lock_sys.mutex.is_owned() at lock_trx_handle_wait_low Problem was that trx->lock.was_chosen_as_wsrep_victim variable was not set back to false after it was set true. wsrep_thd_bf_abort Add assertions for correct mutex status and take necessary mutexes before calling thd->awake_no_mutex(). innobase_rollback_trx() Reset trx->lock.was_chosen_as_wsrep_victim wsrep_abort_slave_trx() Removed unused function. wsrep_innobase_kill_one_trx() Added function comment, removed unnecessary parameters and added debug assertions to enforce correct usage. Added more debug output to help out on error analysis. wsrep_abort_transaction() Added debug assertions and removed unused variables. trx0trx.h Removed assert_trx_is_free macro and replaced it with assert_freed() member function. trx_create() Use above assert_free() and initialize wsrep variables. trx_free() Use assert_free() trx_t::commit_in_memory() Reset lock.was_chosen_as_wsrep_victim trx_rollback_for_mysql() Reset trx->lock.was_chosen_as_wsrep_victim Add test case galera_bf_kill --- .../suite/galera/r/galera_bf_kill.result | 72 ++++++++ .../suite/galera/r/galera_bf_lock_wait.result | 4 +- mysql-test/suite/galera/t/galera_bf_kill.cnf | 7 + mysql-test/suite/galera/t/galera_bf_kill.test | 143 ++++++++++++++++ .../suite/galera/t/galera_bf_lock_wait.cnf | 7 + .../suite/galera/t/galera_bf_lock_wait.test | 8 +- sql/service_wsrep.cc | 10 +- storage/innobase/handler/ha_innodb.cc | 155 ++++++++++-------- storage/innobase/include/ha_prototypes.h | 9 +- storage/innobase/include/trx0trx.h | 46 +++--- storage/innobase/lock/lock0lock.cc | 2 +- storage/innobase/trx/trx0roll.cc | 3 +- storage/innobase/trx/trx0trx.cc | 13 +- 13 files changed, 370 insertions(+), 109 deletions(-) create mode 100644 mysql-test/suite/galera/r/galera_bf_kill.result create mode 100644 mysql-test/suite/galera/t/galera_bf_kill.cnf create mode 100644 mysql-test/suite/galera/t/galera_bf_kill.test create mode 100644 mysql-test/suite/galera/t/galera_bf_lock_wait.cnf diff --git a/mysql-test/suite/galera/r/galera_bf_kill.result b/mysql-test/suite/galera/r/galera_bf_kill.result new file mode 100644 index 00000000000..8b620323e35 --- /dev/null +++ b/mysql-test/suite/galera/r/galera_bf_kill.result @@ -0,0 +1,72 @@ +connection node_2; +connection node_1; +connection node_2; +CREATE TABLE t1(a int not null primary key auto_increment,b int) engine=InnoDB; +insert into t1 values (NULL,1); +connect node_2a, 127.0.0.1, root, , test, $NODE_MYPORT_2; +connection node_2a; +begin; +update t1 set a = 5; +connection node_2; +select * from t1; +a b +2 1 +disconnect node_2a; +connect node_2a, 127.0.0.1, root, , test, $NODE_MYPORT_2; +connection node_2a; +begin; +update t1 set a =5; +connection node_2; +select * from t1; +a b +2 1 +disconnect node_2a; +connect node_2a, 127.0.0.1, root, , test, $NODE_MYPORT_2; +connection node_2a; +begin; +update t1 set a =5, b=2; +connection node_2; +ALTER TABLE t1 ADD UNIQUE KEY b1(b); +ALTER TABLE t1 DROP KEY b1; +select * from t1; +a b +2 1 +disconnect node_2a; +connect node_2a, 127.0.0.1, root, , test, $NODE_MYPORT_2; +connection node_2a; +begin; +update t1 set a =5, b=2; +connect node_2b, 127.0.0.1, root, , test, $NODE_MYPORT_2; +connection node_2b; +begin; +update t1 set a =6, b=7; +connection node_2; +ALTER TABLE t1 ADD UNIQUE KEY b2(b); +ALTER TABLE t1 DROP KEY b2; +select * from t1; +a b +2 1 +disconnect node_2a; +disconnect node_2b; +connect node_2a, 127.0.0.1, root, , test, $NODE_MYPORT_2; +connection node_2a; +SET SESSION wsrep_on=OFF; +begin; +update t1 set a =5, b=2; +connection node_2; +ALTER TABLE t1 ADD UNIQUE KEY b3(b); +select * from t1; +a b +2 1 +disconnect node_2a; +connect node_2a, 127.0.0.1, root, , test, $NODE_MYPORT_2; +connection node_2a; +SET SESSION wsrep_on=OFF; +begin; +update t1 set a =5, b=2; +connection node_2; +select * from t1; +a b +2 1 +disconnect node_2a; +drop table t1; diff --git a/mysql-test/suite/galera/r/galera_bf_lock_wait.result b/mysql-test/suite/galera/r/galera_bf_lock_wait.result index f893848a72d..71627d11a4e 100644 --- a/mysql-test/suite/galera/r/galera_bf_lock_wait.result +++ b/mysql-test/suite/galera/r/galera_bf_lock_wait.result @@ -1,5 +1,7 @@ connection node_2; connection node_1; +connection node_1; +call mtr.add_suppression("WSREP: Trying to continue unpaused monitor"); CREATE TABLE t1 ENGINE=InnoDB select 1 as a, 1 as b union select 2, 2; ALTER TABLE t1 add primary key(a); CREATE PROCEDURE p1() @@ -19,7 +21,7 @@ connect node_2_p1, 127.0.0.1, root, , test, $NODE_MYPORT_2; call p1; connect node_2_p2, 127.0.0.1, root, , test, $NODE_MYPORT_2; call p1; -connection default; +connection node_1; checking error log for 'BF lock wait long' message for 10 times every 10 seconds ... drop table t1; drop procedure p1; diff --git a/mysql-test/suite/galera/t/galera_bf_kill.cnf b/mysql-test/suite/galera/t/galera_bf_kill.cnf new file mode 100644 index 00000000000..e68f891792c --- /dev/null +++ b/mysql-test/suite/galera/t/galera_bf_kill.cnf @@ -0,0 +1,7 @@ +!include ../galera_2nodes.cnf + +[mysqld.1] +wsrep-debug=SERVER + +[mysqld.2] +wsrep-debug=SERVER diff --git a/mysql-test/suite/galera/t/galera_bf_kill.test b/mysql-test/suite/galera/t/galera_bf_kill.test new file mode 100644 index 00000000000..0748b732ead --- /dev/null +++ b/mysql-test/suite/galera/t/galera_bf_kill.test @@ -0,0 +1,143 @@ +--source include/galera_cluster.inc + +# +# Test case 1: Start a transaction on node_2a and kill it +# from other connection on same node +# + +--connection node_2 +CREATE TABLE t1(a int not null primary key auto_increment,b int) engine=InnoDB; +insert into t1 values (NULL,1); + +--connect node_2a, 127.0.0.1, root, , test, $NODE_MYPORT_2 +--connection node_2a +begin; +update t1 set a = 5; + +--connection node_2 + +--let $wait_condition = SELECT COUNT(*) = 1 FROM INFORMATION_SCHEMA.PROCESSLIST WHERE USER = 'root' AND COMMAND = 'Sleep' LIMIT 1 +--source include/wait_condition.inc + +--let $k_thread = `SELECT ID FROM INFORMATION_SCHEMA.PROCESSLIST WHERE USER = 'root' AND COMMAND = 'Sleep' LIMIT 1` + +--disable_query_log +--eval KILL $k_thread +--enable_query_log + +select * from t1; +--disconnect node_2a + +# +# Test case 2: Start a transaction on node_2a and use +# kill query from other connection on same node +# + +--connect node_2a, 127.0.0.1, root, , test, $NODE_MYPORT_2 +--connection node_2a +begin; +update t1 set a =5; + +--connection node_2 +--let $wait_condition = SELECT COUNT(*) = 1 FROM INFORMATION_SCHEMA.PROCESSLIST WHERE USER = 'root' AND COMMAND = 'Sleep' LIMIT 1 +--source include/wait_condition.inc + +--let $k_thread = `SELECT ID FROM INFORMATION_SCHEMA.PROCESSLIST WHERE USER = 'root' AND COMMAND = 'Sleep' LIMIT 1` + +--disable_query_log +--eval KILL QUERY $k_thread +--enable_query_log + +select * from t1; +--disconnect node_2a +# +# Test case 3: Start a transaction on node_2a and start a DDL on other transaction +# that will then abort node_2a transaction +# +--connect node_2a, 127.0.0.1, root, , test, $NODE_MYPORT_2 +--connection node_2a +begin; +update t1 set a =5, b=2; + +--connection node_2 +ALTER TABLE t1 ADD UNIQUE KEY b1(b); +ALTER TABLE t1 DROP KEY b1; + +select * from t1; + +--disconnect node_2a + +# +# Test case 4: Start a transaction on node_2a and conflicting transaction on node_2b +# and start a DDL on other transaction that will then abort node_2a and node_2b +# transactions +# + +--connect node_2a, 127.0.0.1, root, , test, $NODE_MYPORT_2 +--connection node_2a +begin; +update t1 set a =5, b=2; + +--connect node_2b, 127.0.0.1, root, , test, $NODE_MYPORT_2 +--connection node_2b +begin; +send update t1 set a =6, b=7; + +--connection node_2 +ALTER TABLE t1 ADD UNIQUE KEY b2(b); +ALTER TABLE t1 DROP KEY b2; + +select * from t1; + +--disconnect node_2a +--disconnect node_2b + +# +# Test case 5: Start a transaction on node_2a with wsrep disabled +# and start a DDL on other transaction that will then abort node_2a +# transactions +# + +--connect node_2a, 127.0.0.1, root, , test, $NODE_MYPORT_2 +--connection node_2a +SET SESSION wsrep_on=OFF; +begin; +update t1 set a =5, b=2; + +--connection node_2 +ALTER TABLE t1 ADD UNIQUE KEY b3(b); + +select * from t1; + +--disconnect node_2a + +# +# Test case 6: Start a transaction on node_2a with wsrep disabled +# and kill it from other connection on same node +# + +--connect node_2a, 127.0.0.1, root, , test, $NODE_MYPORT_2 +--connection node_2a +SET SESSION wsrep_on=OFF; +begin; +update t1 set a =5, b=2; + +--connection node_2 +--let $wait_condition = SELECT COUNT(*) = 1 FROM INFORMATION_SCHEMA.PROCESSLIST WHERE USER = 'root' AND COMMAND = 'Sleep' LIMIT 1 +--source include/wait_condition.inc + +--let $k_thread = `SELECT ID FROM INFORMATION_SCHEMA.PROCESSLIST WHERE USER = 'root' AND COMMAND = 'Sleep' LIMIT 1` + +--disable_query_log +--eval KILL $k_thread +--enable_query_log + + +select * from t1; + +--disconnect node_2a + +drop table t1; + + + diff --git a/mysql-test/suite/galera/t/galera_bf_lock_wait.cnf b/mysql-test/suite/galera/t/galera_bf_lock_wait.cnf new file mode 100644 index 00000000000..e68f891792c --- /dev/null +++ b/mysql-test/suite/galera/t/galera_bf_lock_wait.cnf @@ -0,0 +1,7 @@ +!include ../galera_2nodes.cnf + +[mysqld.1] +wsrep-debug=SERVER + +[mysqld.2] +wsrep-debug=SERVER diff --git a/mysql-test/suite/galera/t/galera_bf_lock_wait.test b/mysql-test/suite/galera/t/galera_bf_lock_wait.test index e3a9077a888..a3903fd10c0 100644 --- a/mysql-test/suite/galera/t/galera_bf_lock_wait.test +++ b/mysql-test/suite/galera/t/galera_bf_lock_wait.test @@ -1,6 +1,10 @@ --source include/galera_cluster.inc --source include/big_test.inc - + +--connection node_1 + +call mtr.add_suppression("WSREP: Trying to continue unpaused monitor"); + CREATE TABLE t1 ENGINE=InnoDB select 1 as a, 1 as b union select 2, 2; ALTER TABLE t1 add primary key(a); @@ -28,7 +32,7 @@ send call p1; --connect node_2_p2, 127.0.0.1, root, , test, $NODE_MYPORT_2 send call p1; -connection default; +connection node_1; let $counter=10; let $sleep_period=10; diff --git a/sql/service_wsrep.cc b/sql/service_wsrep.cc index 6634bfa7e7a..7cac2bf741b 100644 --- a/sql/service_wsrep.cc +++ b/sql/service_wsrep.cc @@ -207,7 +207,15 @@ extern "C" my_bool wsrep_thd_bf_abort(THD *bf_thd, THD *victim_thd, as RSU has paused the provider. */ if ((ret || !wsrep_on(victim_thd)) && signal) - victim_thd->awake(KILL_QUERY); + { + mysql_mutex_assert_not_owner(&victim_thd->LOCK_thd_data); + mysql_mutex_assert_not_owner(&victim_thd->LOCK_thd_kill); + mysql_mutex_lock(&victim_thd->LOCK_thd_data); + mysql_mutex_lock(&victim_thd->LOCK_thd_kill); + victim_thd->awake_no_mutex(KILL_QUERY); + mysql_mutex_unlock(&victim_thd->LOCK_thd_kill); + mysql_mutex_unlock(&victim_thd->LOCK_thd_data); + } return ret; } diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index a3014fcd036..4fa405c5dc2 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -4720,7 +4720,8 @@ innobase_rollback_trx( if (!trx->has_logged()) { trx->will_lock = 0; #ifdef WITH_WSREP - trx->wsrep = false; + trx->wsrep= false; + trx->lock.was_chosen_as_wsrep_victim= false; #endif DBUG_RETURN(0); } @@ -18635,106 +18636,128 @@ static struct st_mysql_storage_engine innobase_storage_engine= { MYSQL_HANDLERTON_INTERFACE_VERSION }; #ifdef WITH_WSREP -void -wsrep_abort_slave_trx( -/*==================*/ - wsrep_seqno_t bf_seqno, - wsrep_seqno_t victim_seqno) -{ - WSREP_ERROR("Trx %lld tries to abort slave trx %lld. This could be " - "caused by:\n\t" - "1) unsupported configuration options combination, please check documentation.\n\t" - "2) a bug in the code.\n\t" - "3) a database corruption.\n Node consistency compromized, " - "need to abort. Restart the node to resync with cluster.", - (long long)bf_seqno, (long long)victim_seqno); - abort(); -} -/*******************************************************************//** -This function is used to kill one transaction in BF. */ + +/** This function is used to kill one transaction. + +This transaction was open on this node (not-yet-committed), and a +conflicting writeset from some other node that was being applied +caused a locking conflict. First committed (from other node) +wins, thus open transaction is rolled back. BF stands for +brute-force: any transaction can get aborted by galera any time +it is necessary. + +This conflict can happen only when the replicated writeset (from +other node) is being applied, not when it’s waiting in the queue. +If our local transaction reached its COMMIT and this conflicting +writeset was in the queue, then it should fail the local +certification test instead. + +A brute force abort is only triggered by a locking conflict +between a writeset being applied by an applier thread (slave thread) +and an open transaction on the node, not by a Galera writeset +comparison as in the local certification failure. + +@param[in] bf_thd Brute force (BF) thread +@param[in,out] victim_trx Vimtim trx to be killed +@param[in] signal Should victim be signaled */ UNIV_INTERN int wsrep_innobase_kill_one_trx( -/*========================*/ - void * const bf_thd_ptr, - const trx_t * const bf_trx, + THD* bf_thd, trx_t *victim_trx, - ibool signal) + bool signal) { - ut_ad(lock_mutex_own()); - ut_ad(trx_mutex_own(victim_trx)); - ut_ad(bf_thd_ptr); - ut_ad(victim_trx); + ut_ad(bf_thd); + ut_ad(victim_trx); + ut_ad(lock_mutex_own()); + ut_ad(trx_mutex_own(victim_trx)); DBUG_ENTER("wsrep_innobase_kill_one_trx"); - THD *bf_thd = bf_thd_ptr ? (THD*) bf_thd_ptr : NULL; - THD *thd = (THD *) victim_trx->mysql_thd; - int64_t bf_seqno = (bf_thd) ? wsrep_thd_trx_seqno(bf_thd) : 0; - if (!thd) { - DBUG_PRINT("wsrep", ("no thd for conflicting lock")); - WSREP_WARN("no THD for trx: " TRX_ID_FMT, victim_trx->id); - DBUG_RETURN(1); - } + THD *thd= (THD *) victim_trx->mysql_thd; + ut_ad(thd); + /* Note that bf_trx might not exist here e.g. on MDL conflict + case (test: galera_concurrent_ctas). Similarly, BF thread + could be also acquiring MDL-lock causing victim to be + aborted. However, we have not yet called innobase_trx_init() + for BF transaction (test: galera_many_columns)*/ + trx_t* bf_trx= thd_to_trx(bf_thd); + DBUG_ASSERT(wsrep_on(bf_thd)); - if (!bf_thd) { - DBUG_PRINT("wsrep", ("no BF thd for conflicting lock")); - WSREP_WARN("no BF THD for trx: " TRX_ID_FMT, - bf_trx ? bf_trx->id : 0); - DBUG_RETURN(1); - } - WSREP_LOG_CONFLICT(bf_thd, thd, TRUE); wsrep_thd_LOCK(thd); - WSREP_DEBUG("BF kill (" ULINTPF ", seqno: " INT64PF - "), victim: (%lu) trx: " TRX_ID_FMT, - signal, bf_seqno, - thd_get_thread_id(thd), - victim_trx->id); - WSREP_DEBUG("Aborting query: %s conf %s trx: %lld", - (thd && wsrep_thd_query(thd)) ? wsrep_thd_query(thd) : "void", - wsrep_thd_transaction_state_str(thd), - wsrep_thd_transaction_id(thd)); + WSREP_LOG_CONFLICT(bf_thd, thd, TRUE); - /* - * we mark with was_chosen_as_deadlock_victim transaction, - * which is already marked as BF victim - * lock_sys is held until this vicitm has aborted - */ - victim_trx->lock.was_chosen_as_wsrep_victim = TRUE; + WSREP_DEBUG("Aborter %s trx_id: " TRX_ID_FMT " thread: %ld " + "seqno: %lld client_state: %s client_mode: %s transaction_mode: %s " + "query: %s", + wsrep_thd_is_BF(bf_thd, false) ? "BF" : "normal", + bf_trx ? bf_trx->id : TRX_ID_MAX, + thd_get_thread_id(bf_thd), + wsrep_thd_trx_seqno(bf_thd), + wsrep_thd_client_state_str(bf_thd), + wsrep_thd_client_mode_str(bf_thd), + wsrep_thd_transaction_state_str(bf_thd), + wsrep_thd_query(bf_thd)); + WSREP_DEBUG("Victim %s trx_id: " TRX_ID_FMT " thread: %ld " + "seqno: %lld client_state: %s client_mode: %s transaction_mode: %s " + "query: %s", + wsrep_thd_is_BF(thd, false) ? "BF" : "normal", + victim_trx->id, + thd_get_thread_id(thd), + wsrep_thd_trx_seqno(thd), + wsrep_thd_client_state_str(thd), + wsrep_thd_client_mode_str(thd), + wsrep_thd_transaction_state_str(thd), + wsrep_thd_query(thd)); + + /* Mark transaction as a victim for Galera abort */ + victim_trx->lock.was_chosen_as_wsrep_victim= true; + + /* Note that we need to release this as it will be acquired + below in wsrep-lib */ wsrep_thd_UNLOCK(thd); + if (wsrep_thd_bf_abort(bf_thd, thd, signal)) { - if (victim_trx->lock.wait_lock) { + lock_t* wait_lock = victim_trx->lock.wait_lock; + if (wait_lock) { + DBUG_ASSERT(victim_trx->is_wsrep()); WSREP_DEBUG("victim has wait flag: %lu", thd_get_thread_id(thd)); - lock_t* wait_lock = victim_trx->lock.wait_lock; - if (wait_lock) { - WSREP_DEBUG("canceling wait lock"); - victim_trx->lock.was_chosen_as_deadlock_victim= TRUE; - lock_cancel_waiting_and_release(wait_lock); - } + WSREP_DEBUG("canceling wait lock"); + victim_trx->lock.was_chosen_as_deadlock_victim= TRUE; + lock_cancel_waiting_and_release(wait_lock); } } DBUG_RETURN(0); } +/** This function forces the victim transaction to abort. Aborting the + transaction does NOT end it, it still has to be rolled back. + + @param bf_thd brute force THD asking for the abort + @param victim_thd victim THD to be aborted + + @return 0 victim was aborted + @return -1 victim thread was aborted (no transaction) +*/ static int wsrep_abort_transaction( -/*====================*/ handlerton*, THD *bf_thd, THD *victim_thd, my_bool signal) { DBUG_ENTER("wsrep_innobase_abort_thd"); + ut_ad(bf_thd); + ut_ad(victim_thd); trx_t* victim_trx = thd_to_trx(victim_thd); - trx_t* bf_trx = (bf_thd) ? thd_to_trx(bf_thd) : NULL; WSREP_DEBUG("abort transaction: BF: %s victim: %s victim conf: %s", wsrep_thd_query(bf_thd), @@ -18744,7 +18767,7 @@ wsrep_abort_transaction( if (victim_trx) { lock_mutex_enter(); trx_mutex_enter(victim_trx); - int rcode= wsrep_innobase_kill_one_trx(bf_thd, bf_trx, + int rcode= wsrep_innobase_kill_one_trx(bf_thd, victim_trx, signal); trx_mutex_exit(victim_trx); lock_mutex_exit(); diff --git a/storage/innobase/include/ha_prototypes.h b/storage/innobase/include/ha_prototypes.h index b0dab87c1db..28e5d1d4f56 100644 --- a/storage/innobase/include/ha_prototypes.h +++ b/storage/innobase/include/ha_prototypes.h @@ -232,10 +232,11 @@ innobase_casedn_str( #ifdef WITH_WSREP UNIV_INTERN int -wsrep_innobase_kill_one_trx(void * const thd_ptr, - const trx_t * const bf_trx, - trx_t *victim_trx, - ibool signal); +wsrep_innobase_kill_one_trx( + THD* bf_thd, + trx_t *victim_trx, + bool signal); + ulint wsrep_innobase_mysql_sort(int mysql_type, uint charset_number, unsigned char* str, unsigned int str_length, unsigned int buf_length); diff --git a/storage/innobase/include/trx0trx.h b/storage/innobase/include/trx0trx.h index 87bf97f00cf..70df62d0d03 100644 --- a/storage/innobase/include/trx0trx.h +++ b/storage/innobase/include/trx0trx.h @@ -436,32 +436,6 @@ Check transaction state */ ut_error; \ } while (0) -/** Check if transaction is free so that it can be re-initialized. -@param t transaction handle */ -#define assert_trx_is_free(t) do { \ - ut_ad(trx_state_eq((t), TRX_STATE_NOT_STARTED)); \ - ut_ad(!(t)->id); \ - ut_ad(!(t)->has_logged()); \ - ut_ad(!(t)->is_referenced()); \ - ut_ad(!(t)->is_wsrep()); \ - ut_ad(!(t)->read_view.is_open()); \ - ut_ad((t)->lock.wait_thr == NULL); \ - ut_ad(UT_LIST_GET_LEN((t)->lock.trx_locks) == 0); \ - ut_ad((t)->lock.table_locks.empty()); \ - ut_ad(!(t)->autoinc_locks \ - || ib_vector_is_empty((t)->autoinc_locks)); \ - ut_ad(UT_LIST_GET_LEN((t)->lock.evicted_tables) == 0); \ - ut_ad((t)->dict_operation == TRX_DICT_OP_NONE); \ -} while(0) - -/** Check if transaction is in-active so that it can be freed and put back to -transaction pool. -@param t transaction handle */ -#define assert_trx_is_inactive(t) do { \ - assert_trx_is_free((t)); \ - ut_ad((t)->dict_operation_lock_mode == 0); \ -} while(0) - #ifdef UNIV_DEBUG /*******************************************************************//** Assert that an autocommit non-locking select cannot be in the @@ -1158,6 +1132,26 @@ public: } + void assert_freed() const + { + ut_ad(state == TRX_STATE_NOT_STARTED); + ut_ad(!id); + ut_ad(!has_logged()); + ut_ad(!is_referenced()); + ut_ad(!is_wsrep()); +#ifdef WITH_WSREP + ut_ad(!lock.was_chosen_as_wsrep_victim); +#endif + ut_ad(!read_view.is_open()); + ut_ad(!lock.wait_thr); + ut_ad(UT_LIST_GET_LEN(lock.trx_locks) == 0); + ut_ad(lock.table_locks.empty()); + ut_ad(!autoinc_locks || ib_vector_is_empty(autoinc_locks)); + ut_ad(UT_LIST_GET_LEN(lock.evicted_tables) == 0); + ut_ad(dict_operation == TRX_DICT_OP_NONE); + } + + private: /** Assign a rollback segment for modifying temporary tables. @return the assigned rollback segment */ diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc index 349dd01f904..8fe5a8f720f 100644 --- a/storage/innobase/lock/lock0lock.cc +++ b/storage/innobase/lock/lock0lock.cc @@ -1134,7 +1134,7 @@ wsrep_kill_victim( } wsrep_innobase_kill_one_trx(trx->mysql_thd, - trx, lock->trx, TRUE); + lock->trx, true); } } } diff --git a/storage/innobase/trx/trx0roll.cc b/storage/innobase/trx/trx0roll.cc index 637f8b709f5..4c2ee22a375 100644 --- a/storage/innobase/trx/trx0roll.cc +++ b/storage/innobase/trx/trx0roll.cc @@ -229,7 +229,8 @@ dberr_t trx_rollback_for_mysql(trx_t* trx) trx->will_lock = 0; ut_ad(trx->mysql_thd); #ifdef WITH_WSREP - trx->wsrep = false; + trx->wsrep= false; + trx->lock.was_chosen_as_wsrep_victim= false; #endif return(DB_SUCCESS); diff --git a/storage/innobase/trx/trx0trx.cc b/storage/innobase/trx/trx0trx.cc index b1f88b1226f..276a78d00bf 100644 --- a/storage/innobase/trx/trx0trx.cc +++ b/storage/innobase/trx/trx0trx.cc @@ -353,14 +353,13 @@ trx_t *trx_create() { trx_t* trx = trx_pools->get(); - assert_trx_is_free(trx); + trx->assert_freed(); mem_heap_t* heap; ib_alloc_t* alloc; /* We just got trx from pool, it should be non locking */ ut_ad(trx->will_lock == 0); - ut_ad(trx->state == TRX_STATE_NOT_STARTED); ut_ad(!trx->rw_trx_hash_pins); DBUG_LOG("trx", "Create: " << trx); @@ -383,7 +382,7 @@ trx_t *trx_create() ut_ad(UT_LIST_GET_LEN(trx->lock.evicted_tables) == 0); #ifdef WITH_WSREP - trx->wsrep_event = NULL; + trx->wsrep_event= NULL; #endif /* WITH_WSREP */ trx_sys.register_trx(trx); @@ -431,11 +430,11 @@ void trx_free(trx_t*& trx) } trx->dict_operation = TRX_DICT_OP_NONE; - assert_trx_is_inactive(trx); + ut_ad(!trx->dict_operation_lock_mode); trx_sys.deregister_trx(trx); - assert_trx_is_free(trx); + trx->assert_freed(); trx_sys.rw_trx_hash.put_pins(trx); trx->mysql_thd = 0; @@ -1496,7 +1495,6 @@ inline void trx_t::commit_in_memory(const mtr_t *mtr) trx_mutex_enter(this); dict_operation= TRX_DICT_OP_NONE; - lock.was_chosen_as_deadlock_victim= false; DBUG_LOG("trx", "Commit in memory: " << this); state= TRX_STATE_NOT_STARTED; @@ -1510,9 +1508,10 @@ inline void trx_t::commit_in_memory(const mtr_t *mtr) wsrep= false; wsrep_commit_ordered(mysql_thd); } + lock.was_chosen_as_wsrep_victim= false; #endif /* WITH_WSREP */ - assert_trx_is_free(this); + assert_freed(); trx_init(this); trx_mutex_exit(this); From 72789e4b2b0f631635297c6bc785c7364d60ca9f Mon Sep 17 00:00:00 2001 From: Monty Date: Fri, 15 May 2020 15:14:08 +0300 Subject: [PATCH 3/4] Fixed access to not initalized memory Most of the volations came from: sel_arg_range_seq_next(void*, st_key_multi_range*) (opt_range_mrr.cc:342) --- sql/opt_range.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sql/opt_range.cc b/sql/opt_range.cc index bf968878a72..66c870dd2ac 100644 --- a/sql/opt_range.cc +++ b/sql/opt_range.cc @@ -2721,6 +2721,7 @@ int SQL_SELECT::test_quick_select(THD *thd, key_map keys_to_use, param.imerge_cost_buff_size= 0; param.using_real_indexes= TRUE; param.remove_jump_scans= TRUE; + param.max_key_parts= 0; param.remove_false_where_parts= remove_false_parts_of_where; param.force_default_mrr= ordered_output; param.possible_keys.clear_all(); @@ -3435,6 +3436,7 @@ bool calculate_cond_selectivity_for_table(THD *thd, TABLE *table, Item **cond) param.using_real_indexes= FALSE; param.real_keynr[0]= 0; param.alloced_sel_args= 0; + param.max_key_parts= 0; thd->no_errors=1; From a4996f951d731322acc63033646d950ddbb0f60c Mon Sep 17 00:00:00 2001 From: Aleksey Midenkov Date: Fri, 15 May 2020 16:17:15 +0300 Subject: [PATCH 4/4] MDEV-22563 Segfault on duplicate free of Item_func_in::array Same array instance in two Item_func_in instances. First Item_func_in instance is freed on table close. Second one is freed on cleanup_after_query(). get_copy() depends on copy ctor for copying an item and hence does shallow copy for default copy ctor. Use build_clone() for deep copy of Item_func_in. --- mysql-test/main/alter_table.result | 35 ++++++++++++++++++++++++++++++ mysql-test/main/alter_table.test | 35 ++++++++++++++++++++++++++++++ sql/field.cc | 2 +- sql/item.h | 5 +++++ 4 files changed, 76 insertions(+), 1 deletion(-) diff --git a/mysql-test/main/alter_table.result b/mysql-test/main/alter_table.result index 3d21b5c350d..8e8b4362ed8 100644 --- a/mysql-test/main/alter_table.result +++ b/mysql-test/main/alter_table.result @@ -2559,3 +2559,38 @@ drop view v1; # # End of 10.3 tests # +# +# MDEV-22563 Segfault on duplicate free of Item_func_in::array +# +create or replace table person_principal ( +person_id bigint not null, +insurant_id varchar(10) not null, +principal_id bigint not null, +principal_officer_id bigint not null, +nursing_degree tinyint null, +nursing_degree_valid_from date not null default cast(current_timestamp(6) as date), +carma_user_id bigint not null, +current_date_time timestamp(6) not null default current_timestamp(6) on update current_timestamp(6), +constraint pk_person_principal primary key (person_id asc), +constraint ck_person_principal_nursing_degree check (nursing_degree in (1,2,3,4,5))); +Warnings: +Warning 1280 Name 'pk_person_principal' ignored for PRIMARY key. +create or replace table person_principal_hist ( +person_id bigint not null, +insurant_id varchar(10) not null, +principal_id bigint not null, +principal_officer_id bigint not null, +nursing_degree tinyint null, +nursing_degree_valid_from date not null default cast(now() as date), +carma_user_id bigint not null, +orig_date_time datetime(6) not null, +constraint pk_person_principal_hist primary key (person_id asc, orig_date_time asc), +constraint ck_person_principal_hist_nursing_degree check (nursing_degree in (1,2,3,4,5))); +Warnings: +Warning 1280 Name 'pk_person_principal_hist' ignored for PRIMARY key. +insert into person_principal (person_id, insurant_id, principal_id, principal_officer_id, nursing_degree, nursing_degree_valid_from, carma_user_id) +values (1, 'A123456789', 5, 1, 1, '2018-05-06', 1); +alter table person_principal add column if not exists date_mask tinyint null; +update person_principal set date_mask = 0; +alter table person_principal modify column date_mask tinyint not null; +drop tables person_principal_hist, person_principal; diff --git a/mysql-test/main/alter_table.test b/mysql-test/main/alter_table.test index dc6983da38b..f090df56e5f 100644 --- a/mysql-test/main/alter_table.test +++ b/mysql-test/main/alter_table.test @@ -2076,3 +2076,38 @@ drop view v1; --echo # --echo # End of 10.3 tests --echo # + +--echo # +--echo # MDEV-22563 Segfault on duplicate free of Item_func_in::array +--echo # +create or replace table person_principal ( + person_id bigint not null, + insurant_id varchar(10) not null, + principal_id bigint not null, + principal_officer_id bigint not null, + nursing_degree tinyint null, + nursing_degree_valid_from date not null default cast(current_timestamp(6) as date), + carma_user_id bigint not null, + current_date_time timestamp(6) not null default current_timestamp(6) on update current_timestamp(6), + constraint pk_person_principal primary key (person_id asc), + constraint ck_person_principal_nursing_degree check (nursing_degree in (1,2,3,4,5))); + +create or replace table person_principal_hist ( + person_id bigint not null, + insurant_id varchar(10) not null, + principal_id bigint not null, + principal_officer_id bigint not null, + nursing_degree tinyint null, + nursing_degree_valid_from date not null default cast(now() as date), + carma_user_id bigint not null, + orig_date_time datetime(6) not null, + constraint pk_person_principal_hist primary key (person_id asc, orig_date_time asc), + constraint ck_person_principal_hist_nursing_degree check (nursing_degree in (1,2,3,4,5))); + +insert into person_principal (person_id, insurant_id, principal_id, principal_officer_id, nursing_degree, nursing_degree_valid_from, carma_user_id) +values (1, 'A123456789', 5, 1, 1, '2018-05-06', 1); +alter table person_principal add column if not exists date_mask tinyint null; +update person_principal set date_mask = 0; +alter table person_principal modify column date_mask tinyint not null; +drop tables person_principal_hist, person_principal; + diff --git a/sql/field.cc b/sql/field.cc index 5d946ef5d0c..9d118a15748 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -11396,7 +11396,7 @@ Virtual_column_info* Virtual_column_info::clone(THD *thd) return NULL; if (expr) { - dst->expr= expr->get_copy(thd); + dst->expr= expr->build_clone(thd); if (!dst->expr) return NULL; } diff --git a/sql/item.h b/sql/item.h index 54eaaaf8743..b83b997323d 100644 --- a/sql/item.h +++ b/sql/item.h @@ -1586,6 +1586,7 @@ public: virtual bool is_order_clause_position() const { return false; } /* cloning of constant items (0 if it is not const) */ virtual Item *clone_item(THD *thd) { return 0; } + /* deep copy item */ virtual Item* build_clone(THD *thd) { return get_copy(thd); } virtual cond_result eq_cmp_result() const { return COND_OK; } inline uint float_length(uint decimals_par) const @@ -2031,6 +2032,10 @@ public: virtual bool check_index_dependence(void *arg) { return 0; } /*============== End of Item processor list ======================*/ + /* + Does not guarantee deep copy (depends on copy ctor). + See build_clone() for deep copy. + */ virtual Item *get_copy(THD *thd)=0; bool cache_const_expr_analyzer(uchar **arg);