From 591564566c91162a8225f360c9b52bf03840acc7 Mon Sep 17 00:00:00 2001 From: Marc Zinnschlag Date: Tue, 15 Dec 2015 12:19:48 +0100 Subject: [PATCH] made user settings access thread-safe --- apps/opencs/model/prefs/boolsetting.cpp | 11 ++++++++--- apps/opencs/model/prefs/boolsetting.hpp | 2 +- apps/opencs/model/prefs/coloursetting.cpp | 13 ++++++++----- apps/opencs/model/prefs/coloursetting.hpp | 3 ++- apps/opencs/model/prefs/doublesetting.cpp | 12 +++++++++--- apps/opencs/model/prefs/doublesetting.hpp | 3 ++- apps/opencs/model/prefs/enumsetting.cpp | 11 ++++++++--- apps/opencs/model/prefs/enumsetting.hpp | 3 ++- apps/opencs/model/prefs/intsetting.cpp | 11 ++++++++--- apps/opencs/model/prefs/intsetting.hpp | 2 +- apps/opencs/model/prefs/setting.cpp | 16 ++++++++++++++-- apps/opencs/model/prefs/setting.hpp | 6 +++++- apps/opencs/model/prefs/state.cpp | 17 +++++++++++------ apps/opencs/model/prefs/state.hpp | 6 ++++++ 14 files changed, 85 insertions(+), 31 deletions(-) diff --git a/apps/opencs/model/prefs/boolsetting.cpp b/apps/opencs/model/prefs/boolsetting.cpp index 459993325..6c0babaf7 100644 --- a/apps/opencs/model/prefs/boolsetting.cpp +++ b/apps/opencs/model/prefs/boolsetting.cpp @@ -2,6 +2,7 @@ #include "boolsetting.hpp" #include +#include #include @@ -9,8 +10,8 @@ #include "state.hpp" CSMPrefs::BoolSetting::BoolSetting (Category *parent, Settings::Manager *values, - const std::string& key, const std::string& label, bool default_) -: Setting (parent, values, key, label), mDefault (default_) + QMutex *mutex, const std::string& key, const std::string& label, bool default_) +: Setting (parent, values, mutex, key, label), mDefault (default_) {} CSMPrefs::BoolSetting& CSMPrefs::BoolSetting::setTooltip (const std::string& tooltip) @@ -37,6 +38,10 @@ std::pair CSMPrefs::BoolSetting::makeWidgets (QWidget *par void CSMPrefs::BoolSetting::valueChanged (int value) { - getValues().setBool (getKey(), getParent()->getKey(), value); + { + QMutexLocker lock (getMutex()); + getValues().setBool (getKey(), getParent()->getKey(), value); + } + getParent()->getState()->update (*this); } diff --git a/apps/opencs/model/prefs/boolsetting.hpp b/apps/opencs/model/prefs/boolsetting.hpp index dfc28c5ae..f8a321859 100644 --- a/apps/opencs/model/prefs/boolsetting.hpp +++ b/apps/opencs/model/prefs/boolsetting.hpp @@ -15,7 +15,7 @@ namespace CSMPrefs public: BoolSetting (Category *parent, Settings::Manager *values, - const std::string& key, const std::string& label, bool default_); + QMutex *mutex, const std::string& key, const std::string& label, bool default_); BoolSetting& setTooltip (const std::string& tooltip); diff --git a/apps/opencs/model/prefs/coloursetting.cpp b/apps/opencs/model/prefs/coloursetting.cpp index a56485292..d51bfad56 100644 --- a/apps/opencs/model/prefs/coloursetting.cpp +++ b/apps/opencs/model/prefs/coloursetting.cpp @@ -1,9 +1,8 @@ #include "coloursetting.hpp" -#include - #include +#include #include @@ -13,8 +12,8 @@ #include "state.hpp" CSMPrefs::ColourSetting::ColourSetting (Category *parent, Settings::Manager *values, - const std::string& key, const std::string& label, QColor default_) -: Setting (parent, values, key, label), mDefault (default_) + QMutex *mutex, const std::string& key, const std::string& label, QColor default_) +: Setting (parent, values, mutex, key, label), mDefault (default_) {} CSMPrefs::ColourSetting& CSMPrefs::ColourSetting::setTooltip (const std::string& tooltip) @@ -44,6 +43,10 @@ std::pair CSMPrefs::ColourSetting::makeWidgets (QWidget *p void CSMPrefs::ColourSetting::valueChanged() { CSVWidget::ColorEditor& widget = dynamic_cast (*sender()); - getValues().setString (getKey(), getParent()->getKey(), widget.color().name().toUtf8().data()); + { + QMutexLocker lock (getMutex()); + getValues().setString (getKey(), getParent()->getKey(), widget.color().name().toUtf8().data()); + } + getParent()->getState()->update (*this); } diff --git a/apps/opencs/model/prefs/coloursetting.hpp b/apps/opencs/model/prefs/coloursetting.hpp index fed2adc0a..be58426f2 100644 --- a/apps/opencs/model/prefs/coloursetting.hpp +++ b/apps/opencs/model/prefs/coloursetting.hpp @@ -17,7 +17,8 @@ namespace CSMPrefs public: ColourSetting (Category *parent, Settings::Manager *values, - const std::string& key, const std::string& label, QColor default_); + QMutex *mutex, const std::string& key, const std::string& label, + QColor default_); ColourSetting& setTooltip (const std::string& tooltip); diff --git a/apps/opencs/model/prefs/doublesetting.cpp b/apps/opencs/model/prefs/doublesetting.cpp index 2eabc78bf..7c247777d 100644 --- a/apps/opencs/model/prefs/doublesetting.cpp +++ b/apps/opencs/model/prefs/doublesetting.cpp @@ -5,6 +5,7 @@ #include #include +#include #include @@ -12,8 +13,9 @@ #include "state.hpp" CSMPrefs::DoubleSetting::DoubleSetting (Category *parent, Settings::Manager *values, - const std::string& key, const std::string& label, double default_) -: Setting (parent, values, key, label), mMin (0), mMax (std::numeric_limits::max()), + QMutex *mutex, const std::string& key, const std::string& label, double default_) +: Setting (parent, values, mutex, key, label), + mMin (0), mMax (std::numeric_limits::max()), mDefault (default_) {} @@ -64,6 +66,10 @@ std::pair CSMPrefs::DoubleSetting::makeWidgets (QWidget *p void CSMPrefs::DoubleSetting::valueChanged (double value) { - getValues().setFloat (getKey(), getParent()->getKey(), value); + { + QMutexLocker lock (getMutex()); + getValues().setFloat (getKey(), getParent()->getKey(), value); + } + getParent()->getState()->update (*this); } diff --git a/apps/opencs/model/prefs/doublesetting.hpp b/apps/opencs/model/prefs/doublesetting.hpp index d0735d30a..3868f014e 100644 --- a/apps/opencs/model/prefs/doublesetting.hpp +++ b/apps/opencs/model/prefs/doublesetting.hpp @@ -17,7 +17,8 @@ namespace CSMPrefs public: DoubleSetting (Category *parent, Settings::Manager *values, - const std::string& key, const std::string& label, double default_); + QMutex *mutex, const std::string& key, const std::string& label, + double default_); // defaults to [0, std::numeric_limits::max()] DoubleSetting& setRange (double min, double max); diff --git a/apps/opencs/model/prefs/enumsetting.cpp b/apps/opencs/model/prefs/enumsetting.cpp index ea7d0703e..e69f717ea 100644 --- a/apps/opencs/model/prefs/enumsetting.cpp +++ b/apps/opencs/model/prefs/enumsetting.cpp @@ -3,6 +3,7 @@ #include #include +#include #include @@ -39,8 +40,8 @@ CSMPrefs::EnumValues& CSMPrefs::EnumValues::add (const std::string& value, const CSMPrefs::EnumSetting::EnumSetting (Category *parent, Settings::Manager *values, - const std::string& key, const std::string& label, const EnumValue& default_) -: Setting (parent, values, key, label), mDefault (default_) + QMutex *mutex, const std::string& key, const std::string& label, const EnumValue& default_) +: Setting (parent, values, mutex, key, label), mDefault (default_) {} CSMPrefs::EnumSetting& CSMPrefs::EnumSetting::setTooltip (const std::string& tooltip) @@ -102,6 +103,10 @@ std::pair CSMPrefs::EnumSetting::makeWidgets (QWidget *par void CSMPrefs::EnumSetting::valueChanged (int value) { - getValues().setString (getKey(), getParent()->getKey(), mValues.mValues.at (value).mValue); + { + QMutexLocker lock (getMutex()); + getValues().setString (getKey(), getParent()->getKey(), mValues.mValues.at (value).mValue); + } + getParent()->getState()->update (*this); } diff --git a/apps/opencs/model/prefs/enumsetting.hpp b/apps/opencs/model/prefs/enumsetting.hpp index e2102d20e..728de3acd 100644 --- a/apps/opencs/model/prefs/enumsetting.hpp +++ b/apps/opencs/model/prefs/enumsetting.hpp @@ -39,7 +39,8 @@ namespace CSMPrefs public: EnumSetting (Category *parent, Settings::Manager *values, - const std::string& key, const std::string& label, const EnumValue& default_); + QMutex *mutex, const std::string& key, const std::string& label, + const EnumValue& default_); EnumSetting& setTooltip (const std::string& tooltip); diff --git a/apps/opencs/model/prefs/intsetting.cpp b/apps/opencs/model/prefs/intsetting.cpp index 83fb495c5..269a63af4 100644 --- a/apps/opencs/model/prefs/intsetting.cpp +++ b/apps/opencs/model/prefs/intsetting.cpp @@ -5,6 +5,7 @@ #include #include +#include #include @@ -12,8 +13,8 @@ #include "state.hpp" CSMPrefs::IntSetting::IntSetting (Category *parent, Settings::Manager *values, - const std::string& key, const std::string& label, int default_) -: Setting (parent, values, key, label), mMin (0), mMax (std::numeric_limits::max()), + QMutex *mutex, const std::string& key, const std::string& label, int default_) +: Setting (parent, values, mutex, key, label), mMin (0), mMax (std::numeric_limits::max()), mDefault (default_) {} @@ -64,6 +65,10 @@ std::pair CSMPrefs::IntSetting::makeWidgets (QWidget *pare void CSMPrefs::IntSetting::valueChanged (int value) { - getValues().setInt (getKey(), getParent()->getKey(), value); + { + QMutexLocker lock (getMutex()); + getValues().setInt (getKey(), getParent()->getKey(), value); + } + getParent()->getState()->update (*this); } diff --git a/apps/opencs/model/prefs/intsetting.hpp b/apps/opencs/model/prefs/intsetting.hpp index 05acb9fbc..0ee6cf9e3 100644 --- a/apps/opencs/model/prefs/intsetting.hpp +++ b/apps/opencs/model/prefs/intsetting.hpp @@ -17,7 +17,7 @@ namespace CSMPrefs public: IntSetting (Category *parent, Settings::Manager *values, - const std::string& key, const std::string& label, int default_); + QMutex *mutex, const std::string& key, const std::string& label, int default_); // defaults to [0, std::numeric_limits::max()] IntSetting& setRange (int min, int max); diff --git a/apps/opencs/model/prefs/setting.cpp b/apps/opencs/model/prefs/setting.cpp index 39d997988..89924ae29 100644 --- a/apps/opencs/model/prefs/setting.cpp +++ b/apps/opencs/model/prefs/setting.cpp @@ -2,6 +2,7 @@ #include "setting.hpp" #include +#include #include "category.hpp" #include "state.hpp" @@ -11,9 +12,15 @@ Settings::Manager& CSMPrefs::Setting::getValues() return *mValues; } -CSMPrefs::Setting::Setting (Category *parent, Settings::Manager *values, +QMutex *CSMPrefs::Setting::getMutex() +{ + return mMutex; +} + +CSMPrefs::Setting::Setting (Category *parent, Settings::Manager *values, QMutex *mutex, const std::string& key, const std::string& label) -: QObject (parent->getState()), mParent (parent), mValues (values), mKey (key), mLabel (label) +: QObject (parent->getState()), mParent (parent), mValues (values), mMutex (mutex), mKey (key), + mLabel (label) {} CSMPrefs::Setting:: ~Setting() {} @@ -40,26 +47,31 @@ const std::string& CSMPrefs::Setting::getLabel() const int CSMPrefs::Setting::toInt() const { + QMutexLocker lock (mMutex); return mValues->getInt (mKey, mParent->getKey()); } double CSMPrefs::Setting::toDouble() const { + QMutexLocker lock (mMutex); return mValues->getFloat (mKey, mParent->getKey()); } std::string CSMPrefs::Setting::toString() const { + QMutexLocker lock (mMutex); return mValues->getString (mKey, mParent->getKey()); } bool CSMPrefs::Setting::isTrue() const { + QMutexLocker lock (mMutex); return mValues->getBool (mKey, mParent->getKey()); } QColor CSMPrefs::Setting::toColor() const { + QMutexLocker lock (mMutex); return QColor (QString::fromUtf8 (toString().c_str())); } diff --git a/apps/opencs/model/prefs/setting.hpp b/apps/opencs/model/prefs/setting.hpp index 65354469d..00bcc638b 100644 --- a/apps/opencs/model/prefs/setting.hpp +++ b/apps/opencs/model/prefs/setting.hpp @@ -8,6 +8,7 @@ class QWidget; class QColor; +class QMutex; namespace Settings { @@ -24,6 +25,7 @@ namespace CSMPrefs Category *mParent; Settings::Manager *mValues; + QMutex *mMutex; std::string mKey; std::string mLabel; @@ -31,9 +33,11 @@ namespace CSMPrefs Settings::Manager& getValues(); + QMutex *getMutex(); + public: - Setting (Category *parent, Settings::Manager *values, const std::string& key, const std::string& label); + Setting (Category *parent, Settings::Manager *values, QMutex *mutex, const std::string& key, const std::string& label); virtual ~Setting(); diff --git a/apps/opencs/model/prefs/state.cpp b/apps/opencs/model/prefs/state.cpp index ab9fd0a28..64ce512bf 100644 --- a/apps/opencs/model/prefs/state.cpp +++ b/apps/opencs/model/prefs/state.cpp @@ -220,7 +220,8 @@ CSMPrefs::IntSetting& CSMPrefs::State::declareInt (const std::string& key, default_ = mSettings.getInt (key, mCurrentCategory->second.getKey()); CSMPrefs::IntSetting *setting = - new CSMPrefs::IntSetting (&mCurrentCategory->second, &mSettings, key, label, default_); + new CSMPrefs::IntSetting (&mCurrentCategory->second, &mSettings, &mMutex, key, label, + default_); mCurrentCategory->second.addSetting (setting); @@ -240,7 +241,8 @@ CSMPrefs::DoubleSetting& CSMPrefs::State::declareDouble (const std::string& key, default_ = mSettings.getFloat (key, mCurrentCategory->second.getKey()); CSMPrefs::DoubleSetting *setting = - new CSMPrefs::DoubleSetting (&mCurrentCategory->second, &mSettings, key, label, default_); + new CSMPrefs::DoubleSetting (&mCurrentCategory->second, &mSettings, &mMutex, + key, label, default_); mCurrentCategory->second.addSetting (setting); @@ -258,7 +260,8 @@ CSMPrefs::BoolSetting& CSMPrefs::State::declareBool (const std::string& key, default_ = mSettings.getBool (key, mCurrentCategory->second.getKey()); CSMPrefs::BoolSetting *setting = - new CSMPrefs::BoolSetting (&mCurrentCategory->second, &mSettings, key, label, default_); + new CSMPrefs::BoolSetting (&mCurrentCategory->second, &mSettings, &mMutex, key, label, + default_); mCurrentCategory->second.addSetting (setting); @@ -276,7 +279,8 @@ CSMPrefs::EnumSetting& CSMPrefs::State::declareEnum (const std::string& key, default_.mValue = mSettings.getString (key, mCurrentCategory->second.getKey()); CSMPrefs::EnumSetting *setting = - new CSMPrefs::EnumSetting (&mCurrentCategory->second, &mSettings, key, label, default_); + new CSMPrefs::EnumSetting (&mCurrentCategory->second, &mSettings, &mMutex, key, label, + default_); mCurrentCategory->second.addSetting (setting); @@ -294,7 +298,8 @@ CSMPrefs::ColourSetting& CSMPrefs::State::declareColour (const std::string& key, default_.setNamedColor (QString::fromUtf8 (mSettings.getString (key, mCurrentCategory->second.getKey()).c_str())); CSMPrefs::ColourSetting *setting = - new CSMPrefs::ColourSetting (&mCurrentCategory->second, &mSettings, key, label, default_); + new CSMPrefs::ColourSetting (&mCurrentCategory->second, &mSettings, &mMutex, key, label, + default_); mCurrentCategory->second.addSetting (setting); @@ -307,7 +312,7 @@ void CSMPrefs::State::declareSeparator() throw std::logic_error ("no category for setting"); CSMPrefs::Setting *setting = - new CSMPrefs::Setting (&mCurrentCategory->second, &mSettings, "", ""); + new CSMPrefs::Setting (&mCurrentCategory->second, &mSettings, &mMutex, "", ""); mCurrentCategory->second.addSetting (setting); } diff --git a/apps/opencs/model/prefs/state.hpp b/apps/opencs/model/prefs/state.hpp index 7807dac25..bcd76c671 100644 --- a/apps/opencs/model/prefs/state.hpp +++ b/apps/opencs/model/prefs/state.hpp @@ -5,6 +5,7 @@ #include #include +#include #ifndef Q_MOC_RUN #include @@ -25,6 +26,10 @@ namespace CSMPrefs class BoolSetting; class ColourSetting; + /// \brief User settings state + /// + /// \note Access to the user settings is thread-safe once all declarations and loading has + /// been completed. class State : public QObject { Q_OBJECT @@ -43,6 +48,7 @@ namespace CSMPrefs Settings::Manager mSettings; Collection mCategories; Iterator mCurrentCategory; + QMutex mMutex; // not implemented State (const State&);