Revert MDEV-18464 and MDEV-12009

This reverts commit 21b2fada7ab7f35c898c02d2f918461409cc9c8e
and commit 81d71ee6b21870772c336bff15b71904914f146a.

The MDEV-18464 change introduces a few data race issues. Contrary to
the documentation, the field trx_t::victim is not always being protected
by lock_sys_t::mutex and trx_t::mutex. Most importantly, it seems
that KILL QUERY could wrongly avoid acquiring both mutexes when
invoking lock_trx_handle_wait_low(), in case another thread had
already set trx->victim=true.

We also revert MDEV-12009, because it should depend on the MDEV-18464
fix being present.

1 files changed, 18 insertions, 32 deletions

diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc
index 33563a03f1a..f06fcd6c4d8 100644
--- a/storage/innobase/lock/lock0lock.cc
+++ b/storage/innobase/lock/lock0lock.cc
@@ -1,7 +1,7 @@
 /*****************************************************************************
 
 Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved.
-Copyright (c) 2014, 2019, MariaDB Corporation.
+Copyright (c) 2014, 2018, 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
@@ -1793,8 +1793,10 @@ wsrep_kill_victim(
 				}
 			}
 
+			lock->trx->abort_type = TRX_WSREP_ABORT;
 			wsrep_innobase_kill_one_trx(trx->mysql_thd,
 				(const trx_t*) trx, lock->trx, TRUE);
+			lock->trx->abort_type = TRX_SERVER_ABORT;
 		}
 	}
 }
@@ -4780,11 +4782,12 @@ lock_report_waiters_to_mysql(
 			if (w_trx->id != victim_trx_id) {
 				/* If thd_report_wait_for() decides to kill the
 				transaction, then we will get a call back into
-				innobase_kill_query.*/
-				trx_mutex_enter(w_trx);
-				w_trx->victim = true;
+				innobase_kill_query. We mark this by setting
+				current_lock_mutex_owner, so we can avoid trying
+				to recursively take lock_sys->mutex. */
+				w_trx->abort_type = TRX_REPLICATION_ABORT;
 				thd_report_wait_for(mysql_thd, w_trx->mysql_thd);
-				trx_mutex_exit(w_trx);
+				w_trx->abort_type = TRX_SERVER_ABORT;
 			}
 			++i;
 		}
@@ -7964,23 +7967,6 @@ lock_trx_release_locks(
 	lock_mutex_exit();
 }
 
-inline dberr_t lock_trx_handle_wait_low(trx_t* trx)
-{
-	ut_ad(lock_mutex_own());
-	ut_ad(trx_mutex_own(trx));
-
-	if (trx->lock.was_chosen_as_deadlock_victim) {
-		return DB_DEADLOCK;
-	}
-	if (!trx->lock.wait_lock) {
-		/* The lock was probably granted before we got here. */
-		return DB_SUCCESS;
-	}
-
-	lock_cancel_waiting_and_release(trx->lock.wait_lock);
-	return DB_LOCK_WAIT;
-}
-
 /*********************************************************************//**
 Check whether the transaction has already been rolled back because it
 was selected as a deadlock victim, or if it has to wait then cancel
@@ -7992,19 +7978,19 @@ lock_trx_handle_wait(
 /*=================*/
 	trx_t*	trx)	/*!< in/out: trx lock state */
 {
-	if (!trx->victim) {
-		lock_mutex_enter();
-		trx_mutex_enter(trx);
-	}
-
-	dberr_t err = lock_trx_handle_wait_low(trx);
+	ut_ad(lock_mutex_own());
+	ut_ad(trx_mutex_own(trx));
 
-	if (!trx->victim) {
-		lock_mutex_exit();
-		trx_mutex_exit(trx);
+	if (trx->lock.was_chosen_as_deadlock_victim) {
+		return DB_DEADLOCK;
+	}
+	if (!trx->lock.wait_lock) {
+		/* The lock was probably granted before we got here. */
+		return DB_SUCCESS;
 	}
 
-	return err;
+	lock_cancel_waiting_and_release(trx->lock.wait_lock);
+	return DB_LOCK_WAIT;
 }
 
 /*********************************************************************//**