Merge pull request #9510 from tamland/fix_peripherals_deadlock

[peripherals] fix deadlock

4 files changed, 31 insertions, 14 deletions

diff --git a/xbmc/peripherals/Peripherals.cpp b/xbmc/peripherals/Peripherals.cpp
index 46fceba39c..51cd664749 100644
--- a/xbmc/peripherals/Peripherals.cpp
+++ b/xbmc/peripherals/Peripherals.cpp
@@ -115,10 +115,7 @@ void CPeripherals::Initialise()
 
     /* initialise all known busses and run an initial scan for devices */
     for (auto& bus : m_busses)
-    {
       bus->Initialise();
-      bus->TriggerDeviceScan();
-    }
   }
 
   m_eventScanner.Start();
@@ -165,8 +162,13 @@ void CPeripherals::Clear()
 
 void CPeripherals::TriggerDeviceScan(const PeripheralBusType type /* = PERIPHERAL_BUS_UNKNOWN */)
 {
-  CSingleLock lock(m_critSectionBusses);
-  for (auto& bus : m_busses)
+  std::vector<PeripheralBusPtr> busses;
+  {
+    CSingleLock lock(m_critSectionBusses);
+    busses = m_busses;
+  }
+
+  for (auto& bus : busses)
   {
     bool bScan = false;
 
@@ -252,7 +254,7 @@ int CPeripherals::GetPeripheralsWithFeature(std::vector<CPeripheral *> &results,
 size_t CPeripherals::GetNumberOfPeripherals() const
 {
   size_t iReturn(0);
-  CSingleLock lock(m_critSection);
+  CSingleLock lock(m_critSectionBusses);
   for (const auto& bus : m_busses)
     iReturn += bus->GetNumberOfPeripherals();
 
diff --git a/xbmc/peripherals/bus/PeripheralBus.cpp b/xbmc/peripherals/bus/PeripheralBus.cpp
index c7a6966b77..b58696b873 100644
--- a/xbmc/peripherals/bus/PeripheralBus.cpp
+++ b/xbmc/peripherals/bus/PeripheralBus.cpp
@@ -60,11 +60,15 @@ void CPeripheralBus::OnDeviceRemoved(const std::string &strLocation)
 
 void CPeripheralBus::Clear(void)
 {
-  if (m_bNeedsPolling)
   {
-    m_bStop = true;
-    m_triggerEvent.Set();
-    StopThread(true);
+    CSingleLock lock(m_critSection);
+    if (m_bNeedsPolling)
+    {
+      m_bStop = true;
+      lock.Leave();
+      m_triggerEvent.Set();
+      StopThread(true);
+    }
   }
 
   CSingleLock lock(m_critSection);
@@ -134,6 +138,7 @@ bool CPeripheralBus::ScanForDevices(void)
     bReturn = true;
   }
 
+  CSingleLock lock(m_critSection);
   m_bInitialised = true;
   return bReturn;
 }
@@ -214,10 +219,18 @@ void CPeripheralBus::Process(void)
     if (!ScanForDevices())
       break;
 
+    // depending on bus implementation
+    // needsPolling can be set properly
+    // only after unitial scan.
+    // if this is the case, bail out.
+    if (!m_bNeedsPolling)
+      break;
+
     if (!m_bStop)
       m_triggerEvent.WaitMSec(m_iRescanTime);
   }
 
+  CSingleLock lock(m_critSection);
   m_bIsStarted = false;
 }
 
@@ -322,5 +335,6 @@ CPeripheral *CPeripheralBus::GetByPath(const std::string &strPath) const
 
 size_t CPeripheralBus::GetNumberOfPeripherals() const
 {
+  CSingleLock lock(m_critSection);
   return m_peripherals.size();
 }
diff --git a/xbmc/peripherals/bus/PeripheralBus.h b/xbmc/peripherals/bus/PeripheralBus.h
index 230409dda3..8093ca2ee6 100644
--- a/xbmc/peripherals/bus/PeripheralBus.h
+++ b/xbmc/peripherals/bus/PeripheralBus.h
@@ -54,7 +54,7 @@ namespace PERIPHERALS
     /*!
      * @return True if this bus needs to be polled for changes, false if this bus performs updates via callbacks
      */
-    bool NeedsPolling(void) const { return m_bNeedsPolling; }
+    bool NeedsPolling(void) const { CSingleLock lock(m_critSection); return m_bNeedsPolling; }
 
     /*!
      * @brief Get the instance of the peripheral at the given location.
@@ -149,7 +149,7 @@ namespace PERIPHERALS
 
     virtual bool FindComPort(std::string &strLocation) { return false; }
 
-    virtual bool IsInitialised(void) const { return m_bInitialised; }
+    virtual bool IsInitialised(void) const { CSingleLock lock(m_critSection); return m_bInitialised; }
 
     /*!
      * \brief Poll for events
@@ -174,8 +174,8 @@ namespace PERIPHERALS
     bool                       m_bInitialised;
     bool                       m_bIsStarted;
     bool                       m_bNeedsPolling; /*!< true when this bus needs to be polled for new devices, false when it uses callbacks to notify this bus of changed */
-    CPeripherals *             m_manager;
-    PeripheralBusType          m_type;
+    CPeripherals *const        m_manager;
+    const PeripheralBusType    m_type;
     CCriticalSection           m_critSection;
     CEvent                     m_triggerEvent;
   };
diff --git a/xbmc/peripherals/bus/linux/PeripheralBusUSBLibUdev.cpp b/xbmc/peripherals/bus/linux/PeripheralBusUSBLibUdev.cpp
index 85da3823b5..15709c696f 100644
--- a/xbmc/peripherals/bus/linux/PeripheralBusUSBLibUdev.cpp
+++ b/xbmc/peripherals/bus/linux/PeripheralBusUSBLibUdev.cpp
@@ -209,6 +209,7 @@ void CPeripheralBusUSB::Process(void)
       ScanForDevices();
   }
 
+  CSingleLock lock(m_critSection);
   m_bIsStarted = false;
 }