Merge pull request xbmc#18964 from ksooo/pvr-fix-startstop-races

[PVR] Fix PVR manager start/stop races

xbmc/pvr/PVRManager.cpp

@@ -311,7 +311,6 @@ void CPVRManager::Clear()
 {
  m_playbackState->Clear();
  m_pendingUpdates->Clear();
- m_epgContainer.Clear();
 
  CSingleLock lock(m_critSection);
 
@@ -393,6 +392,7 @@ void CPVRManager::Stop()
 
  m_addons->Stop();
  m_pendingUpdates->Stop();
+ m_timers->Stop();
  m_epgContainer.Stop();
  m_guiInfo->Stop();
 
@@ -506,8 +506,12 @@ void CPVRManager::Process()
  return;
  }
 
+ // Load EPGs from database.
+ m_epgContainer.Load();
+
  m_guiInfo->Start();
- m_epgContainer.Start(true);
+ m_epgContainer.Start();
+ m_timers->Start();
  m_pendingUpdates->Start();
 
  SetState(ManagerStateStarted);
@@ -661,6 +665,7 @@ void CPVRManager::UnloadComponents()
  m_recordings->Unload();
  m_timers->Unload();
  m_channelGroups->Unload();
+ m_epgContainer.Unload();
 }
 
 void CPVRManager::TriggerPlayChannelOnStartup()

xbmc/pvr/epg/EpgContainer.cpp

@@ -113,7 +113,7 @@ CPVREpgContainer::CPVREpgContainer() :
 CPVREpgContainer::~CPVREpgContainer()
 {
  Stop();
- Clear();
+ Unload();
 }
 
 std::shared_ptr<CPVREpgDatabase> CPVREpgContainer::GetEpgDatabase() const
@@ -138,106 +138,69 @@ int CPVREpgContainer::NextEpgId()
  return ++m_iNextEpgId;
 }
 
-void CPVREpgContainer::Clear()
+void CPVREpgContainer::Start()
 {
- /* make sure the update thread is stopped */
- bool bThreadRunning = !m_bStop;
- if (bThreadRunning)
- Stop();
+ Stop();
 
- std::vector<std::shared_ptr<CPVREpg>> epgs;
  {
  CSingleLock lock(m_critSection);
-
- /* clear all epg tables and remove pointers to epg tables on channels */
- for (const auto& epgEntry : m_epgIdToEpgMap)
- epgs.emplace_back(epgEntry.second);
-
- m_epgIdToEpgMap.clear();
- m_channelUidToEpgMap.clear();
- m_iNextEpgUpdate = 0;
- m_bStarted = false;
  m_bIsInitialising = true;
- m_iNextEpgId = 0;
- m_bUpdateNotificationPending = false;
- }
 
- for (const auto& epg : epgs)
- epg->Events().Unsubscribe(this);
+ CheckPlayingEvents();
 
- m_events.Publish(PVREvent::EpgContainer);
+ Create();
+ SetPriority(-1);
 
- if (bThreadRunning)
-  Start(true);
+  m_bStarted = true;
+ }
 }
 
-class CPVREpgContainerStartJob : public CJob
+void CPVREpgContainer::Stop()
 {
-public:
- CPVREpgContainerStartJob() = default;
- ~CPVREpgContainerStartJob() override = default;
+ StopThread();
 
- bool DoWork() override
  {
- CServiceBroker::GetPVRManager().EpgContainer().Start(false);
- return true;
+ CSingleLock lock(m_critSection);
+ m_bStarted = false;
  }
-};
+}
 
-void CPVREpgContainer::Start(bool bAsync)
+bool CPVREpgContainer::Load()
 {
- if (bAsync)
- {
- CPVREpgContainerStartJob* job = new CPVREpgContainerStartJob();
- CJobManager::GetInstance().AddJob(job, NULL);
- return;
- }
-
- Stop();
+ // EPGs must be loaded via PVR Manager -> channel groups -> EPG container to associate the
+ // channels with the right EPG.
+ CServiceBroker::GetPVRManager().TriggerEpgsCreate();
+ return true;
+}
 
