MDEV-8869: Potential lock_sys->mutex deadlock

In wsrep brute force (BF) we have already took lock_sys and trx
mutex either on wsrep_abort_transaction() or
before wsrep_kill_victim().

4 files changed, 42 insertions, 31 deletions

diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc
index 7afc90d9709..8bc00e40051 100644
--- a/storage/innobase/handler/ha_innodb.cc
+++ b/storage/innobase/handler/ha_innodb.cc
@@ -4334,7 +4334,6 @@ innobase_kill_query(
         enum thd_kill_levels level) /*!< in: kill level */
 {
 	trx_t*	trx;
-	bool	took_lock_sys = false;
 
 	DBUG_ENTER("innobase_kill_query");
 	DBUG_ASSERT(hton == innodb_hton_ptr);
@@ -4356,29 +4355,28 @@ innobase_kill_query(
 	trx = thd_to_trx(thd);
 
 	if (trx) {
-		THD *cur = current_thd;
-		THD *owner = trx->current_lock_mutex_owner;
+		/* In wsrep BF we have already took lock_sys and trx
+		mutex either on wsrep_abort_transaction() or
+		before wsrep_kill_victim() */
 
-		/* Cancel a pending lock request. */
-		if (owner != cur) {
+		if (!wsrep_thd_is_BF(trx->mysql_thd, FALSE)) {
 			ut_ad(!lock_mutex_own());
+			ut_ad(!trx_mutex_own(trx));
 			lock_mutex_enter();
-			took_lock_sys = true;
+			trx_mutex_enter(trx);
 		}
 
-		ut_ad(!trx_mutex_own(trx));
-		trx_mutex_enter(trx);
+		ut_ad(lock_mutex_own());
+		ut_ad(trx_mutex_own(trx));
 
+		/* We did dirty read above in case when abort is not
+		from wsrep. */
 		if (trx->lock.wait_lock) {
 			lock_cancel_waiting_and_release(trx->lock.wait_lock);
 		}
 
-	        ut_ad(lock_mutex_own());
-		ut_ad(trx_mutex_own(trx));
-
-		trx_mutex_exit(trx);
-
-		if (took_lock_sys) {
+		if (!wsrep_thd_is_BF(trx->mysql_thd, FALSE)) {
+			trx_mutex_exit(trx);
 			lock_mutex_exit();
 		}
 	}
@@ -17560,13 +17558,13 @@ wsrep_abort_transaction(
 
 	if (victim_trx) {
 		lock_mutex_enter();
-		victim_trx->current_lock_mutex_owner = victim_thd;
 		trx_mutex_enter(victim_trx);
+		victim_trx->wsrep_abort = TRUE;
 		int rcode = wsrep_innobase_kill_one_trx(bf_thd, bf_trx,
                                                         victim_trx, signal);
 		trx_mutex_exit(victim_trx);
-		victim_trx->current_lock_mutex_owner = NULL;
 		lock_mutex_exit();
+		victim_trx->wsrep_abort = FALSE;
 		wsrep_srv_conc_cancel_wait(victim_trx);
 		DBUG_RETURN(rcode);
 	} else {
diff --git a/storage/innobase/include/lock0lock.h b/storage/innobase/include/lock0lock.h
index 88246afebdc..eb7ab5dce9d 100644
--- a/storage/innobase/include/lock0lock.h
+++ b/storage/innobase/include/lock0lock.h
@@ -654,6 +654,16 @@ lock_get_type(
 	const lock_t*	lock);	/*!< in: lock */
 
 /*******************************************************************//**
+Gets the trx of the lock. Non-inline version for using outside of the
+lock module.
+@return	trx_t* */
+UNIV_INTERN
+trx_t*
+lock_get_trx(
+/*=========*/
+	const lock_t*	lock);	/*!< in: lock */
+
+/*******************************************************************//**
 Gets the id of the transaction owning a lock.
 @return	transaction id */
 UNIV_INTERN
diff --git a/storage/innobase/include/trx0trx.h b/storage/innobase/include/trx0trx.h
index dd26e420aa2..6a58dcd25da 100644
--- a/storage/innobase/include/trx0trx.h
+++ b/storage/innobase/include/trx0trx.h
@@ -848,6 +848,8 @@ struct trx_t{
 	/*------------------------------*/
 	THD*		mysql_thd;	/*!< MySQL thread handle corresponding
 					to this trx, or NULL */
+	ibool		wsrep_abort;	/*!< Transaction is aborted from
+					wsrep functions. */
 	const char*	mysql_log_file_name;
 					/*!< if MySQL binlog is used, this field
 					contains a pointer to the latest file
@@ -992,11 +994,6 @@ struct trx_t{
 					count of tables being flushed. */
 
 	/*------------------------------*/
-	THD*		current_lock_mutex_owner;
-					/*!< If this is equal to current_thd,
-					then in innobase_kill_query() we know we
-					already hold the lock_sys->mutex. */
-	/*------------------------------*/
 #ifdef UNIV_DEBUG
 	ulint		start_line;	/*!< Track where it was started from */
 	const char*	start_file;	/*!< Filename where it was started */
diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc
index f72f7f03018..b072aea2432 100644
--- a/storage/innobase/lock/lock0lock.cc
+++ b/storage/innobase/lock/lock0lock.cc
@@ -1729,8 +1729,10 @@ wsrep_kill_victim(
 				}
 			}
 
+			lock->trx->wsrep_abort = TRUE;
 			wsrep_innobase_kill_one_trx(trx->mysql_thd,
 				(const trx_t*) trx, lock->trx, TRUE);
+			lock->trx->wsrep_abort = FALSE;
 		}
 	}
 }
@@ -4399,9 +4401,7 @@ lock_report_waiters_to_mysql(
 				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->current_lock_mutex_owner = mysql_thd;
 				thd_report_wait_for(mysql_thd, w_trx->mysql_thd);
-				w_trx->current_lock_mutex_owner = NULL;
 			}
 			++i;
 		}
@@ -6769,7 +6769,7 @@ lock_clust_rec_modify_check_and_lock(
 
 	lock_mutex_enter();
 	trx_t*		trx = thr_get_trx(thr);
-	trx->current_lock_mutex_owner = trx->mysql_thd;
+
 	ut_ad(lock_table_has(trx, index->table, LOCK_IX));
 
 	err = lock_rec_lock(TRUE, LOCK_X | LOCK_REC_NOT_GAP,
@@ -6777,7 +6777,6 @@ lock_clust_rec_modify_check_and_lock(
 
 	MONITOR_INC(MONITOR_NUM_RECLOCK_REQ);
 
-	trx->current_lock_mutex_owner = NULL;
 	lock_mutex_exit();
 
 	ut_ad(lock_rec_queue_validate(FALSE, block, rec, index, offsets));
@@ -6831,7 +6830,6 @@ lock_sec_rec_modify_check_and_lock(
 
 	trx_t* trx = thr_get_trx(thr);
 	lock_mutex_enter();
-	trx->current_lock_mutex_owner = trx->mysql_thd;
 
 	ut_ad(lock_table_has(trx, index->table, LOCK_IX));
 
@@ -6840,7 +6838,6 @@ lock_sec_rec_modify_check_and_lock(
 
 	MONITOR_INC(MONITOR_NUM_RECLOCK_REQ);
 
-	trx->current_lock_mutex_owner = NULL;
 	lock_mutex_exit();
 
 #ifdef UNIV_DEBUG
@@ -6933,7 +6930,6 @@ lock_sec_rec_read_check_and_lock(
 
 	trx_t* trx = thr_get_trx(thr);
 	lock_mutex_enter();
-	trx->current_lock_mutex_owner = trx->mysql_thd;
 
 	ut_ad(mode != LOCK_X
 	      || lock_table_has(trx, index->table, LOCK_IX));
@@ -6945,7 +6941,6 @@ lock_sec_rec_read_check_and_lock(
 
 	MONITOR_INC(MONITOR_NUM_RECLOCK_REQ);
 
-	trx->current_lock_mutex_owner = NULL;
 	lock_mutex_exit();
 
 	ut_ad(lock_rec_queue_validate(FALSE, block, rec, index, offsets));
@@ -7008,7 +7003,6 @@ lock_clust_rec_read_check_and_lock(
 
 	lock_mutex_enter();
 	trx_t* trx = thr_get_trx(thr);
-	trx->current_lock_mutex_owner = trx->mysql_thd;
 
 	ut_ad(mode != LOCK_X
 	      || lock_table_has(trx, index->table, LOCK_IX));
@@ -7020,7 +7014,6 @@ lock_clust_rec_read_check_and_lock(
 
 	MONITOR_INC(MONITOR_NUM_RECLOCK_REQ);
 
-	trx->current_lock_mutex_owner = NULL;
 	lock_mutex_exit();
 
 	ut_ad(lock_rec_queue_validate(FALSE, block, rec, index, offsets));
@@ -7167,6 +7160,19 @@ lock_get_type(
 }
 
 /*******************************************************************//**
+Gets the trx of the lock. Non-inline version for using outside of the
+lock module.
+@return	trx_t* */
+UNIV_INTERN
+trx_t*
+lock_get_trx(
+/*=========*/
+	const lock_t*	lock)	/*!< in: lock */
+{
+	return (lock->trx);
+}
+
+/*******************************************************************//**
 Gets the id of the transaction owning a lock.
 @return	transaction id */
 UNIV_INTERN