Merge pull request xbmc#12473 from AlwinEsch/add-bin-addon-handler

[addons] add blocked addon call event to prevent problems after disable

xbmc/addons/AddonEvents.h

@@ -71,5 +71,17 @@ namespace ADDON
      * @deprecated Use Enabled, ReInstalled and UnInstalled instead.
      */
     struct InstalledChanged : AddonEvent {};
+
+    struct Load : AddonEvent
+    {
+      std::string id;
+      explicit Load(std::string id) : id(std::move(id)) {}
+    };
+
+    struct Unload : AddonEvent
+    {
+      std::string id;
+      explicit Unload(std::string id) : id(std::move(id)) {}
+    };
   };
 };

xbmc/addons/AddonInstaller.cpp

@@ -604,11 +604,17 @@ bool CAddonInstallJob::DoWork()
   // run any pre-install functions
   ADDON::OnPreInstall(m_addon);
 
+  if (!CAddonMgr::GetInstance().UnloadAddon(m_addon))
+  {
+    CLog::Log(LOGERROR, "CAddonInstallJob[%s]: failed to unload addon.", m_addon->ID().c_str());
+    return false;
+  }
+
   // perform install
   if (!Install(installFrom, m_repo))
     return false;
 
-  if (!CAddonMgr::GetInstance().ReloadAddon(m_addon))
+  if (!CAddonMgr::GetInstance().LoadAddon(m_addon->ID()))
   {
     CLog::Log(LOGERROR, "CAddonInstallJob[%s]: failed to reload addon", m_addon->ID().c_str());
     return false;

xbmc/addons/AddonManager.cpp

@@ -706,26 +706,27 @@ bool CAddonMgr::UnloadAddon(const AddonPtr& addon)
     if (m_cpluff->uninstall_plugin(m_cp_context, addon->ID().c_str()) == CP_OK)
     {
       CLog::Log(LOGDEBUG, "CAddonMgr: %s unloaded", addon->ID().c_str());
-      m_events.Publish(AddonEvents::InstalledChanged());
-      m_events.Publish(AddonEvents::UnInstalled(addon->ID()));
+
+      lock.Leave();
+      AddonEvents::Unload event(addon->ID());
+      m_unloadEvents.HandleEvent(event);
       return true;
     }
   }
   CLog::Log(LOGERROR, "CAddonMgr: failed to unload %s", addon->ID().c_str());
   return false;
 }
 
-bool CAddonMgr::ReloadAddon(AddonPtr& addon)
+bool CAddonMgr::LoadAddon(const std::string& addonId)
 {
   CSingleLock lock(m_critSection);
-  if (!addon ||!m_cpluff || !m_cp_context)
+  if (!m_cpluff || !m_cp_context)
     return false;
 
-  AddonPtr tmp;
-  bool isReinstall = GetAddon(addon->ID(), tmp, ADDON_UNKNOWN, false);
-  if (isReinstall)
+  AddonPtr addon;
+  if (GetAddon(addonId, addon, ADDON_UNKNOWN, false))
   {
-    m_cpluff->uninstall_plugin(m_cp_context, addon->ID().c_str());
+    return true;
   }
 
   if (!FindAddons())
@@ -734,22 +735,26 @@ bool CAddonMgr::ReloadAddon(AddonPtr& addon)
     return false;
   }
 
-  if (!GetAddon(addon->ID(), addon, ADDON_UNKNOWN, false))
+  if (!GetAddon(addonId, addon, ADDON_UNKNOWN, false))
   {
-    CLog::Log(LOGERROR, "CAddonMgr: could not reload add-on %s. No add-on with that ID is installed.", addon->ID().c_str());
+    CLog::Log(LOGERROR, "CAddonMgr: could not load add-on %s. No add-on with that ID is installed.", addon->ID().c_str());
     return false;
   }
 
-  if (isReinstall)
-    m_events.Publish(AddonEvents::ReInstalled(addon->ID()));
+  lock.Leave();
 
-  if (!EnableAddon(addon->ID()))
+  AddonEvents::Load event(addon->ID());
+  m_unloadEvents.HandleEvent(event);
+
+  if (IsAddonDisabled(addon->ID()))
   {
-    CLog::Log(LOGERROR, "CAddonMgr: '%s' was installed but could not be enabled", addon->ID().c_str());
-    return false;
+    EnableAddon(addon->ID());
+    return true;
   }
 
-  CLog::Log(LOGDEBUG, "CAddonMgr: %s reloaded", addon->ID().c_str());
+  m_events.Publish(AddonEvents::ReInstalled(addon->ID()));
+  m_events.Publish(AddonEvents::InstalledChanged());
+  CLog::Log(LOGDEBUG, "CAddonMgr: %s successfully loaded", addon->ID().c_str());
   return true;
 }
 
@@ -758,6 +763,8 @@ void CAddonMgr::OnPostUnInstall(const std::string& id)
   CSingleLock lock(m_critSection);
   m_disabled.erase(id);
   m_updateBlacklist.erase(id);
