Commit ca40330d authored by Marko Mäkelä's avatar Marko Mäkelä

Fix a deadlock in thd_report_wait_for()

Unlike commit a54abf01 claimed,
the caller of THD::awake() may actually hold the InnoDB lock_sys->mutex.
That commit introduced a deadlock of threads in the replication slave
when running the test rpl.rpl_parallel_optimistic_nobinlog.

lock_trx_handle_wait(): Expect the callers to acquire and release
lock_sys->mutex and trx->mutex.

innobase_kill_query(): Restore the logic for conditionally acquiring
and releasing the mutexes. THD::awake() can be called from inside
InnoDB while holding one or both mutexes, via thd_report_wait_for() and
via wsrep_innobase_kill_one_trx().
parent dbb3960a
...@@ -4915,8 +4915,31 @@ static void innobase_kill_query(handlerton*, THD* thd, enum thd_kill_levels) ...@@ -4915,8 +4915,31 @@ static void innobase_kill_query(handlerton*, THD* thd, enum thd_kill_levels)
if (trx_t* trx = thd_to_trx(thd)) { if (trx_t* trx = thd_to_trx(thd)) {
ut_ad(trx->mysql_thd == thd); ut_ad(trx->mysql_thd == thd);
switch (trx->abort_type) {
case TRX_WSREP_ABORT:
break;
case TRX_SERVER_ABORT:
if (!wsrep_thd_is_BF(trx->mysql_thd, FALSE)) {
lock_mutex_enter();
}
/* fall through */
case TRX_REPLICATION_ABORT:
trx_mutex_enter(trx);
}
/* Cancel a pending lock request if there are any */ /* Cancel a pending lock request if there are any */
lock_trx_handle_wait(trx); lock_trx_handle_wait(trx);
switch (trx->abort_type) {
case TRX_WSREP_ABORT:
break;
case TRX_SERVER_ABORT:
if (!wsrep_thd_is_BF(trx->mysql_thd, FALSE)) {
lock_mutex_exit();
}
/* fall through */
case TRX_REPLICATION_ABORT:
trx_mutex_exit(trx);
}
} }
DBUG_VOID_RETURN; DBUG_VOID_RETURN;
......
...@@ -7977,26 +7977,19 @@ lock_trx_handle_wait( ...@@ -7977,26 +7977,19 @@ lock_trx_handle_wait(
/*=================*/ /*=================*/
trx_t* trx) /*!< in/out: trx lock state */ trx_t* trx) /*!< in/out: trx lock state */
{ {
dberr_t err; ut_ad(lock_mutex_own());
ut_ad(trx_mutex_own(trx));
lock_mutex_enter();
trx_mutex_enter(trx);
if (trx->lock.was_chosen_as_deadlock_victim) { if (trx->lock.was_chosen_as_deadlock_victim) {
err = DB_DEADLOCK; return DB_DEADLOCK;
} else if (trx->lock.wait_lock != NULL) { }
lock_cancel_waiting_and_release(trx->lock.wait_lock); if (!trx->lock.wait_lock) {
err = DB_LOCK_WAIT;
} else {
/* The lock was probably granted before we got here. */ /* The lock was probably granted before we got here. */
err = DB_SUCCESS; return DB_SUCCESS;
} }
lock_mutex_exit(); lock_cancel_waiting_and_release(trx->lock.wait_lock);
trx_mutex_exit(trx); return DB_LOCK_WAIT;
return(err);
} }
/*********************************************************************//** /*********************************************************************//**
......
...@@ -4615,7 +4615,11 @@ row_search_for_mysql( ...@@ -4615,7 +4615,11 @@ row_search_for_mysql(
a deadlock and the transaction had to wait then a deadlock and the transaction had to wait then
release the lock it is waiting on. */ release the lock it is waiting on. */
lock_mutex_enter();
trx_mutex_enter(trx);
err = lock_trx_handle_wait(trx); err = lock_trx_handle_wait(trx);
lock_mutex_exit();
trx_mutex_exit(trx);
switch (err) { switch (err) {
case DB_SUCCESS: case DB_SUCCESS:
......
...@@ -5511,8 +5511,31 @@ static void innobase_kill_query(handlerton*, THD* thd, enum thd_kill_levels) ...@@ -5511,8 +5511,31 @@ static void innobase_kill_query(handlerton*, THD* thd, enum thd_kill_levels)
#endif /* WITH_WSREP */ #endif /* WITH_WSREP */
if (trx_t* trx = thd_to_trx(thd)) { if (trx_t* trx = thd_to_trx(thd)) {
ut_ad(trx->mysql_thd == thd); ut_ad(trx->mysql_thd == thd);
switch (trx->abort_type) {
case TRX_WSREP_ABORT:
break;
case TRX_SERVER_ABORT:
if (!wsrep_thd_is_BF(trx->mysql_thd, FALSE)) {
lock_mutex_enter();
}
/* fall through */
case TRX_REPLICATION_ABORT:
trx_mutex_enter(trx);
}
/* Cancel a pending lock request if there are any */ /* Cancel a pending lock request if there are any */
lock_trx_handle_wait(trx); lock_trx_handle_wait(trx);
switch (trx->abort_type) {
case TRX_WSREP_ABORT:
break;
case TRX_SERVER_ABORT:
if (!wsrep_thd_is_BF(trx->mysql_thd, FALSE)) {
lock_mutex_exit();
}
/* fall through */
case TRX_REPLICATION_ABORT:
trx_mutex_exit(trx);
}
} }
DBUG_VOID_RETURN; DBUG_VOID_RETURN;
......
...@@ -8087,26 +8087,19 @@ lock_trx_handle_wait( ...@@ -8087,26 +8087,19 @@ lock_trx_handle_wait(
/*=================*/ /*=================*/
trx_t* trx) /*!< in/out: trx lock state */ trx_t* trx) /*!< in/out: trx lock state */
{ {
dberr_t err; ut_ad(lock_mutex_own());
ut_ad(trx_mutex_own(trx));
lock_mutex_enter();
trx_mutex_enter(trx);
if (trx->lock.was_chosen_as_deadlock_victim) { if (trx->lock.was_chosen_as_deadlock_victim) {
err = DB_DEADLOCK; return DB_DEADLOCK;
} else if (trx->lock.wait_lock != NULL) { }
lock_cancel_waiting_and_release(trx->lock.wait_lock); if (!trx->lock.wait_lock) {
err = DB_LOCK_WAIT;
} else {
/* The lock was probably granted before we got here. */ /* The lock was probably granted before we got here. */
err = DB_SUCCESS; return DB_SUCCESS;
} }
lock_mutex_exit(); lock_cancel_waiting_and_release(trx->lock.wait_lock);
trx_mutex_exit(trx); return DB_LOCK_WAIT;
return(err);
} }
/*********************************************************************//** /*********************************************************************//**
......
...@@ -4627,7 +4627,11 @@ row_search_for_mysql( ...@@ -4627,7 +4627,11 @@ row_search_for_mysql(
a deadlock and the transaction had to wait then a deadlock and the transaction had to wait then
release the lock it is waiting on. */ release the lock it is waiting on. */
lock_mutex_enter();
trx_mutex_enter(trx);
err = lock_trx_handle_wait(trx); err = lock_trx_handle_wait(trx);
lock_mutex_exit();
trx_mutex_exit(trx);
switch (err) { switch (err) {
case DB_SUCCESS: case DB_SUCCESS:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment