Commit 272a1289 authored by Marko Mäkelä's avatar Marko Mäkelä

MDEV-24884 Hang in ssux_lock_low::write_lock()

ssux_lock_low::write_lock(): Before invoking writer_wait(), keep
attempting write_lock_wait_try() as long as no conflict exists.

rw_lock::upgrade_trylock(): Relax a bogus assertion and correct
the acquisition operation. Another thread may be executing in
ssux_lock_low::write_lock() on the same latch. Because we are the
only thread that can make progress on that latch, we must become
the writer. Any waiting thread will be eventually woken up by
ssux_lock_low::u_unlock() or ssux_lock_low::wr_unlock(), but not
by wr_u_downgrade() because the upgrade is a very rare operation.
parent 584e5211
...@@ -92,16 +92,22 @@ class rw_lock ...@@ -92,16 +92,22 @@ class rw_lock
bool upgrade_trylock() bool upgrade_trylock()
{ {
auto l= UPDATER; auto l= UPDATER;
while (!lock.compare_exchange_strong(l, l ^ (WRITER | UPDATER), while (!lock.compare_exchange_strong(l, WRITER,
std::memory_order_acquire, std::memory_order_acquire,
std::memory_order_relaxed)) std::memory_order_relaxed))
{ {
DBUG_ASSERT(!(~l & (UPDATER - 1))); /* Either conflicting (read) locks have been granted, or
the WRITER_WAITING flag was set by some thread that is waiting
to become WRITER. */
DBUG_ASSERT(!(~l & (UPDATER - 1)) || l == (UPDATER | WRITER_WAITING));
DBUG_ASSERT(((WRITER | UPDATER) & l) == UPDATER); DBUG_ASSERT(((WRITER | UPDATER) & l) == UPDATER);
if (~(WRITER_WAITING | UPDATER) & l) if (~(WRITER_WAITING | UPDATER) & l)
return false; return false;
} }
DBUG_ASSERT((l & ~WRITER_WAITING) == UPDATER); DBUG_ASSERT((l & ~WRITER_WAITING) == UPDATER);
/* Any thread that had set WRITER_WAITING will eventually be woken
up by ssux_lock_low::x_unlock() or ssux_lock_low::u_unlock()
(not ssux_lock_low::wr_u_downgrade() to keep the code simple). */
return true; return true;
} }
/** Downgrade an exclusive lock to an update lock. */ /** Downgrade an exclusive lock to an update lock. */
......
...@@ -204,7 +204,8 @@ void ssux_lock_low::write_lock(bool holding_u) ...@@ -204,7 +204,8 @@ void ssux_lock_low::write_lock(bool holding_u)
ut_delay(srv_spin_wait_delay); ut_delay(srv_spin_wait_delay);
} }
l= holding_u ? WRITER_WAITING | UPDATER : WRITER_WAITING; const uint32_t e= holding_u ? WRITER_WAITING | UPDATER : WRITER_WAITING;
l= e;
if (write_lock_wait_try(l)) if (write_lock_wait_try(l))
return; return;
...@@ -220,7 +221,11 @@ void ssux_lock_low::write_lock(bool holding_u) ...@@ -220,7 +221,11 @@ void ssux_lock_low::write_lock(bool holding_u)
if (holding_u && upgrade_trylock()) if (holding_u && upgrade_trylock())
return; return;
} }
l= write_lock_wait_start() | WRITER_WAITING;
for (l= write_lock_wait_start() | WRITER_WAITING;
(l | WRITER_WAITING) == e; )
if (write_lock_wait_try(l))
return;
} }
else else
DBUG_ASSERT(~WRITER_WAITING & l); DBUG_ASSERT(~WRITER_WAITING & l);
......
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