Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-88247

Memory ordering problem in QBasicMutex::lockInternal()

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: P2: Important
    • Resolution: Done
    • Affects Version/s: 5.15.1
    • Fix Version/s: 5.12.11
    • Component/s: Core: Threads
    • Labels:
      None
    • Commits:
      d08e3b6de16118becaada17a58aed4042f400a5a (qt/qtbase/dev) 90d227a58f3ecf530ac25ae80403cb945d616fb9 (qt/qtbase/5.15) 940414aff1de8b963edf064b864a05c5cdfb5094 (qt/qtbase/5.12)

      Description

      We observe that a thread hangs

      (lldb) bt
      * thread #3, name = 'QThread'
        * frame #0: 0x00000001841fa634 libsystem_kernel.dylib`semaphore_wait_trap + 8
          frame #1: 0x0000000102cebc5c QtCore`QBasicMutex::lockInternal(int) [inlined] QMutexPrivate::wait(this=<unavailable>, timeout=-1) at qmutex_mac.cpp:70:17 [opt]
          frame #2: 0x0000000102cebc18 QtCore`QBasicMutex::lockInternal(this=0x000000011ea041a8, timeout=-1) at qmutex.cpp:645 [opt]
          frame #3: 0x0000000102ceb8a4 QtCore`QMutex::lock() [inlined] QBasicMutex::lockInternal(this=<unavailable>) at qmutex.cpp:561:5 [opt]
          frame #4: 0x0000000102ceb89c QtCore`QMutex::lock() [inlined] QBasicMutex::lock(this=<unavailable>) at qmutex.h:81 [opt]
          frame #5: 0x0000000102ceb890 QtCore`QMutex::lock() [inlined] QRecursiveMutexPrivate::lock(this=0x000000011ea04190, timeout=-1) at qmutex.cpp:778 [opt]
          frame #6: 0x0000000102ceb870 QtCore`QMutex::lock(this=<unavailable>) at qmutex.cpp:233 [opt]
      

      while trying to acquire a mutex that is unlocked:

      (lldb) f 5
      frame #5: 0x0000000102ceb890 QtCore`QMutex::lock() [inlined] QRecursiveMutexPrivate::lock(this=0x000000011ea04190, timeout=-1) at qmutex.cpp:778 [opt]
      (lldb) p *this
      (QRecursiveMutexPrivate) $5 = {
        QMutexData = (recursive = true)
        owner = {
          QBasicAtomicPointer<void> = {
            _q_value = {
              Value = 0x0000000000000000
            }
          }
        }
        count = 0
        mutex = {
          QBasicMutex = {
            d_ptr = {
              _q_value = {
                Value = 0x0000000000000000
              }
            }
          }
        }
      }
      

      The blocked thread appears to cache a stale value of QBasicMutex::d_ptr:

      (lldb) p *(QMutexPrivate*)$x21
      (QMutexPrivate) $9 = {
        QMutexData = (recursive = false)
        refCount = {
          QAtomicInteger<int> = {
            QBasicAtomicInteger<int> = {
              _q_value = {
                Value = 1
              }
            }
          }
        }
        id = 0
        waiters = {
          QAtomicInteger<int> = {
            QBasicAtomicInteger<int> = {
              _q_value = {
                Value = 1
              }
            }
          }
        }
        possiblyUnlocked = {
          QAtomicInteger<int> = {
            QBasicAtomicInteger<int> = {
              _q_value = {
                Value = 0
              }
            }
          }
        }
        mach_semaphore = 22787
      }
      

      We believe that the problem is that the store to QMutexPrivate::waiters and the load from QBasicMutex::d_ptr are not ordered:

          Thread 1                               Thread 2
          --------                               --------
          
           QBasicMutex::lockInternal()            QBasicMutex::unlockInternal()
          
            d_ptr.testAndSetOrdered(..., d)        d = d_ptr.loadAcquire()
          
                                                   d->waiters.fetchAndAddRelease() (1)
            d->waiters.testAndSetRelaxed(...) (2)
                                                   d_ptr.testAndSetRelease(d, 0)   (3)
            if (d != d_ptr.loadAcquire())     (4)
          
            d->wait()
      

      The operation (4) isn't serialised with (2) so its memory effect may be observed before the effect of the operation (2). However, if memory effects are observed in the following order (1) -> (4) -> (2) -> (3) then Thread 1 doesn't notice that Thread 2 updates d_ptr and goes to sleep with d pointing to a stale object, this object isn't reachable since d_ptr is zeroed so Thread 1 cannot be woken up

        Attachments

          Activity

            People

            Assignee:
            ca2plus Alexander Kartashov
            Reporter:
            ca2plus Alexander Kartashov
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved: