CExclusiveLock: replace with std::unique_lock

xbmc/guilib/LocalizeStrings.cpp

@@ -18,6 +18,7 @@
 #include "utils/URIUtils.h"
 #include "utils/log.h"
 
+#include <mutex>
 #include <shared_mutex>
 
 /*! \brief Tries to load ids and strings from a strings.po file to the `strings` map.
@@ -141,14 +142,14 @@ CLocalizeStrings::~CLocalizeStrings(void) = default;
 void CLocalizeStrings::ClearSkinStrings()
 {
   // clear the skin strings
-  CExclusiveLock lock(m_stringsMutex);
+  std::unique_lock<CSharedSection> lock(m_stringsMutex);
   Clear(31000, 31999);
 }
 
 bool CLocalizeStrings::LoadSkinStrings(const std::string& path, const std::string& language)
 {
   //! @todo shouldn't hold lock while loading file
-  CExclusiveLock lock(m_stringsMutex);
+  std::unique_lock<CSharedSection> lock(m_stringsMutex);
   ClearSkinStrings();
   // load the skin strings in.
   return LoadWithFallback(path, language, m_strings);
@@ -184,7 +185,7 @@ bool CLocalizeStrings::Load(const std::string& strPathName, const std::string& s
   strings[20210].strTranslated = "yard/s";
   strings[20211].strTranslated = "Furlong/Fortnight";
 
-  CExclusiveLock lock(m_stringsMutex);
+  std::unique_lock<CSharedSection> lock(m_stringsMutex);
   Clear();
   m_strings = std::move(strings);
   return true;
@@ -203,13 +204,13 @@ const std::string& CLocalizeStrings::Get(uint32_t dwCode) const
 
 void CLocalizeStrings::Clear()
 {
-  CExclusiveLock lock(m_stringsMutex);
+  std::unique_lock<CSharedSection> lock(m_stringsMutex);
   m_strings.clear();
 }
 
 void CLocalizeStrings::Clear(uint32_t start, uint32_t end)
 {
-  CExclusiveLock lock(m_stringsMutex);
+  std::unique_lock<CSharedSection> lock(m_stringsMutex);
   iStrings it = m_strings.begin();
   while (it != m_strings.end())
   {
@@ -226,7 +227,7 @@ bool CLocalizeStrings::LoadAddonStrings(const std::string& path, const std::stri
   if (!LoadWithFallback(path, language, strings))
     return false;
 
-  CExclusiveLock lock(m_addonStringsMutex);
+  std::unique_lock<CSharedSection> lock(m_addonStringsMutex);
   auto it = m_addonStrings.find(addonId);
   if (it != m_addonStrings.end())
     m_addonStrings.erase(it);

xbmc/peripherals/addons/PeripheralAddon.cpp

@@ -101,7 +101,7 @@ void CPeripheralAddon::ResetProperties(void)
 
 bool CPeripheralAddon::CreateAddon(void)
 {
-  CExclusiveLock lock(m_dllSection);
+  std::unique_lock<CSharedSection> lock(m_dllSection);
 
   // Reset all properties to defaults
   ResetProperties();
@@ -141,7 +141,7 @@ void CPeripheralAddon::DestroyAddon()
   }
 
   {
-    CExclusiveLock lock(m_dllSection);
+    std::unique_lock<CSharedSection> lock(m_dllSection);
     DestroyInstance();
   }
 }

xbmc/settings/SettingAddon.cpp

@@ -14,6 +14,8 @@
 #include "utils/XMLUtils.h"
 #include "utils/log.h"
 
+#include <mutex>
+
 CSettingAddon::CSettingAddon(const std::string &id, CSettingsManager *settingsManager /* = nullptr */)
   : CSettingString(id, settingsManager)
 { }
