Fixes #10870. Fix Start/Stop race condition in StreamHandler.

Take two... The locking order documented in streamhandler.h wasn't being adhered to in the DVBStreamHandler. The actual locking order is now documented and a new lock has been added for AddListener/RemoveListener as per the suggested code from Roger James.
MythTV · Jul 24, 2012 · f612576 · f612576
1 parent 3825701
commit f612576
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 20 deletions.
diff --git a/mythtv/libs/libmythtv/recorders/streamhandler.cpp b/mythtv/libs/libmythtv/recorders/streamhandler.cpp
@@ -26,9 +26,15 @@ StreamHandler::StreamHandler(const QString &device) :
 
 StreamHandler::~StreamHandler()
 {
-    if (!_stream_data_list.empty())
+    QMutexLocker locker(&_add_rm_lock);
+
     {
-        LOG(VB_GENERAL, LOG_ERR, LOC + "dtor & _stream_data_list not empty");
+        QMutexLocker locker2(&_listener_lock);
+        if (!_stream_data_list.empty())
+        {
+            LOG(VB_GENERAL, LOG_ERR, LOC +
+                "dtor & _stream_data_list not empty");
+        }
     }
 
     // This should never be triggered.. just to be safe..
@@ -41,6 +47,8 @@ void StreamHandler::AddListener(MPEGStreamData *data,
                                 bool needs_buffering,
                                 QString output_file)
 {
+    QMutexLocker locker(&_add_rm_lock);
+
     LOG(VB_RECORD, LOG_INFO, LOC + QString("AddListener(0x%1) -- begin")
                 .arg((uint64_t)data,0,16));
     if (!data)
@@ -93,6 +101,8 @@ void StreamHandler::AddListener(MPEGStreamData *data,
 
 void StreamHandler::RemoveListener(MPEGStreamData *data)
 {
+    QMutexLocker locker(&_add_rm_lock);
+
     LOG(VB_RECORD, LOG_INFO, LOC + QString("RemoveListener(0x%1) -- begin")
                 .arg((uint64_t)data,0,16));
     if (!data)
@@ -158,12 +168,6 @@ void StreamHandler::Start(void)
     while (!_running && !_error && _running_desired)
         _running_state_changed.wait(&_start_stop_lock, 100);
 
-    if (!_running_desired)
-    {
-        LOG(VB_GENERAL, LOG_WARNING, LOC +
-            "Programmer Error: Stop called before Start finished");
-    }
-
     if (_error)
     {
         LOG(VB_GENERAL, LOG_WARNING, LOC + "Start failed");
@@ -175,17 +179,9 @@ void StreamHandler::Stop(void)
 {
     QMutexLocker locker(&_start_stop_lock);
 
-    do
-    {
-        SetRunningDesired(false);
-        while (!_running_desired && _running)
-            _running_state_changed.wait(&_start_stop_lock, 100);
-        if (_running_desired)
-        {
-            LOG(VB_GENERAL, LOG_WARNING, LOC +
-                "Programmer Error: Start called before Stop finished");
-        }
-    } while (_running_desired);
+    SetRunningDesired(false);
+    while (_running)
+        _running_state_changed.wait(&_start_stop_lock, 100);
 
     wait();
 }

diff --git a/mythtv/libs/libmythtv/recorders/streamhandler.h b/mythtv/libs/libmythtv/recorders/streamhandler.h
@@ -45,7 +45,9 @@ class PIDInfo
 typedef QMap<uint,PIDInfo*> PIDInfoMap;
 
 // locking order
-// _pid_lock -> _listener_lock -> _start_stop_lock
+// _pid_lock -> _listener_lock
+// _add_rm_lock -> _listener_lock
+//              -> _start_stop_lock
 
 class StreamHandler : protected MThread, public DeviceReaderCB
 {
@@ -101,6 +103,8 @@ class StreamHandler : protected MThread, public DeviceReaderCB
     bool              _needs_buffering;
     bool              _allow_section_reader;
 
+    QMutex            _add_rm_lock;
+
     mutable QMutex    _start_stop_lock;
     volatile bool     _running_desired;
     volatile bool     _error;