+void CPVREpgContainer::Unload()
+{
+ std::vector<std::shared_ptr<CPVREpg>> epgs;
  {
  CSingleLock lock(m_critSection);
 
- m_bIsInitialising = true;
- m_bStop = false;
+ /* clear all epg tables and remove pointers to epg tables on channels */
+ for (const auto& epgEntry : m_epgIdToEpgMap)
+ epgs.emplace_back(epgEntry.second);
+
+ m_epgIdToEpgMap.clear();
+ m_channelUidToEpgMap.clear();
+ m_epgTagChanges.clear();
+ m_updateRequests.clear();
 
  m_iNextEpgUpdate = 0;
+ m_iNextEpgId = 0;
  m_iNextEpgActiveTagCheck = 0;
  m_bUpdateNotificationPending = false;
- }
-
- LoadFromDB();
-
- bool bStop = false;
- {
- CSingleLock lock(m_critSection);
- bStop = m_bStop;
- if (!m_bStop)
- {
- CheckPlayingEvents();
-
- Create();
- SetPriority(-1);
-
- m_bStarted = true;
- }
- }
+ m_bLoaded = false;
 
- if (!bStop)
- {
- CServiceBroker::GetPVRManager().TriggerEpgsCreate();
- CLog::Log(LOGINFO, "EPG thread started");
+ m_database->Close();
  }
-}
 
-void CPVREpgContainer::Stop()
-{
- StopThread();
-
- m_database->Close();
+ for (const auto& epg : epgs)
+ epg->Events().Unsubscribe(this);
 
- CSingleLock lock(m_critSection);
- m_bStarted = false;
+ m_events.Publish(PVREvent::EpgContainer);
 }
 
 void CPVREpgContainer::Notify(const PVREvent& event)

xbmc/pvr/epg/EpgContainer.h

@@ -65,19 +65,24 @@ namespace PVR
 
  /*!
  * @brief Start the EPG update thread.
- * @param bAsync Should the EPG container starts asynchronously
  */
- void Start(bool bAsync);
+ void Start();
 
  /*!
  * @brief Stop the EPG update thread.
  */
  void Stop();
 
- /*!
- * @brief Clear all EPG entries.
+ /**
+ * @brief (re)load EPG data.
+ * @return True if loaded successfully, false otherwise.
+ */
+ bool Load();
+
+ /**
+ * @brief unload all EPG data.
  */
- void Clear();
+ void Unload();
 
  /*!
  * @brief Check whether the EpgContainer has fully started.

xbmc/pvr/timers/PVRTimers.cpp

@@ -101,24 +101,33 @@ bool CPVRTimers::Load()
  // load local timers from database
  bool bReturn = LoadFromDatabase();
 
- Update(); // update from clients
-
- CServiceBroker::GetPVRManager().EpgContainer().Events().Subscribe(this, &CPVRTimers::Notify);
- Create();
+ // update from clients
+ Update();
 
  return bReturn;
 }
 
 void CPVRTimers::Unload()
 {
- StopThread();
- CServiceBroker::GetPVRManager().EpgContainer().Events().Unsubscribe(this);
-
  // remove all tags
  CSingleLock lock(m_critSection);
  m_tags.clear();
 }
 
+void CPVRTimers::Start()
+{
+ Stop();
+
+ CServiceBroker::GetPVRManager().EpgContainer().Events().Subscribe(this, &CPVRTimers::Notify);
+ Create();
+}
+
+void CPVRTimers::Stop()
+{
+ StopThread();
+ CServiceBroker::GetPVRManager().EpgContainer().Events().Unsubscribe(this);
+}
+
 bool CPVRTimers::Update()
 {
  {

xbmc/pvr/timers/PVRTimers.h

@@ -68,6 +68,16 @@ namespace PVR
  CPVRTimers();
  ~CPVRTimers() override = default;
 
+ /**
+ * @brief start the timer update thread.
+ */
+ void Start();
+
+ /**
+ * @brief stop the timer update thread.
+ */
+ void Stop();
+
  /**
  * @brief (re)load the timers from the clients.
  * @return True if loaded successfully, false otherwise.