From 4d5ffe38fc13fcf8fda6e8f1f59377068e3dcc9e Mon Sep 17 00:00:00 2001 From: WarmUpTill <19472752+WarmUpTill@users.noreply.github.com> Date: Mon, 23 Mar 2026 19:51:32 +0100 Subject: [PATCH] Fix race conditions when accessing variables --- lib/variables/variable-string.cpp | 7 +++-- lib/variables/variable.cpp | 52 +++++++++++++++++++++++-------- lib/variables/variable.hpp | 9 +++--- tests/test-variable.cpp | 2 +- 4 files changed, 49 insertions(+), 21 deletions(-) diff --git a/lib/variables/variable-string.cpp b/lib/variables/variable-string.cpp index c555558b..31f5c2c0 100644 --- a/lib/variables/variable-string.cpp +++ b/lib/variables/variable-string.cpp @@ -9,11 +9,12 @@ void StringVariable::Resolve() const _resolvedValue = _value; return; } - if (_lastResolve == GetLastVariableChangeTime()) { + const auto lastChange = GetLastVariableChangeTime(); + if (_lastResolve == lastChange) { return; } _resolvedValue = SubstitueVariables(_value); - _lastResolve = GetLastVariableChangeTime(); + _lastResolve = lastChange; } StringVariable::operator std::string() const @@ -81,7 +82,7 @@ std::string SubstitueVariables(std::string str) const auto &variable = std::dynamic_pointer_cast(v); const std::string pattern = "${" + variable->Name() + "}"; if (ReplaceAll(str, pattern, variable->Value(false))) { - variable->UpdateLastUsed(); + variable->MarkAsUsed(); } } return str; diff --git a/lib/variables/variable.cpp b/lib/variables/variable.cpp index 615460a9..a752b888 100644 --- a/lib/variables/variable.cpp +++ b/lib/variables/variable.cpp @@ -12,16 +12,23 @@ static std::deque> variables; // Keep track of the last time a variable was changed to save some work when // when resolving strings containing variables, etc. +static std::mutex lastVariableChangeMutex; static std::chrono::high_resolution_clock::time_point lastVariableChange{}; +static void setLastVariableChangeTime() +{ + std::lock_guard lock(lastVariableChangeMutex); + lastVariableChange = std::chrono::high_resolution_clock::now(); +} + Variable::Variable() : Item() { - lastVariableChange = std::chrono::high_resolution_clock::now(); + setLastVariableChangeTime(); } Variable::~Variable() { - lastVariableChange = std::chrono::high_resolution_clock::now(); + setLastVariableChangeTime(); } void Variable::Load(obs_data_t *obj) @@ -37,7 +44,7 @@ void Variable::Load(obs_data_t *obj) SetValue(_defaultValue); } - lastVariableChange = std::chrono::high_resolution_clock::now(); + setLastVariableChangeTime(); } void Variable::Save(obs_data_t *obj) const @@ -45,8 +52,11 @@ void Variable::Save(obs_data_t *obj) const Item::Save(obj); obs_data_set_int(obj, "saveAction", static_cast(_saveAction)); - if (_saveAction == SaveAction::SAVE) { - obs_data_set_string(obj, "value", _value.c_str()); + { + std::lock_guard lock(_mutex); + if (_saveAction == SaveAction::SAVE) { + obs_data_set_string(obj, "value", _value.c_str()); + } } obs_data_set_string(obj, "defaultValue", _defaultValue.c_str()); @@ -62,6 +72,18 @@ std::string Variable::Value(bool updateLastUsed) const return _value; } +std::string Variable::GetPreviousValue() const +{ + std::lock_guard lock(_mutex); + return _previousValue; +} + +int Variable::GetValueChangeCount() const +{ + std::lock_guard lock(_mutex); + return _valueChangeCount; +} + std::optional Variable::DoubleValue() const { return GetDouble(Value()); @@ -79,8 +101,11 @@ void Variable::SetValue(const std::string &value) _value = value; UpdateLastUsed(); - UpdateLastChanged(); - lastVariableChange = std::chrono::high_resolution_clock::now(); + if (_previousValue != _value) { + _lastChanged = std::chrono::high_resolution_clock::now(); + ++_valueChangeCount; + } + setLastVariableChangeTime(); } void Variable::SetValue(double value) @@ -90,6 +115,7 @@ void Variable::SetValue(double value) std::optional Variable::GetSecondsSinceLastUse() const { + std::lock_guard lock(_mutex); if (_lastUsed.time_since_epoch().count() == 0) { return {}; } @@ -101,6 +127,7 @@ std::optional Variable::GetSecondsSinceLastUse() const std::optional Variable::GetSecondsSinceLastChange() const { + std::lock_guard lock(_mutex); if (_lastChanged.time_since_epoch().count() == 0) { return {}; } @@ -116,12 +143,10 @@ void Variable::UpdateLastUsed() const _lastUsed = std::chrono::high_resolution_clock::now(); } -void Variable::UpdateLastChanged() +void Variable::MarkAsUsed() const { - if (_previousValue != _value) { - _lastChanged = std::chrono::high_resolution_clock::now(); - ++_valueChangeCount; - } + std::lock_guard lock(_mutex); + UpdateLastUsed(); } static void populateSaveActionSelection(QComboBox *list) @@ -206,7 +231,7 @@ bool VariableSettingsDialog::AskForSettings(QWidget *parent, Variable &settings) dialog._defaultValue->toPlainText().toStdString(); settings._saveAction = static_cast(dialog._save->currentIndex()); - lastVariableChange = std::chrono::high_resolution_clock::now(); + setLastVariableChangeTime(); return true; } @@ -461,6 +486,7 @@ void ImportVariables(obs_data_t *data) std::chrono::high_resolution_clock::time_point GetLastVariableChangeTime() { + std::lock_guard lock(lastVariableChangeMutex); return lastVariableChange; } diff --git a/lib/variables/variable.hpp b/lib/variables/variable.hpp index 91613829..970ce74e 100644 --- a/lib/variables/variable.hpp +++ b/lib/variables/variable.hpp @@ -34,16 +34,15 @@ public: EXPORT std::string Value(bool updateLastUsed = true) const; EXPORT std::optional DoubleValue() const; EXPORT std::optional IntValue() const; - std::string GetPreviousValue() const { return _previousValue; }; + std::string GetPreviousValue() const; std::string GetDefaultValue() const { return _defaultValue; } EXPORT void SetValue(const std::string &value); void SetValue(double value); SaveAction GetSaveAction() const { return _saveAction; } - int GetValueChangeCount() const { return _valueChangeCount; } + int GetValueChangeCount() const; std::optional GetSecondsSinceLastUse() const; std::optional GetSecondsSinceLastChange() const; - void UpdateLastUsed() const; - void UpdateLastChanged(); + void MarkAsUsed() const; private: SaveAction _saveAction = SaveAction::DONT_SAVE; @@ -55,6 +54,8 @@ private: mutable std::chrono::high_resolution_clock::time_point _lastChanged; mutable std::mutex _mutex; + void UpdateLastUsed() const; + friend VariableSelection; friend VariableSettingsDialog; }; diff --git a/tests/test-variable.cpp b/tests/test-variable.cpp index 265e54e9..1030c9ef 100644 --- a/tests/test-variable.cpp +++ b/tests/test-variable.cpp @@ -40,7 +40,7 @@ TEST_CASE("Variable", "[variable]") variable.Value(false); REQUIRE(*variable.GetSecondsSinceLastUse() > 1); - variable.UpdateLastUsed(); + variable.MarkAsUsed(); REQUIRE(*variable.GetSecondsSinceLastUse() == 0); variable.SetValue(123);