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

QObject::connect using loadRelaxed to synchronize non-atomic writes

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Reported
    • Priority: P2: Important
    • Resolution: Unresolved
    • Affects Version/s: 5.15.0 RC2, 6.2, 6.4
    • Fix Version/s: None
    • Component/s: Core: Object Model
    • Labels:
      None

      Description

      In the following program:

      #include <QtWidgets/QApplication>                                                                                       
      #include <QtCore/QThread>                                                                                               
                                                                                                                              
      int main(int argc, char** argv)                                                                                             
      {                                                                                                                       
          QApplication qa(argc, argv);                                                                                        
                                                                                                                              
          QThread qthread;                                                                                                    
          qthread.start();                                                                                                       
                                                                                                                                 
          qa.connect(&qthread, SIGNAL(finished()), &qa, SLOT(closeAllWindows()));                                                
                                                                                                                                 
          for (int i =0; i < 10000000; ++i) {}                                                                                                                                                                           
                                                                                                                                                                                   
          qthread.exit();                                                                                                        
          qthread.wait();                                                                                                     
      }

      I get the following thread sanitizer violation:

      ==================                                                                                                                                                                                                 
      WARNING: ThreadSanitizer: data race (pid=150110)                                                                        
        Atomic read of size 8 at 0x7b0c00024998 by thread T2:                                                                 
          #0 __tsan_atomic64_load /tmp/gcc-v9.3.0d1rh65_lnx86/gcc.source/libsanitizer/tsan/tsan_interface_atomic.cc:538 (libtsan.so.0+0x64499)
          #1 std::__atomic_base<QObjectPrivate::SignalVector*>::load(std::memory_order) const /gcc/include/c++/9.3.0/bits/atomic_base.h:740 (libQt5Core.so.5+0x44511d)
          #2 std::atomic<QObjectPrivate::SignalVector*>::load(std::memory_order) const /gcc/include/c++/9.3.0/atomic:519 (libQt5Core.so.5+0x44511d)
          #3 QObjectPrivate::SignalVector* QAtomicOps<QObjectPrivate::SignalVector*>::loadRelaxed<QObjectPrivate::SignalVector*>(std::atomic<QObjectPrivate::SignalVector*> const&) ../../include/QtCore/../../../../qt5/qtbase/src/corelib/thread/qatomic_cxx11.h:239 (libQt5Core.so.5+0x44511d)
          #4 QBasicAtomicPointer<QObjectPrivate::SignalVector>::loadRelaxed() const ../../include/QtCore/../../../../qt5/qtbase/src/corelib/thread/qbasicatomic.h:248 (libQt5Core.so.5+0x44511d)
          #5 QObjectPrivate::maybeSignalConnected(unsigned int) const /inout/qt5/qtbase/src/corelib/kernel/qobject.cpp:482 (libQt5Core.so.5+0x44511d)
          #6 void doActivate<false>(QObject*, int, void**) /inout/qt5/qtbase/src/corelib/kernel/qobject.cpp:3788 (libQt5Core.so.5+0x4594a6)
          #7 QMetaObject::activate(QObject*, QMetaObject const*, int, void**) /inout/qt5/qtbase/src/corelib/kernel/qobject.cpp:3946 (libQt5Core.so.5+0x4511ce)
          #8 QThread::started(QThread::QPrivateSignal) .moc/moc_qthread.cpp:163 (libQt5Core.so.5+0xfd0b0)                     
          #9 QThreadPrivate::start(void*) /inout/qt5/qtbase/src/corelib/thread/qthread_unix.cpp:324 (libQt5Core.so.5+0x1008ab)
      
        Previous write of size 8 at 0x7b0c00024998 by main thread:                                                            
          #0 operator new(unsigned long) /tmp/gcc-v9.3.0d1rh65_lnx86/gcc.source/libsanitizer/tsan/tsan_new_delete.cc:63 (libtsan.so.0+0x731fa)
          #1 QObjectPrivate::ensureConnectionData() /inout/qt5/qtbase/src/corelib/kernel/qobject_p.h:373 (libQt5Core.so.5+0x444b81)
          #2 QObjectPrivate::addConnection(int, QObjectPrivate::Connection*) /inout/qt5/qtbase/src/corelib/kernel/qobject.cpp:324 (libQt5Core.so.5+0x444b81)
          #3 QMetaObjectPrivate::connect(QObject const*, int, QMetaObject const*, QObject const*, int, QMetaObject const*, int, int*) /inout/qt5/qtbase/src/corelib/kernel/qobject.cpp:3436 (libQt5Core.so.5+0x447584)
          #4 QObject::connect(QObject const*, char const*, QObject const*, char const*, Qt::ConnectionType) /inout/qt5/qtbase/src/corelib/kernel/qobject.cpp:2894 (libQt5Core.so.5+0x44d8d5)
          #5 main /home/breno/SCRs/tsan/qt/main.cpp:11 (main+0x4012b9)
      
        Location is heap block of size 40 at 0x7b0c00024990 allocated by main thread:                                         
          #0 operator new(unsigned long) /tmp/gcc-v9.3.0d1rh65_lnx86/gcc.source/libsanitizer/tsan/tsan_new_delete.cc:63 (libtsan.so.0+0x731fa)
          #1 QObjectPrivate::ensureConnectionData() /inout/qt5/qtbase/src/corelib/kernel/qobject_p.h:373 (libQt5Core.so.5+0x444b81)
          #2 QObjectPrivate::addConnection(int, QObjectPrivate::Connection*) /inout/qt5/qtbase/src/corelib/kernel/qobject.cpp:324 (libQt5Core.so.5+0x444b81)
          #3 QMetaObjectPrivate::connect(QObject const*, int, QMetaObject const*, QObject const*, int, QMetaObject const*, int, int*) /inout/qt5/qtbase/src/corelib/kernel/qobject.cpp:3436 (libQt5Core.so.5+0x447584)
          #4 QObject::connect(QObject const*, char const*, QObject const*, char const*, Qt::ConnectionType) /inout/qt5/qtbase/src/corelib/kernel/qobject.cpp:2894 (libQt5Core.so.5+0x44d8d5)
          #5 main /home/breno/SCRs/tsan/qt/main.cpp:11 (main+0x4012b9) 
      
        Thread T2 'QThread' (tid=150113, running) created by main thread at:                                                  
          #0 pthread_create /tmp/gcc-v9.3.0d1rh65_lnx86/gcc.source/libsanitizer/tsan/tsan_interceptors.cc:964 (libtsan.so.0+0x2d56b)
          #1 QThread::start(QThread::Priority) /inout/qt5/qtbase/src/corelib/thread/qthread_unix.cpp:703 (libQt5Core.so.5+0x10010b)
          #2 main /home/breno/SCRs/tsan/qt/main.cpp:9 (main+0x401278) 

      Problem here, as far as I can tell, is that QObject is using relaxed atomics to synchronize other memory. On qobject_p.h we have this runnin in the main thread:

          void ensureConnectionData()                                                                                            
          {                                                                                                                      
              if (connections.loadRelaxed())                                                                                     
                  return;                                                                                                        
              ConnectionData *cd = new ConnectionData;                                                                           
              cd->ref.ref();                                                                                                     
              connections.storeRelaxed(cd);                                                                                      
          }

      It allocates and writes to memory positions in ConnectionData, then set the pointer in the atomic.

      While in the other thread we have qobject.cpp:

      bool QObjectPrivate::maybeSignalConnected(uint signalIndex) const                                                       
      {                                                                                                                       
          ConnectionData *cd = connections.loadRelaxed();                                                                     
          if (!cd)                                                                                                            
              return false;                                                                                                   
          SignalVector *signalVector = cd->signalVector.loadRelaxed();
      

      But in the second thread, it's entirely possible that it can see the pointer to ConnectionData set to something, but it did not see the initialization of ConnectionData yet.
      So, when reading cd->signalVector, there would be a data race.

      Another version of this issue but in a reduced, qt-free example, is:

      #include <atomic>                                                                                                       
      #include <thread>                                                                                                       
                                                                                                                              
      int main(int argc, char** argv)                                                                                         
      {                                                                                                                       
          int* i = new int;                                                                                                   
          std::atomic<bool> init {false};                                                                                     
                                                                                                                              
          std::thread t([&] {                                                                                                 
              // Just wait for a little bit                                                                                   
              for (int i = 0; i < 1000000; ++i) {}                                                                            
                                                                                                                              
              if (init.load(std::memory_order_relaxed))                                                                                                                                                                  
                  int k = *i;                                                                                                 
          });                                                                                                                 
                                                                                                                              
          if (!init)                                                                                                          
          {                                                                                                                   
              *i = 42;                                                                                                        
              init = true;                                                                                                    
          }                                                                                                                   
                                                                                                                              
          t.join();                                                                                                           
      } 
      
      

      Here, we cannot guarantee that reading the {int  k = *i} will result in 42.

        Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

          Activity

            People

            Assignee:
            mmutz Marc Mutz
            Reporter:
            breno_guimaraes Breno Rodrigues GuimarĂ£es
            PM Owner:
            Vladimir Minenko Vladimir Minenko
            RnD Owner:
            Alex Blasche Alex Blasche
            Votes:
            0 Vote for this issue
            Watchers:
            8 Start watching this issue

              Dates

              Created:
              Updated:

                Gerrit Reviews

                There are no open Gerrit changes