+  m_events.Publish(AddonEvents::InstalledChanged());
+  m_events.Publish(AddonEvents::UnInstalled(id));
 }
 
 bool CAddonMgr::RemoveFromUpdateBlacklist(const std::string& id)

xbmc/addons/AddonManager.h

@@ -82,6 +82,7 @@ namespace ADDON
     virtual ~CAddonMgr();
 
     CEventStream<AddonEvent>& Events() { return m_events; }
+    CEventStream<AddonEvent>& UnloadEvents() { return m_unloadEvents; }
 
     IAddonMgrCallback* GetCallbackForType(TYPE type);
     bool RegisterAddonMgrCallback(TYPE type, IAddonMgrCallback* cb);
@@ -178,7 +179,7 @@ namespace ADDON
      *
      * Returns true if the addon was successfully loaded and enabled; otherwise false.
      */
-    bool ReloadAddon(AddonPtr& addon);
+    bool LoadAddon(const std::string& addonId);
 
     /*! @note: should only be called by AddonInstaller
      *
@@ -312,6 +313,7 @@ namespace ADDON
     CCriticalSection m_critSection;
     CAddonDatabase m_database;
     CEventSource<AddonEvent> m_events;
+    CBlockingEventSource<AddonEvent> m_unloadEvents;
     std::set<std::string> m_systemAddons;
     std::set<std::string> m_optionalAddons;
   };

xbmc/addons/Service.cpp

@@ -50,6 +50,7 @@ CServiceAddonManager::CServiceAddonManager(CAddonMgr& addonMgr) :
 CServiceAddonManager::~CServiceAddonManager()
 {
   m_addonMgr.Events().Unsubscribe(this);
+  m_addonMgr.UnloadEvents().Unsubscribe(this);
 }
 
 void CServiceAddonManager::OnEvent(const ADDON::AddonEvent& event)
@@ -68,9 +69,26 @@ void CServiceAddonManager::OnEvent(const ADDON::AddonEvent& event)
   }
   else if (auto reinstallEvent = dynamic_cast<const AddonEvents::ReInstalled*>(&event))
   {
-    Stop(reinstallEvent->id);
     Start(reinstallEvent->id);
   }
+
+  else if (auto e = dynamic_cast<const ADDON::AddonEvents::Unload*>(&event))
+  {
+    CLog::Log(LOGINFO, "CServiceAddonManager: Unloading %s", e->id.c_str());
+    CSingleLock lock(m_criticalSection);
+    Stop(e->id);
+    m_blacklistedAddons.push_back(e->id);
+  }
+  else if (auto e = dynamic_cast<const ADDON::AddonEvents::Load*>(&event))
+  {
+    CLog::Log(LOGINFO, "CServiceAddonManager: Removing %s from blacklist", e->id.c_str());
+    CSingleLock lock(m_criticalSection);
+    auto it = std::find(m_blacklistedAddons.begin(), m_blacklistedAddons.end(), e->id);
+    if (it != m_blacklistedAddons.end())
+    {
+      m_blacklistedAddons.erase(it);
+    }
+  }
 }
 
 void CServiceAddonManager::StartBeforeLogin()
@@ -92,6 +110,7 @@ void CServiceAddonManager::StartBeforeLogin()
 void CServiceAddonManager::Start()
 {
   m_addonMgr.Events().Subscribe(this, &CServiceAddonManager::OnEvent);
+  m_addonMgr.UnloadEvents().Subscribe(this, &CServiceAddonManager::OnEvent);
   VECADDONS addons;
   if (m_addonMgr.GetAddons(addons, ADDON_SERVICE))
   {
@@ -120,6 +139,12 @@ void CServiceAddonManager::Start(const AddonPtr& addon)
     return;
   }
 
+  if (std::find(m_blacklistedAddons.begin(), m_blacklistedAddons.end(), addon->ID()) != m_blacklistedAddons.end())
+  {
+    CLog::Log(LOGINFO, "CServiceAddonManager: Not executing blacklisted addon %s", addon->ID().c_str());
+    return;
+  }
+
   if (StringUtils::EndsWith(addon->LibPath(), ".py"))
   {
     CLog::Log(LOGDEBUG, "CServiceAddonManager: starting %s", addon->ID().c_str());

xbmc/addons/Service.h

@@ -89,5 +89,7 @@ namespace ADDON
     CCriticalSection m_criticalSection;
     /** add-on id -> script id */
     std::map<std::string, int> m_services;
+
+    std::vector<std::string> m_blacklistedAddons;
   };
 }

xbmc/utils/EventStream.h

@@ -88,3 +88,18 @@ class CEventSource : public CEventStream<Event>
     CJobManager::GetInstance().Submit(std::move(task));
   }
 };
+
+template<typename Event>
+class CBlockingEventSource : public CEventStream<Event>
+{
+public:
+  template<typename A>
+  void HandleEvent(A event)
+  {
+    CSingleLock lock(this->m_criticalSection);
+    for (const auto& subscription : this->m_subscriptions)
+    {
+      subscription->HandleEvent(event);
+    }
+  }
+};