Commit 9f136700 authored by Marko Mäkelä's avatar Marko Mäkelä

MDEV-24738 fixup: heap-use-after-poison in lock_sys_t::deadlock_check()

Deadlock::report(): Require the caller to acquire lock_sys.latch
if invoking on a transaction that is now owned by the current thread.
parent e92c34ce
...@@ -5894,14 +5894,15 @@ namespace Deadlock ...@@ -5894,14 +5894,15 @@ namespace Deadlock
@return nullptr if no deadlock */ @return nullptr if no deadlock */
static trx_t *report(trx_t *const trx, bool current_trx= true) static trx_t *report(trx_t *const trx, bool current_trx= true)
{ {
mysql_mutex_unlock(&lock_sys.wait_mutex); mysql_mutex_assert_owner(&lock_sys.wait_mutex);
ut_ad(lock_sys.is_writer() == !current_trx);
/* Normally, trx should be a direct part of the deadlock /* Normally, trx should be a direct part of the deadlock
cycle. However, if innodb_deadlock_detect had been OFF in the cycle. However, if innodb_deadlock_detect had been OFF in the
past, our trx may be waiting for a lock that is held by a past, or if current_trx=false, trx may be waiting for a lock that
participant of a pre-existing deadlock, without being part of the is held by a participant of a pre-existing deadlock, without being
deadlock itself. That is, the path to the deadlock may be P-shaped part of the deadlock itself. That is, the path to the deadlock may be
instead of O-shaped, with trx being at the foot of the P. P-shaped instead of O-shaped, with trx being at the foot of the P.
We will process the entire path leading to a cycle, and we will We will process the entire path leading to a cycle, and we will
choose the victim (to be aborted) among the cycle. */ choose the victim (to be aborted) among the cycle. */
...@@ -5909,23 +5910,31 @@ namespace Deadlock ...@@ -5909,23 +5910,31 @@ namespace Deadlock
static const char rollback_msg[]= "*** WE ROLL BACK TRANSACTION (%u)\n"; static const char rollback_msg[]= "*** WE ROLL BACK TRANSACTION (%u)\n";
char buf[9 + sizeof rollback_msg]; char buf[9 + sizeof rollback_msg];
/* trx is owned by this thread; we can safely call these without /* If current_trx=true, trx is owned by this thread, and we can
holding any mutex */ safely invoke these without holding trx->mutex or lock_sys.latch.
If current_trx=false, a concurrent commit is protected by both
lock_sys.latch and lock_sys.wait_mutex. */
const undo_no_t trx_weight= TRX_WEIGHT(trx) | const undo_no_t trx_weight= TRX_WEIGHT(trx) |
(trx->mysql_thd && thd_has_edited_nontrans_tables(trx->mysql_thd) (trx->mysql_thd && thd_has_edited_nontrans_tables(trx->mysql_thd)
? 1ULL << 63 : 0); ? 1ULL << 63 : 0);
trx_t *victim= nullptr; trx_t *victim= nullptr;
undo_no_t victim_weight= ~0ULL; undo_no_t victim_weight= ~0ULL;
unsigned victim_pos= 0, trx_pos= 0; unsigned victim_pos= 0, trx_pos= 0;
if (current_trx && !lock_sys.wr_lock_try())
{ {
unsigned l= 0; mysql_mutex_unlock(&lock_sys.wait_mutex);
LockMutexGuard g{SRW_LOCK_CALL}; lock_sys.wr_lock(SRW_LOCK_CALL);
mysql_mutex_lock(&lock_sys.wait_mutex); mysql_mutex_lock(&lock_sys.wait_mutex);
}
{
unsigned l= 0;
/* Now that we are holding lock_sys.wait_mutex again, check /* Now that we are holding lock_sys.wait_mutex again, check
whether a cycle still exists. */ whether a cycle still exists. */
trx_t *cycle= find_cycle(trx); trx_t *cycle= find_cycle(trx);
if (!cycle) if (!cycle)
return nullptr; /* One of the transactions was already aborted. */ goto func_exit; /* One of the transactions was already aborted. */
for (trx_t *next= cycle;;) for (trx_t *next= cycle;;)
{ {
next= next->lock.wait_trx; next= next->lock.wait_trx;
...@@ -6027,6 +6036,9 @@ namespace Deadlock ...@@ -6027,6 +6036,9 @@ namespace Deadlock
#endif #endif
} }
func_exit:
if (current_trx)
lock_sys.wr_unlock();
return victim; return victim;
} }
} }
...@@ -6058,11 +6070,27 @@ void lock_sys_t::deadlock_check() ...@@ -6058,11 +6070,27 @@ void lock_sys_t::deadlock_check()
{ {
lock_sys.assert_unlocked(); lock_sys.assert_unlocked();
mysql_mutex_assert_owner(&lock_sys.wait_mutex); mysql_mutex_assert_owner(&lock_sys.wait_mutex);
bool locked= false;
if (Deadlock::to_be_checked) if (Deadlock::to_be_checked)
{ {
while (!Deadlock::to_check.empty()) for (;;)
{ {
auto i= Deadlock::to_check.begin(); auto i= Deadlock::to_check.begin();
if (i == Deadlock::to_check.end())
break;
if (!locked)
{
locked= lock_sys.wr_lock_try();
if (!locked)
{
locked= true;
mysql_mutex_unlock(&lock_sys.wait_mutex);
lock_sys.wr_lock(SRW_LOCK_CALL);
mysql_mutex_lock(&lock_sys.wait_mutex);
continue;
}
}
trx_t *trx= *i; trx_t *trx= *i;
Deadlock::to_check.erase(i); Deadlock::to_check.erase(i);
if (Deadlock::find_cycle(trx)) if (Deadlock::find_cycle(trx))
...@@ -6071,6 +6099,8 @@ void lock_sys_t::deadlock_check() ...@@ -6071,6 +6099,8 @@ void lock_sys_t::deadlock_check()
Deadlock::to_be_checked= false; Deadlock::to_be_checked= false;
} }
ut_ad(Deadlock::to_check.empty()); ut_ad(Deadlock::to_check.empty());
if (locked)
lock_sys.wr_unlock();
} }
......
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