Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-25512

Deadlock between sux_lock::u_x_upgrade() and sux_lock::u_lock()

    XMLWordPrintable

    Details

      Description

      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.

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              marko Marko Mäkelä
              Reporter:
              marko Marko Mäkelä
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: