No subject


Sun Mar 1 18:40:12 UTC 2009


careless condition wait handling and therefore potential races. I'm not
exactly sure what it so different in my test machine (perhaps its just way
too overloaded) that it seems to be able catch quite easily these kind of
synchronization problems, but at least its possible to find at at least
the most often occurring race conditions and propably even figure out a
way to patch them.

Regards,
Tomi Orava
-- 

------=_20090402111933_72116
Content-Type: text/x-patch; name="tv_rec.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="tv_rec.patch"

Index: libs/libmythtv/tv_rec.h
===================================================================
--- libs/libmythtv/tv_rec.h	(revision 20298)
+++ libs/libmythtv/tv_rec.h	(working copy)
@@ -251,12 +251,13 @@ class MPUBLIC TVRec : public SignalMonitorListener
 
     static TVRec *GetTVRec(uint cardid);
 
-    virtual void AllGood(void) { triggerEventLoop.wakeAll(); }
+    virtual void AllGood(void) { wakeEventLoop(); }
     virtual void StatusSignalLock(const SignalMonitorValue&) { }
     virtual void StatusSignalStrength(const SignalMonitorValue&) { }
 
   protected:
     void RunTV(void);
+
     bool WaitForEventThreadSleep(bool wake = true, ulong time = ULONG_MAX);
     static void *EventThread(void *param);
     static void *RecorderThread(void *param);
@@ -265,6 +266,7 @@ class MPUBLIC TVRec : public SignalMonitorListener
     void SetRingBuffer(RingBuffer *);
     void SetPseudoLiveTVRecording(ProgramInfo*);
     void TeardownAll(void);
+    void wakeEventLoop(void);
 
     static bool GetDevices(int cardid,
                            GeneralDBOptions   &general_opts,
@@ -378,8 +380,15 @@ class MPUBLIC TVRec : public SignalMonitorListener
     TuningQueue    tuningRequests;
     TuningRequest  lastTuningRequest;
     QDateTime      eitScanStartTime;
+
     QWaitCondition triggerEventLoop;
+    QMutex         triggerEventLoopMutex;
+    bool           triggerEventLoopSignal;
+
     QWaitCondition triggerEventSleep;
+    QMutex         triggerEventSleepMutex;
+    bool           triggerEventSleepSignal;
+
     bool           m_switchingBuffer;
 
     // Current recording info
Index: libs/libmythtv/tv_rec.cpp
===================================================================
--- libs/libmythtv/tv_rec.cpp	(revision 20298)
+++ libs/libmythtv/tv_rec.cpp	(working copy)
@@ -125,6 +125,8 @@ TVRec::TVRec(int capturecardnum)
       internalState(kState_None), desiredNextState(kState_None),
       changeState(false), pauseNotify(true),
       stateFlags(0), lastTuningRequest(0),
+      triggerEventLoopSignal(false),
+      triggerEventSleepSignal(false),
       m_switchingBuffer(false),
       // Current recording info
       curRecording(NULL), autoRunJobs(JOB_NONE),
@@ -309,6 +311,19 @@ void TVRec::TeardownAll(void)
     SetRingBuffer(NULL);
 }
 
+/** \fn TVRec::wakeEventLoop(void)
+ *  \brief Trigger the EventLoopThread.
+ *
+ *
+ */
+void TVRec::wakeEventLoop(void)
+{
+    triggerEventLoopMutex.lock();
+    triggerEventLoopSignal = true;
+    triggerEventLoop.wakeAll();
+    triggerEventLoopMutex.unlock();
+}
+
 /** \fn TVRec::GetState() const
  *  \brief Returns the TVState of the recorder.
  *
@@ -437,6 +452,8 @@ QDateTime TVRec::GetRecordEndTime(const ProgramInf
  */
 void TVRec::CancelNextRecording(bool cancel)
 {
+    QMutexLocker lock(&stateChangeLock);
+
     VERBOSE(VB_RECORD, LOC + "CancelNextRecording("<<cancel<<") -- begin");
 
     PendingMap::iterator it = pendingRecordings.find(cardid);
@@ -951,7 +968,8 @@ void TVRec::ChangeState(TVState nextState)
 
     desiredNextState = nextState;
     changeState = true;
-    triggerEventLoop.wakeAll();
+    lock.unlock();
+    wakeEventLoop();
 }
 
 /** \fn TVRec::SetupRecorder(RecordingProfile&)
@@ -1371,11 +1389,6 @@ void TVRec::RunTV(void)
     else
         eitScanStartTime = eitScanStartTime.addYears(1);
 
-    // 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 (HasFlags(kFlagRunMainLoop))
     {
         // If there is a state change queued up, do it...
@@ -1526,11 +1539,21 @@ void TVRec::RunTV(void)
         // WaitforEventThreadSleep() will still work...
         if (tuningRequests.empty() && !changeState)
         {
+            lock.mutex()->unlock();
+
+            triggerEventSleepMutex.lock();
+            triggerEventSleepSignal = true;
             triggerEventSleep.wakeAll();
-            lock.mutex()->unlock();
+            triggerEventSleepMutex.unlock();
+
             sched_yield();
-            triggerEventSleep.wakeAll();
-            triggerEventLoop.wait(&mutex, 1000 /* ms */);
+
+            triggerEventLoopMutex.lock();
+            if (triggerEventLoopSignal == false)
+                triggerEventLoop.wait(&triggerEventLoopMutex, 1000 /* ms */);
+            triggerEventLoopSignal = false;
+            triggerEventLoopMutex.unlock();
+
             lock.mutex()->lock();
         }
     }
@@ -1542,27 +1565,33 @@ void TVRec::RunTV(void)
     }
 }
 
+/** \fn TVRec::WaitForEventThreadSleep(bool wake, ulong time)
+ *
+ *  You MUST HAVE the stateChange-lock locked when you call this method!
+ */
+
 bool TVRec::WaitForEventThreadSleep(bool wake, ulong time)
 {
-    // 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();
-
     bool ok = false;
     MythTimer t;
     t.start();
 
     while (!ok && ((unsigned long) t.elapsed()) < time)
     {
+        stateChangeLock.unlock();
         if (wake)
-            triggerEventLoop.wakeAll();
+            wakeEventLoop();
 
-        stateChangeLock.unlock();
-        // It is possible for triggerEventSleep.wakeAll() to be sent
-        // before we enter wait so we only wait 100 ms so we can try
-        // again a few times before 15 second timeout on frontend...
-        triggerEventSleep.wait(&mutex, 100);
+        VERBOSE(VB_IMPORTANT, QString("changeState=%1, tuningRequests=%2, time=%3")
+                .arg(changeState).arg(tuningRequests.size()).arg((unsigned long) t.elapsed() < time));
+
+
+        triggerEventSleepMutex.lock();
+        if (triggerEventSleepSignal == false)
+            triggerEventSleep.wait(&triggerEventSleepMutex);
+        triggerEventSleepSignal = false;
+        triggerEventSleepMutex.unlock();
+
         stateChangeLock.lock();
 
         // verify that we were triggered.
@@ -2440,6 +2469,7 @@ bool TVRec::CheckChannelPrefix(const QString &pref
 bool TVRec::SetVideoFiltersForChannel(uint  sourceid,
                                       const QString &channum)
 {
+    QMutexLocker lock(&stateChangeLock);
     if (!recorder)
         return false;
 
@@ -2459,6 +2489,7 @@ bool TVRec::SetVideoFiltersForChannel(uint  source
  */
 bool TVRec::IsReallyRecording(void)
 {
+    QMutexLocker lock(&stateChangeLock);
     return ((recorder && recorder->IsRecording()) ||
             HasFlags(kFlagDummyRecorderRunning));
 }
@@ -2738,6 +2769,8 @@ static uint get_input_id(uint cardid, const QStrin
  */
 void TVRec::NotifySchedulerOfRecording(ProgramInfo *rec)
 {
+    QMutexLocker locker(&stateChangeLock);
+
     if (!channel)
         return;
 
@@ -2852,8 +2885,8 @@ void TVRec::StopLiveTV(void)
     VERBOSE(VB_RECORD, LOC + "StopLiveTV(void) curRec: "<<curRecording
             <<" pseudoRec: "<<pseudoLiveTVRecording);
 
-    if (internalState != kState_WatchingLiveTV)
-        return;
+    if (internalState == kState_None)
+        return; // already stopped
 
     bool hadPseudoLiveTVRec = pseudoLiveTVRecording;
     CheckForRecGroupChange();
@@ -2910,8 +2943,7 @@ void TVRec::RecorderPaused(void)
 {
     if (pauseNotify)
     {
-        QMutexLocker lock(&stateChangeLock);
-        triggerEventLoop.wakeAll();
+        wakeEventLoop();
     }
 }
 
@@ -3463,7 +3495,7 @@ bool TVRec::TuningOnSameMultiplex(TuningRequest &r
     return false;
 }
 
-/** \fn TVRec::HandleTuning(void)
+/** \fn TVRecHleTuning(void)
  *  \brief Handles all tuning events.
  *
  *   This method pops tuning events off the tuningState queue
@@ -4216,7 +4248,8 @@ void TVRec::SetFlags(uint f)
     stateFlags |= f;
     VERBOSE(VB_RECORD, LOC + QString("SetFlags(%1) -> %2")
             .arg(FlagToString(f)).arg(FlagToString(stateFlags)));
-    triggerEventLoop.wakeAll();
+    lock.unlock();
+    wakeEventLoop();
 }
 
 void TVRec::ClearFlags(uint f)
@@ -4225,7 +4258,8 @@ void TVRec::ClearFlags(uint f)
     stateFlags &= ~f;
     VERBOSE(VB_RECORD, LOC + QString("ClearFlags(%1) -> %2")
             .arg(FlagToString(f)).arg(FlagToString(stateFlags)));
-    triggerEventLoop.wakeAll();
+    lock.unlock();
+    wakeEventLoop();
 }
 
 QString TVRec::FlagToString(uint f)
------=_20090402111933_72116--




More information about the mythtv-dev mailing list