[#MDEV-25512] Deadlock between sux_lock::u_x_upgrade() and sux_lock::u

A unit test program revealed a deadlock between sux_lock::u_x_upgrade() and sux_lock::u_lock() operations. The u_x_upgrade() operation is very rarely used, and contention with u_lock() is very rare. It has only been observed when using the non-default setting innodb_adaptive_hash_index=ON.

The futex-based implementation in the 10.6.0 server works correctly. Only the fallback to POSIX mutex and condition variables is affected. The following patch fixes the issue:

diff --git a/storage/innobase/include/rw_lock.h b/storage/innobase/include/rw_lock.h

index 94f59f761c0..3f1d76d8d97 100644

--- a/storage/innobase/include/rw_lock.h

+++ b/storage/innobase/include/rw_lock.h

@@ -161,13 +161,14 @@ class rw_lock

   bool read_unlock()

     auto l= lock.fetch_sub(1, std::memory_order_release);

+    DBUG_ASSERT(!(l & WRITER)); /* no write lock must have existed */

 #ifdef SUX_LOCK_GENERIC

     DBUG_ASSERT(~(WRITER_PENDING | UPDATER) & l); /* at least one read lock */

+    return (~(WRITER_PENDING | UPDATER) & l) == 1;

 #else /* SUX_LOCK_GENERIC */

     DBUG_ASSERT(~(WRITER_PENDING) & l); /* at least one read lock */

-#endif /* SUX_LOCK_GENERIC */

-    DBUG_ASSERT(!(l & WRITER)); /* no write lock must have existed */

     return (~WRITER_PENDING & l) == 1;

+#endif /* SUX_LOCK_GENERIC */

 #ifdef SUX_LOCK_GENERIC

   /** Release an update lock */

diff --git a/storage/innobase/sync/srw_lock.cc b/storage/innobase/sync/srw_lock.cc

index 63fc88285cb..b76194c89e3 100644

--- a/storage/innobase/sync/srw_lock.cc

+++ b/storage/innobase/sync/srw_lock.cc

@@ -111,13 +111,13 @@ void ssux_lock_low::update_lock(uint32_t l)

do

-    if (l == WRITER_WAITING)

+    if ((l | UPDATER) == (UPDATER | WRITER_WAITING))

     wake_writer:

       pthread_mutex_lock(&mutex);

       for (;;)

-        if (l == WRITER_WAITING)

+        if ((l | UPDATER) == (UPDATER | WRITER_WAITING))

           pthread_cond_signal(&cond_exclusive);

         l= value();

         if (!(l & WRITER_PENDING))

@@ -133,7 +133,7 @@ void ssux_lock_low::update_lock(uint32_t l)

         ut_delay(srv_spin_wait_delay);

         if (update_trylock(l))

           return;

-        else if (l == WRITER_WAITING)

+        else if ((l | UPDATER) == (UPDATER | WRITER_WAITING))

           goto wake_writer;

The cause of the problem is that this implementation cannot remember more than one pending exclusive lock request. If multiple requests have been enqueued, the WRITER_WAITING flag will be cleared when the first waiting writer acquires the exclusive lock. If the other waiting writers are not woken up, the flag would not be set.

The refinement of the futex-based implementation (Linux, OpenBSD, Microsoft Windows) in ~~MDEV-25404~~ is not affected by this, because we can reliably count up to 2,147,483,648 pending requests.

[MDEV-25512] Deadlock between sux_lock::u_x_upgrade() and sux_lock::u_lock() Created: 2021-04-25 Updated: 2021-04-25 Resolved: 2021-04-25
Status:	Closed
Project:	MariaDB Server
Component/s:	Storage Engine - InnoDB
Affects Version/s:	10.6.0
Fix Version/s:	10.6.1

[MDEV-25512] Deadlock between sux_lock::u_x_upgrade() and sux_lock::u_lock() Created: 2021-04-25 Updated: 2021-04-25 Resolved: 2021-04-25