Merge pull request #18758 from howie-f/v19-fix-iss18723

[addons] fix: don't check updated repos for updates during migration

4 files changed, 74 insertions, 24 deletions

diff --git a/xbmc/addons/AddonInstaller.cpp b/xbmc/addons/AddonInstaller.cpp
index 7fdd8c66c8..6db0160e6a 100644
--- a/xbmc/addons/AddonInstaller.cpp
+++ b/xbmc/addons/AddonInstaller.cpp
@@ -196,14 +196,15 @@ bool CAddonInstaller::InstallOrUpdate(const std::string& addonID,
   if (!CAddonInstallJob::GetAddon(addonID, repo, addon))
     return false;
 
-  return DoInstall(addon, repo, background, modal, AutoUpdateJob::NO, DependencyJob::NO);
+  return DoInstall(addon, repo, background, modal, AutoUpdateJob::NO, DependencyJob::NO,
+                   AllowCheckForUpdates::YES);
 }
 
 bool CAddonInstaller::InstallOrUpdateDependency(const ADDON::AddonPtr& dependsId,
                                                 const ADDON::RepositoryPtr& repo)
 {
   return DoInstall(dependsId, repo, BackgroundJob::NO, ModalJob::NO, AutoUpdateJob::NO,
-                   DependencyJob::YES);
+                   DependencyJob::YES, AllowCheckForUpdates::YES);
 }
 
 bool CAddonInstaller::Install(const std::string& addonId,
@@ -224,7 +225,7 @@ bool CAddonInstaller::Install(const std::string& addonId,
     return false;
 
   return DoInstall(addon, std::static_pointer_cast<CRepository>(repo), BackgroundJob::YES,
-                   ModalJob::NO, AutoUpdateJob::NO, DependencyJob::NO);
+                   ModalJob::NO, AutoUpdateJob::NO, DependencyJob::NO, AllowCheckForUpdates::YES);
 }
 
 bool CAddonInstaller::DoInstall(const AddonPtr& addon,
@@ -232,7 +233,8 @@ bool CAddonInstaller::DoInstall(const AddonPtr& addon,
                                 BackgroundJob background,
                                 ModalJob modal,
                                 AutoUpdateJob autoUpdate,
-                                DependencyJob dependsInstall)
+                                DependencyJob dependsInstall,
+                                AllowCheckForUpdates allowCheckForUpdates)
 {
   // check whether we already have the addon installing
   CSingleLock lock(m_critSection);
@@ -256,6 +258,7 @@ bool CAddonInstaller::DoInstall(const AddonPtr& addon,
   lock.Leave();
 
   installJob->SetDependsInstall(dependsInstall);
+  installJob->SetAllowCheckForUpdates(allowCheckForUpdates);
 
   bool result = false;
   if (modal == ModalJob::YES)
@@ -297,7 +300,7 @@ bool CAddonInstaller::InstallFromZip(const std::string &path)
   AddonPtr addon;
   if (CServiceBroker::GetAddonMgr().LoadAddonDescription(items[0]->GetPath(), addon))
     return DoInstall(addon, RepositoryPtr(), BackgroundJob::YES, ModalJob::NO, AutoUpdateJob::NO,
-                     DependencyJob::NO);
+                     DependencyJob::NO, AllowCheckForUpdates::YES);
 
   CServiceBroker::GetEventLog().AddWithNotification(EventPtr(new CNotificationEvent(24045,
       StringUtils::Format(g_localizeStrings.Get(24143).c_str(), path.c_str()),
@@ -440,7 +443,9 @@ void CAddonInstaller::PrunePackageCache()
   }
 }
 
-void CAddonInstaller::InstallAddons(const VECADDONS& addons, bool wait)
+void CAddonInstaller::InstallAddons(const VECADDONS& addons,
+                                    bool wait,
+                                    AllowCheckForUpdates allowCheckForUpdates)
 {
   for (const auto& addon : addons)
   {
@@ -448,7 +453,7 @@ void CAddonInstaller::InstallAddons(const VECADDONS& addons, bool wait)
     RepositoryPtr repo;
     if (CAddonInstallJob::GetAddon(addon->ID(), repo, toInstall))
       DoInstall(toInstall, repo, BackgroundJob::NO, ModalJob::NO, AutoUpdateJob::YES,
-                DependencyJob::NO);
+                DependencyJob::NO, allowCheckForUpdates);
   }
   if (wait)
   {
@@ -678,9 +683,24 @@ bool CAddonInstallJob::DoWork()
   else if (m_addon->HasMainType(ADDON_REPOSITORY))
   {
     origin = m_addon->ID(); // use own id as origin if repository
-    if (m_isUpdate)
-      CServiceBroker::GetRepositoryUpdater().CheckForUpdates(
-          std::static_pointer_cast<CRepository>(m_addon), false);
+
+    // if a repository is updated during the add-on migration process, we need to skip
+    // calling CheckForUpdates() on the repo to prevent deadlock issues during migration
+
+    if (m_allowCheckForUpdates == AllowCheckForUpdates::YES)
+    {
+      if (m_isUpdate)
+      {
+        CLog::Log(LOGDEBUG, "ADDONS: repository [{}] updated. now checking for content updates.",
+                  m_addon->ID());
+        CServiceBroker::GetRepositoryUpdater().CheckForUpdates(
+            std::static_pointer_cast<CRepository>(m_addon), false);
+      }
+    }
+    else
+    {
+      CLog::Log(LOGDEBUG, "ADDONS: skipping CheckForUpdates() on repository [{}].", m_addon->ID());
+    }
   }
   else if (m_repo)
   {
diff --git a/xbmc/addons/AddonInstaller.h b/xbmc/addons/AddonInstaller.h
index 0b7f502599..28c7e551de 100644
--- a/xbmc/addons/AddonInstaller.h
+++ b/xbmc/addons/AddonInstaller.h
@@ -56,6 +56,12 @@ enum class InstallModalPrompt
   NO_PROMPT,
 };
 
+enum class AllowCheckForUpdates
+{
+  YES,
+  NO,
+};
+
 class CAddonInstaller : public IJobCallback
 {
 public:
@@ -96,11 +102,15 @@ public:
                                  const ADDON::RepositoryPtr& repo);
 
   /*! \brief Installs a vector of addons
-   \param addons the list of addons to install
-   \param wait if the method should wait for all the DoInstall jobs to finish or if it should return right away
-   \sa DoInstall
+   *  \param addons the list of addons to install
+   *  \param wait if the method should wait for all the DoInstall jobs to finish or if it should return right away
+   *  \param allowCheckForUpdates indicates if content update checks are allowed
+   *         after installation of a repository addon from the vector
+   *  \sa DoInstall
    */
-  void InstallAddons(const ADDON::VECADDONS& addons, bool wait);
+  void InstallAddons(const ADDON::VECADDONS& addons,
+                     bool wait,
+                     AllowCheckForUpdates allowCheckForUpdates);
 
   /*! \brief Install an addon from the given zip path
    \param path the zip file to install from
@@ -162,17 +172,24 @@ private:
   ~CAddonInstaller() override;
 
   /*! \brief Install an addon from a repository or zip
-   \param addon the AddonPtr describing the addon
-   \param repo the repository to install addon from
-   \param background whether to install in the background or not.
-   \return true on successful install, false on failure.
+   *  \param addon the AddonPtr describing the addon
+   *  \param repo the repository to install addon from
+   *  \param background whether to install in the background or not.
+   *  \param modal whether to install in modal mode or not.
+   *  \param autoUpdate whether the addon is installed in auto update mode.
+   *         (i.e. no notification)
+   *  \param dependsInstall whether this is the installation of a dependency addon
+   *  \param allowCheckForUpdates whether content update check after installation of
+   *         a repository addon is allowed
+   *  \return true on successful install, false on failure.
    */
   bool DoInstall(const ADDON::AddonPtr& addon,
                  const ADDON::RepositoryPtr& repo,
                  BackgroundJob background,
                  ModalJob modal,
                  AutoUpdateJob autoUpdate,
-                 DependencyJob dependsInstall);
+                 DependencyJob dependsInstall,
+                 AllowCheckForUpdates allowCheckForUpdates);
 
   /*! \brief Check whether dependencies of an addon exist or are installable.
    Iterates through the addon's dependencies, checking they're installed or installable.
@@ -221,6 +238,10 @@ public:
   static bool GetAddon(const std::string& addonID, ADDON::RepositoryPtr& repo, ADDON::AddonPtr& addon);
 
   void SetDependsInstall(DependencyJob dependsInstall) { m_dependsInstall = dependsInstall; };
+  void SetAllowCheckForUpdates(AllowCheckForUpdates allowCheckForUpdates)
+  {
+    m_allowCheckForUpdates = allowCheckForUpdates;
+  };
 
 private:
   void OnPreInstall();
@@ -242,6 +263,7 @@ private:
   bool m_isUpdate;
   AutoUpdateJob m_isAutoUpdate;
   DependencyJob m_dependsInstall = DependencyJob::NO;
+  AllowCheckForUpdates m_allowCheckForUpdates = AllowCheckForUpdates::YES;
   const char* m_currentType = TYPE_DOWNLOAD;
 };
 
diff --git a/xbmc/addons/AddonManager.cpp b/xbmc/addons/AddonManager.cpp
index 4a68eab7c0..7606c6f95e 100644
--- a/xbmc/addons/AddonManager.cpp
+++ b/xbmc/addons/AddonManager.cpp
@@ -458,7 +458,7 @@ std::vector<AddonInfoPtr> CAddonMgr::MigrateAddons()
   CLog::Log(LOGINFO, "ADDON: waiting for add-ons to update...");
   VECADDONS updates;
   GetAddonUpdateCandidates(updates);
-  InstallAddonUpdates(updates, true);
+  InstallAddonUpdates(updates, true, AllowCheckForUpdates::NO);
 
   // get addons that became incompatible and disable them
   std::vector<AddonInfoPtr> incompatible;
@@ -497,7 +497,7 @@ void CAddonMgr::CheckAndInstallAddonUpdates(bool wait) const
   std::lock_guard<std::mutex> lock(m_installAddonsMutex);
   VECADDONS updates;
   GetAddonUpdateCandidates(updates);
-  InstallAddonUpdates(updates, wait);
+  InstallAddonUpdates(updates, wait, AllowCheckForUpdates::YES);
 }
 
 bool CAddonMgr::GetAddonUpdateCandidates(VECADDONS& updates) const
@@ -556,11 +556,13 @@ void CAddonMgr::SortByDependencies(VECADDONS& updates) const
   updates = sorted;
 }
 
-void CAddonMgr::InstallAddonUpdates(VECADDONS& updates, bool wait) const
+void CAddonMgr::InstallAddonUpdates(VECADDONS& updates,
+                                    bool wait,
+                                    AllowCheckForUpdates allowCheckForUpdates) const
 {
   // sort addons by dependencies (ensure install order) and install all
   SortByDependencies(updates);
-  CAddonInstaller::GetInstance().InstallAddons(updates, wait);
+  CAddonInstaller::GetInstance().InstallAddons(updates, wait, allowCheckForUpdates);
 }
 
 bool CAddonMgr::GetAddon(const std::string& str,
diff --git a/xbmc/addons/AddonManager.h b/xbmc/addons/AddonManager.h
index dfce76af4a..a7efdfac58 100644
--- a/xbmc/addons/AddonManager.h
+++ b/xbmc/addons/AddonManager.h
@@ -26,6 +26,8 @@ namespace ADDON
 
   const std::string ADDON_PYTHON_EXT           = "*.py";
 
+  enum class AllowCheckForUpdates;
+
   enum class AddonCheckType
   {
     OUTDATED_ADDONS,
@@ -556,8 +558,12 @@ namespace ADDON
      * Install the list of addon updates via AddonInstaller
      * \param[in,out] updates the vector of addons to install (will be sorted)
      * \param wait if the process should wait for all addons to install
+     * \param allowCheckForUpdates indicates if content update checks are allowed
+     *        after installation of a repository addon from the list
      */
-    void InstallAddonUpdates(VECADDONS& updates, bool wait) const;
+    void InstallAddonUpdates(VECADDONS& updates,
+                             bool wait,
+                             AllowCheckForUpdates allowCheckForUpdates) const;
 
     // This guards the addon installation process to make sure
     // addon updates are not installed concurrently