[mythtv-commits] Ticket #6305: Incorrect QWaitCondition & mutex usage in multiple places

[MythTV]

#6305: Incorrect QWaitCondition & mutex usage in multiple places
---------------------------------------+------------------------------------
 Reporter:  tomimo at ncircle.nullnet.fi  |       Owner:  ijr    
     Type:  defect                     |      Status:  new    
 Priority:  minor                      |   Milestone:  unknown
Component:  MythTV - General           |     Version:  head   
 Severity:  medium                     |     Mlocked:  0      
---------------------------------------+------------------------------------
 I've noticed a quite frequent hang due to frequent status queries:

 Example:

 Thread 12 (Thread 0x49396950 (LWP 11152)):
 #0  0x00007fdd7ee1c2d9 in pthread_cond_wait@@GLIBC_2.3.2 () from
 /lib/libpthread.so.0
 #1  0x00007fdd7d4ebb5b in ?? () from /usr/lib/libQtCore.so.4
 #2  0x00007fdd7d4e73cd in QMutex::lock () from /usr/lib/libQtCore.so.4
 #3  0x0000000000438e31 in QMutexLocker::relock (this=0x49393780) at
 /usr/include/qt4/QtCore/qmutex.h:116
 #4  0x000000000043bdd9 in QMutexLocker (this=0x49393780, m=0xa77af8) at
 /usr/include/qt4/QtCore/qmutex.h:98
 #5  0x00007fdd833642a5 in TVRec::IsBusy (this=0xa77a20, busy_input=0x0,
 time_buffer=5) at tv_rec.cpp:2466
 #6  0x00000000004422a7 in EncoderLink::IsBusy (this=0xa9a530,
 busy_input=0x0, time_buffer=5) at encoderlink.cpp:102
 #7  0x00000000004ff7fa in GetCurrentMaxBitrate (encoderList=0x776cc8) at
 backendutil.cpp:47
 #8  0x00000000005012e8 in BackendQueryDiskSpace (strlist=@0x49394ba0,
 encoderList=0x776cc8, consolidated=true, allHosts=true) at
 backendutil.cpp:193
 #9  0x0000000000453e96 in HttpStatus::FillStatusXML (this=0x7fdd70029170,
 pDoc=0x493952d0) at httpstatus.cpp:337
 #10 0x0000000000456fd3 in HttpStatus::GetStatusHTML (this=0x7fdd70029170,
 pRequest=0x7fdd7052ce10) at httpstatus.cpp:134
 #11 0x0000000000457236 in HttpStatus::ProcessRequest (this=0x7fdd70029170,
 pRequest=0x7fdd7052ce10) at httpstatus.cpp:88
 #12 0x00007fdd81a049be in HttpServer::DelegateRequest
 (this=0x7fdd7001d600, pThread=0x7fdd70009170, pRequest=0x7fdd7052ce10) at
 httpserver.cpp:171
 #13 0x00007fdd81a05196 in HttpWorkerThread::ProcessWork
 (this=0x7fdd70009170) at httpserver.cpp:297
 #14 0x00007fdd81a01879 in WorkerThread::WakeForWork (this=0x7fdd70009170)
 at threadpool.cpp:218
 #15 0x00007fdd81a01c88 in WorkerEvent::customEvent (this=0xa968a0,
 e=0x7fdd68830530) at threadpool.cpp:135
 #16 0x00007fdd7d5e3dfd in QObject::event () from /usr/lib/libQtCore.so.4
 #17 0x00007fdd7dd8fc3d in QApplicationPrivate::notify_helper () from
 /usr/lib/libQtGui.so.4
 #18 0x00007fdd7dd979ba in QApplication::notify () from
 /usr/lib/libQtGui.so.4
 #19 0x00007fdd7d5d4d61 in QCoreApplication::notifyInternal () from
 /usr/lib/libQtCore.so.4
 #20 0x00007fdd7d5d59fa in QCoreApplicationPrivate::sendPostedEvents ()
 from /usr/lib/libQtCore.so.4
 #21 0x00007fdd7d5fd4d3 in ?? () from /usr/lib/libQtCore.so.4
 #22 0x00007fdd79e14d3b in g_main_context_dispatch () from
 /usr/lib/libglib-2.0.so.0
 #23 0x00007fdd79e1850d in ?? () from /usr/lib/libglib-2.0.so.0
 #24 0x00007fdd79e186cb in g_main_context_iteration () from
 /usr/lib/libglib-2.0.so.0
 #25 0x00007fdd7d5fd15f in QEventDispatcherGlib::processEvents () from
 /usr/lib/libQtCore.so.4
 #26 0x00007fdd7d5d3682 in QEventLoop::processEvents () from
 /usr/lib/libQtCore.so.4
 #27 0x00007fdd7d5d380d in QEventLoop::exec () from /usr/lib/libQtCore.so.4
 #28 0x00007fdd7d4e93f8 in QThread::exec () from /usr/lib/libQtCore.so.4
 #29 0x00007fdd81a0146d in WorkerThread::run (this=0x7fdd70009170) at
 threadpool.cpp:265
 #30 0x00007fdd7d4ec362 in ?? () from /usr/lib/libQtCore.so.4
 #31 0x00007fdd7ee183ea in start_thread () from /lib/libpthread.so.0
 #32 0x00007fdd7ca4acbd in clone () from /lib/libc.so.6
 #33 0x0000000000000000 in ?? ()

 It looks there is multiple places where the QWaitCondition.wait/wakeall
 have been implemented incorrectly and therefore the code doesn't work as
 intended.

 Example:

 // Qt4 requires a QMutex as a parameter...
 // not sure if this is the best solution.  Mutex Must be locked before
 wait.
      QMutex mutex;
      mutex.lock();

      while (encoding)
      {
          if (request_pause)
          {
             mainpaused = true;
             pauseWait.wakeAll();
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 No locks around this and no predicative set
 to make sure that the signal is detected in the receiver

             if (IsPaused() && tvrec)
                 tvrec->RecorderPaused();

             unpauseWait.wait(&mutex, 100);
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 Uses a bogus mutex, doesn't have a proper locking around
 the wait-call and no predicative checking in order to detect
 if the wake-call has already been sent before we come to the wait()-call.

             if (cleartimeonpause)
                 gettimeofday(&stm, &tzone);
             continue;
 ------------------------------------------------------------

 I'll attach a patch file which shows how I fixed the problem in
 recorderbase.cpp and .h. Because these pauseWait's and unpauseWait's are
 being used in several places, I'll upload a full patch after I've verified
 that my fix doesn't have anmy additional bugs.

-- 
Ticket URL: <http://svn.mythtv.org/trac/ticket/6305>
MythTV <http://www.mythtv.org/>
MythTV