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

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

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

          is caused by

          Bug - A problem which impairs or prevents the functions of the product. MDEV-24142 rw_lock_t has unnecessarily complex wait logic

          • Major - Major loss of function.
          • Closed

          Activity

            People

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

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.