@@ -35,7 +37,7 @@ SettingPtr CSettingAddon::Clone(const std::string &id) const
 
 bool CSettingAddon::Deserialize(const TiXmlNode *node, bool update /* = false */)
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
 
   if (!CSettingString::Deserialize(node, update))
     return false;
@@ -75,6 +77,6 @@ void CSettingAddon::copyaddontype(const CSettingAddon &setting)
 {
   CSettingString::Copy(setting);
 
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
   m_addonType = setting.m_addonType;
 }

xbmc/settings/SettingPath.cpp

@@ -15,6 +15,8 @@
 #include "utils/XMLUtils.h"
 #include "utils/log.h"
 
+#include <mutex>
+
 #define XML_ELM_DEFAULT     "default"
 #define XML_ELM_CONSTRAINTS "constraints"
 
@@ -39,7 +41,7 @@ SettingPtr CSettingPath::Clone(const std::string &id) const
 
 bool CSettingPath::Deserialize(const TiXmlNode *node, bool update /* = false */)
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
 
   if (!CSettingString::Deserialize(node, update))
     return false;
@@ -135,7 +137,7 @@ void CSettingPath::copy(const CSettingPath& setting)
 {
   CSettingString::Copy(setting);
 
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
   m_writable = setting.m_writable;
   m_sources = setting.m_sources;
   m_hideExtension = setting.m_hideExtension;

xbmc/settings/lib/Setting.cpp

@@ -16,6 +16,7 @@
 #include "utils/XMLUtils.h"
 #include "utils/log.h"
 
+#include <mutex>
 #include <shared_mutex>
 #include <sstream>
 #include <utility>
@@ -395,7 +396,7 @@ void CSettingList::MergeDetails(const CSetting& other)
 
 bool CSettingList::Deserialize(const TiXmlNode *node, bool update /* = false */)
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
 
   if (m_definition == nullptr)
     return false;
@@ -503,7 +504,7 @@ bool CSettingList::CheckValidity(const std::string &value) const
 
 void CSettingList::Reset()
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
   SettingList values;
   for (const auto& it : m_defaults)
     values.push_back(it->Clone(it->GetId()));
@@ -522,7 +523,7 @@ bool CSettingList::FromString(const std::vector<std::string> &value)
 
 bool CSettingList::SetValue(const SettingList &values)
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
 
   if ((int)values.size() < m_minimumItems ||
      (m_maximumItems > 0 && (int)values.size() > m_maximumItems))
@@ -565,7 +566,7 @@ bool CSettingList::SetValue(const SettingList &values)
 
 void CSettingList::SetDefault(const SettingList &values)
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
 
   m_defaults.clear();
   m_defaults.insert(m_defaults.begin(), values.begin(), values.end());
@@ -702,7 +703,7 @@ void CSettingBool::MergeDetails(const CSetting& other)
 
 bool CSettingBool::Deserialize(const TiXmlNode *node, bool update /* = false */)
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
 
   if (!CSetting::Deserialize(node, update))
     return false;
@@ -748,7 +749,7 @@ bool CSettingBool::CheckValidity(const std::string &value) const
 
 bool CSettingBool::SetValue(bool value)
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
 
   if (value == m_value)
     return true;
@@ -775,7 +776,7 @@ bool CSettingBool::SetValue(bool value)
 
 void CSettingBool::SetDefault(bool value)
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
 
   m_default = value;
   if (!m_changed)
@@ -897,7 +898,7 @@ void CSettingInt::MergeDetails(const CSetting& other)
 
 bool CSettingInt::Deserialize(const TiXmlNode *node, bool update /* = false */)
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
 
   if (!CSetting::Deserialize(node, update))
     return false;
@@ -1025,7 +1026,7 @@ bool CSettingInt::CheckValidity(int value) const
 
 bool CSettingInt::SetValue(int value)
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
 
   if (value == m_value)
     return true;
@@ -1055,7 +1056,7 @@ bool CSettingInt::SetValue(int value)
 
 void CSettingInt::SetDefault(int value)
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
 
   m_default = value;
   if (!m_changed)
@@ -1077,7 +1078,7 @@ SettingOptionsType CSettingInt::GetOptionsType() const
 
 IntegerSettingOptions CSettingInt::UpdateDynamicOptions()
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
   IntegerSettingOptions options;
   if (m_optionsFiller == nullptr &&
      (m_optionsFillerName.empty() || m_settingsManager == nullptr))
@@ -1126,7 +1127,7 @@ void CSettingInt::copy(const CSettingInt &setting)
 {
   CSetting::Copy(setting);
 
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
 
   m_value = setting.m_value;
   m_default = setting.m_default;
@@ -1220,7 +1221,7 @@ void CSettingNumber::MergeDetails(const CSetting& other)
 
 bool CSettingNumber::Deserialize(const TiXmlNode *node, bool update /* = false */)
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
 
   if (!CSetting::Deserialize(node, update))
     return false;
@@ -1294,7 +1295,7 @@ bool CSettingNumber::CheckValidity(double value) const
 
 bool CSettingNumber::SetValue(double value)
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
 
   if (value == m_value)
     return true;
@@ -1324,7 +1325,7 @@ bool CSettingNumber::SetValue(double value)
 
 void CSettingNumber::SetDefault(double value)
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
 
   m_default = value;
   if (!m_changed)
@@ -1334,7 +1335,7 @@ void CSettingNumber::SetDefault(double value)
 void CSettingNumber::copy(const CSettingNumber &setting)
 {
   CSetting::Copy(setting);
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
 
   m_value = setting.m_value;
   m_default = setting.m_default;
@@ -1420,7 +1421,7 @@ void CSettingString::MergeDetails(const CSetting& other)
 
 bool CSettingString::Deserialize(const TiXmlNode *node, bool update /* = false */)
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
 
   if (!CSetting::Deserialize(node, update))
     return false;
@@ -1514,7 +1515,7 @@ bool CSettingString::CheckValidity(const std::string &value) const
 
 bool CSettingString::SetValue(const std::string &value)
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
 
   if (value == m_value)
     return true;
@@ -1566,7 +1567,7 @@ SettingOptionsType CSettingString::GetOptionsType() const
 
 StringSettingOptions CSettingString::UpdateDynamicOptions()
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
   StringSettingOptions options;
   if (m_optionsFiller == nullptr &&
      (m_optionsFillerName.empty() || m_settingsManager == nullptr))
@@ -1616,7 +1617,7 @@ void CSettingString::copy(const CSettingString &setting)
 {
   CSetting::Copy(setting);
 
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
   m_value = setting.m_value;
   m_default = setting.m_default;
   m_allowEmpty = setting.m_allowEmpty;
@@ -1684,6 +1685,6 @@ void CSettingAction::copy(const CSettingAction& setting)
 {
   CSetting::Copy(setting);
 
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
   m_data = setting.m_data;
 }

xbmc/settings/lib/SettingsManager.cpp

@@ -18,6 +18,7 @@
 
 #include <algorithm>
 #include <map>
+#include <mutex>
 #include <shared_mutex>
 #include <unordered_set>
 #include <utility>
@@ -79,8 +80,8 @@ uint32_t CSettingsManager::ParseVersion(const TiXmlElement* root) const
 
 bool CSettingsManager::Initialize(const TiXmlElement *root)
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
-  CExclusiveLock settingsLock(m_settingsCritical);
+  std::unique_lock<CSharedSection> settingsLock(m_settingsCritical);
   if (m_initialized || root == nullptr)
     return false;
 
@@ -140,7 +141,7 @@ bool CSettingsManager::Initialize(const TiXmlElement *root)
 bool CSettingsManager::Load(const TiXmlElement *root, bool &updated, bool triggerEvents /* = true */, std::map<std::string, SettingPtr> *loadedSettings /* = nullptr */)
 {
   std::shared_lock<CSharedSection> lock(m_critical);
-  CExclusiveLock settingsLock(m_settingsCritical);
+  std::unique_lock<CSharedSection> settingsLock(m_settingsCritical);
   if (m_loaded || root == nullptr)
     return false;
 
@@ -197,7 +198,7 @@ bool CSettingsManager::Save(
 
 void CSettingsManager::Unload()
 {
-  CExclusiveLock lock(m_settingsCritical);
+  std::unique_lock<CSharedSection> lock(m_settingsCritical);
   if (!m_loaded)
     return;
 
@@ -213,7 +214,7 @@ void CSettingsManager::Unload()
 
 void CSettingsManager::Clear()
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
   Unload();
 
   m_settings.clear();
@@ -246,7 +247,7 @@ bool CSettingsManager::LoadSetting(const TiXmlNode *node, const std::string &set
 
 void CSettingsManager::SetInitialized()
 {
-  CExclusiveLock lock(m_settingsCritical);
+  std::unique_lock<CSharedSection> lock(m_settingsCritical);
   if (m_initialized)
     return;
 
@@ -269,8 +270,8 @@ void CSettingsManager::AddSection(const SettingSectionPtr& section)
   if (section == nullptr)
     return;
 
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
-  CExclusiveLock settingsLock(m_settingsCritical);
+  std::unique_lock<CSharedSection> settingsLock(m_settingsCritical);
 
   section->CheckRequirements();
   m_sections[section->GetId()] = section;
@@ -314,8 +315,8 @@ bool CSettingsManager::AddSetting(const std::shared_ptr<CSetting>& setting,
   if (setting == nullptr || section == nullptr || category == nullptr || group == nullptr)
     return false;
 
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
-  CExclusiveLock settingsLock(m_settingsCritical);
+  std::unique_lock<CSharedSection> settingsLock(m_settingsCritical);
 
   // check if a setting with the given ID already exists
   if (FindSetting(setting->GetId()) != m_settings.end())
@@ -364,7 +365,7 @@ bool CSettingsManager::AddSetting(const std::shared_ptr<CSetting>& setting,
 
 void CSettingsManager::RegisterCallback(ISettingCallback *callback, const std::set<std::string> &settingList)
 {
-  CExclusiveLock lock(m_settingsCritical);
+  std::unique_lock<CSharedSection> lock(m_settingsCritical);
   if (callback == nullptr)
     return;
 
@@ -387,14 +388,14 @@ void CSettingsManager::RegisterCallback(ISettingCallback *callback, const std::s
 
 void CSettingsManager::UnregisterCallback(ISettingCallback *callback)
 {
-  CExclusiveLock lock(m_settingsCritical);
+  std::unique_lock<CSharedSection> lock(m_settingsCritical);
   for (auto& setting : m_settings)
     setting.second.callbacks.erase(callback);
 }
 
 void CSettingsManager::RegisterSettingType(const std::string &settingType, ISettingCreator *settingCreator)
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
   if (settingType.empty() || settingCreator == nullptr)
     return;
 
@@ -408,7 +409,7 @@ void CSettingsManager::RegisterSettingControl(const std::string &controlType, IS
   if (controlType.empty() || settingControlCreator == nullptr)
     return;
 
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
   auto creatorIt = m_settingControlCreators.find(controlType);
   if (creatorIt == m_settingControlCreators.end())
     m_settingControlCreators.insert(std::make_pair(controlType, settingControlCreator));
@@ -419,7 +420,7 @@ void CSettingsManager::RegisterSettingsHandler(ISettingsHandler *settingsHandler
   if (settingsHandler == nullptr)
     return;
 
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
   if (find(m_settingsHandlers.begin(), m_settingsHandlers.end(), settingsHandler) == m_settingsHandlers.end())
   {
     if (bFront)
@@ -434,7 +435,7 @@ void CSettingsManager::UnregisterSettingsHandler(ISettingsHandler *settingsHandl
   if (settingsHandler == nullptr)
     return;
 
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
   auto it = std::find(m_settingsHandlers.begin(), m_settingsHandlers.end(), settingsHandler);
   if (it != m_settingsHandlers.end())
     m_settingsHandlers.erase(it);
@@ -458,7 +459,7 @@ void CSettingsManager::RegisterSettingOptionsFiller(const std::string &identifie
 
 void CSettingsManager::UnregisterSettingOptionsFiller(const std::string &identifier)
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
   m_optionsFillers.erase(identifier);
 }
 
@@ -709,7 +710,7 @@ void CSettingsManager::SetDefaults()
 
 void CSettingsManager::AddCondition(const std::string &condition)
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
   if (condition.empty())
     return;
 
@@ -718,7 +719,7 @@ void CSettingsManager::AddCondition(const std::string &condition)
 
 void CSettingsManager::AddDynamicCondition(const std::string &identifier, SettingConditionCheck condition, void *data /*= nullptr*/)
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
   if (identifier.empty() || condition == nullptr)
     return;
 
@@ -727,7 +728,7 @@ void CSettingsManager::AddDynamicCondition(const std::string &identifier, Settin
 
 void CSettingsManager::RemoveDynamicCondition(const std::string &identifier)
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
   if (identifier.empty())
     return;
 
@@ -1345,7 +1346,7 @@ void CSettingsManager::CleanupIncompleteSettings()
 
 void CSettingsManager::RegisterSettingOptionsFiller(const std::string &identifier, void *filler, SettingOptionsFillerType type)
 {
-  CExclusiveLock lock(m_critical);
+  std::unique_lock<CSharedSection> lock(m_critical);
   auto it = m_optionsFillers.find(identifier);
   if (it != m_optionsFillers.end())
     return;

xbmc/threads/SharedSection.h

@@ -52,16 +52,3 @@ public:
     }
   }
 };
-
-class CExclusiveLock : public std::unique_lock<CSharedSection>
-{
-public:
-  inline explicit CExclusiveLock(CSharedSection& cs) : std::unique_lock<CSharedSection>(cs) {}
-
-  inline bool IsOwner() const { return owns_lock(); }
-
-private:
-  CExclusiveLock(const CExclusiveLock&) = delete;
-  CExclusiveLock& operator=(const CExclusiveLock&) = delete;
-};
-

xbmc/threads/test/TestSharedSection.cpp

@@ -75,7 +75,7 @@ TEST(TestSharedSection, GetSharedLockWhileTryingExclusiveLock)
 
   std::shared_lock<CSharedSection> l1(sec); // get a shared lock
 
-  locker<CExclusiveLock> l2(sec,&mutex);
+  locker<std::unique_lock<CSharedSection>> l2(sec, &mutex);
   thread waitThread1(l2); // try to get an exclusive lock
 
   EXPECT_TRUE(waitForThread(mutex, 1, 10000ms));
@@ -133,7 +133,7 @@ TEST(TestSharedSection, TwoCase)
 
   locker<std::shared_lock<CSharedSection>> l2(sec, &mutex, &event);
   {
-    CExclusiveLock lock(sec); // get exclusive lock
+    std::unique_lock<CSharedSection> lock(sec); // get exclusive lock
     thread waitThread2(l2); // thread should block
 
     EXPECT_TRUE(waitForThread(mutex, 1, 10000ms));
@@ -181,7 +181,7 @@ TEST(TestMultipleSharedSection, General)
   locker<std::shared_lock<CSharedSection>> l4(sec, &mutex, &event);
   locker<std::shared_lock<CSharedSection>> l5(sec, &mutex, &event);
   {
-    CExclusiveLock lock(sec);
+    std::unique_lock<CSharedSection> lock(sec);
     thread waitThread1(l2);
     thread waitThread2(l3);
     thread waitThread3(l4);