From ba38b8bf2755f85b6eaf1df1e7c793f5545cfbc4 Mon Sep 17 00:00:00 2001 From: WarmUpTill <19472752+WarmUpTill@users.noreply.github.com> Date: Sat, 4 Apr 2026 19:05:37 +0200 Subject: [PATCH] Don't block UI while executing long runnig actions The previous approach had the problem of losing any action internal state changes in the created copy. Revert "Fix temp var values of actions not being accessible" This reverts commit df425383190004b7bcb0230793041e44e2d516f2. Revert "Don't block UI while running actions" This reverts commit a01d26e25dc7e640e9ca211e4793f5453b200531. --- lib/macro/macro-segment.cpp | 18 ----- lib/macro/macro-segment.hpp | 1 - lib/macro/macro.cpp | 10 +-- lib/utils/sync-helpers.cpp | 11 +++ lib/utils/sync-helpers.hpp | 18 +++++ plugins/base/macro-action-media.cpp | 3 +- plugins/base/macro-action-media.hpp | 2 +- plugins/base/macro-action-play-audio.cpp | 5 +- plugins/base/macro-action-run.cpp | 17 ++++- plugins/base/macro-action-wait.cpp | 7 +- plugins/http/macro-action-http.cpp | 76 +++++++++++--------- plugins/scripting/macro-action-script.cpp | 1 + plugins/scripting/macro-condition-script.cpp | 1 + 13 files changed, 102 insertions(+), 68 deletions(-) diff --git a/lib/macro/macro-segment.cpp b/lib/macro/macro-segment.cpp index 4900c365..f10907a8 100644 --- a/lib/macro/macro-segment.cpp +++ b/lib/macro/macro-segment.cpp @@ -167,24 +167,6 @@ bool MacroSegment::IsTempVarInUse(const std::string &id) const return false; } -void MacroSegment::CopyTempVarValuesFrom(const MacroSegment &other) -{ - for (const auto &src : other._tempVariables) { - for (auto &dst : _tempVariables) { - if (dst.ID() != src.ID()) { - continue; - } - auto value = src.Value(); - if (value) { - dst.SetValue(*value); - } else { - dst.InvalidateValue(); - } - break; - } - } -} - void MacroSegment::SetTempVarValue(const std::string &id, const std::string &value) { diff --git a/lib/macro/macro-segment.hpp b/lib/macro/macro-segment.hpp index b5824b54..803379d2 100644 --- a/lib/macro/macro-segment.hpp +++ b/lib/macro/macro-segment.hpp @@ -57,7 +57,6 @@ protected: bool IsTempVarInUse(const std::string &id) const; void SetTempVarValue(const std::string &id, const std::string &value); - void CopyTempVarValuesFrom(const MacroSegment &other); template, bool>::value>> diff --git a/lib/macro/macro.cpp b/lib/macro/macro.cpp index e38db3a0..0e00ab17 100644 --- a/lib/macro/macro.cpp +++ b/lib/macro/macro.cpp @@ -428,13 +428,9 @@ bool Macro::RunActionsHelper( } if (action->Enabled()) { action->LogAction(); - std::shared_ptr actionCopy; - action->WithLock([&action, &actionCopy]() { - actionCopy = action->Copy(); - }); - bool actionResult = actionCopy->PerformAction(); - action->WithLock([&action, &actionCopy]() { - action->CopyTempVarValuesFrom(*actionCopy); + bool actionResult = false; + action->WithLock([&action, &actionResult]() { + actionResult = action->PerformAction(); }); actionsExecutedSuccessfully = actionsExecutedSuccessfully && actionResult; diff --git a/lib/utils/sync-helpers.cpp b/lib/utils/sync-helpers.cpp index b7052b87..63c2b9c3 100644 --- a/lib/utils/sync-helpers.cpp +++ b/lib/utils/sync-helpers.cpp @@ -66,4 +66,15 @@ void Lockable::WithLock(const std::function &func) func(); } +SuspendLock::SuspendLock(Lockable &lockable) + : _mtx(static_cast(lockable._mtx)) +{ + _mtx.unlock(); +} + +SuspendLock::~SuspendLock() +{ + _mtx.lock(); +} + } // namespace advss diff --git a/lib/utils/sync-helpers.hpp b/lib/utils/sync-helpers.hpp index bd23c1a0..bbe99e31 100644 --- a/lib/utils/sync-helpers.hpp +++ b/lib/utils/sync-helpers.hpp @@ -51,6 +51,24 @@ public: private: PerInstanceMutex _mtx; + + friend class SuspendLock; +}; + +// RAII guard that temporarily releases a Lockable's per-segment lock. +// Use this inside PerformAction() / CheckCondition() to unblock the UI +// during long-running operations while still running on the original segment. +// The caller MUST be holding the lock (i.e. be inside WithLock) when +// constructing this object; the lock is re-acquired on destruction. +class EXPORT SuspendLock { +public: + SuspendLock(Lockable &lockable); + ~SuspendLock(); + SuspendLock(const SuspendLock &) = delete; + SuspendLock &operator=(const SuspendLock &) = delete; + +private: + std::mutex &_mtx; }; } // namespace advss diff --git a/plugins/base/macro-action-media.cpp b/plugins/base/macro-action-media.cpp index 70f84726..5f85c553 100644 --- a/plugins/base/macro-action-media.cpp +++ b/plugins/base/macro-action-media.cpp @@ -95,7 +95,7 @@ static void waitHelper(std::unique_lock *lock, Macro *macro, } } -void MacroActionMedia::PerformActionHelper(obs_source_t *source) const +void MacroActionMedia::PerformActionHelper(obs_source_t *source) { obs_media_state state = obs_source_media_get_state(source); @@ -130,6 +130,7 @@ void MacroActionMedia::PerformActionHelper(obs_source_t *source) const SeekToPercentage(source); break; case Action::WAIT_FOR_PLAYBACK_STOP: { + SuspendLock suspendLock(*this); std::unique_lock lock(*GetMutex()); waitHelper(&lock, GetMacro(), source); break; diff --git a/plugins/base/macro-action-media.hpp b/plugins/base/macro-action-media.hpp index f0892658..3df6b4de 100644 --- a/plugins/base/macro-action-media.hpp +++ b/plugins/base/macro-action-media.hpp @@ -45,7 +45,7 @@ public: SceneSelection _scene; private: - void PerformActionHelper(obs_source_t *) const; + void PerformActionHelper(obs_source_t *); void SeekToPercentage(obs_source_t *source) const; static bool _registered; diff --git a/plugins/base/macro-action-play-audio.cpp b/plugins/base/macro-action-play-audio.cpp index a6d53f57..a4f4c502 100644 --- a/plugins/base/macro-action-play-audio.cpp +++ b/plugins/base/macro-action-play-audio.cpp @@ -159,7 +159,10 @@ bool MacroActionPlayAudio::PerformAction() if (_waitForCompletion) { SetMacroAbortWait(false); - waitForPlaybackToEnd(GetMacro(), source, maxMs); + { + SuspendLock suspendLock(*this); + waitForPlaybackToEnd(GetMacro(), source, maxMs); + } deactivatePlayback(source, wantsOutput); return true; } diff --git a/plugins/base/macro-action-run.cpp b/plugins/base/macro-action-run.cpp index 59680e5e..f7af708a 100644 --- a/plugins/base/macro-action-run.cpp +++ b/plugins/base/macro-action-run.cpp @@ -15,9 +15,20 @@ bool MacroActionRun::_registered = MacroActionFactory::Register( bool MacroActionRun::PerformAction() { if (_wait) { - _procConfig.StartProcessAndWait(_timeout.Milliseconds()); - SetTempVarValues(); - + // Snapshot config before releasing the lock for the blocking wait + auto procConfig = _procConfig; + const auto timeout = _timeout.Milliseconds(); + { + SuspendLock suspendLock(*this); + procConfig.StartProcessAndWait(timeout); + } + SetTempVarValue("process.id", procConfig.GetProcessId()); + SetTempVarValue("process.exitCode", + procConfig.GetProcessExitCode()); + SetTempVarValue("process.stream.output", + procConfig.GetProcessOutputStream()); + SetTempVarValue("process.stream.error", + procConfig.GetProcessErrorStream()); return true; } diff --git a/plugins/base/macro-action-wait.cpp b/plugins/base/macro-action-wait.cpp index 16b73871..10d3541b 100644 --- a/plugins/base/macro-action-wait.cpp +++ b/plugins/base/macro-action-wait.cpp @@ -57,8 +57,11 @@ bool MacroActionWait::PerformAction() std::chrono::milliseconds((int)(sleepDuration * 1000)); SetMacroAbortWait(false); - std::unique_lock lock(*GetMutex()); - waitHelper(&lock, GetMacro(), time); + { + SuspendLock suspendLock(*this); + std::unique_lock lock(*GetMutex()); + waitHelper(&lock, GetMacro(), time); + } return !MacroWaitShouldAbort(); } diff --git a/plugins/http/macro-action-http.cpp b/plugins/http/macro-action-http.cpp index 2df65c25..445b5d8d 100644 --- a/plugins/http/macro-action-http.cpp +++ b/plugins/http/macro-action-http.cpp @@ -112,48 +112,56 @@ static URLInfo getURLInfo(const std::string &input, bool keepParams) bool MacroActionHttp::PerformAction() { + // Capture all config while holding the segment lock const auto [host, path] = getURLInfo(_url, !_setParams); - httplib::Client cli(host); setTimeout(cli, _timeout); const auto params = _setParams ? getParams(_params) : httplib::Params(); const auto headers = _setHeaders ? getHeaders(_headers) : httplib::Headers(); + const auto method = _method; + const std::string body = _body; + const std::string contentType = _contentType; + // Release the segment lock for the blocking network call httplib::Result response; - switch (_method) { - case MacroActionHttp::Method::GET: - response = cli.Get(path, params, headers); - break; - case MacroActionHttp::Method::POST: { - const auto pathWithParam = - httplib::append_query_params(path, params); - response = - cli.Post(pathWithParam, headers, _body, _contentType); - break; - } - case MacroActionHttp::Method::PUT: { - const auto pathWithParam = - httplib::append_query_params(path, params); - response = cli.Put(pathWithParam, headers, _body, _contentType); - break; - } - case MacroActionHttp::Method::PATCH: { - const auto pathWithParam = - httplib::append_query_params(path, params); - response = - cli.Patch(pathWithParam, headers, _body, _contentType); - break; - } - case MacroActionHttp::Method::DELETE: { - const auto pathWithParam = - httplib::append_query_params(path, params); - response = - cli.Delete(pathWithParam, headers, _body, _contentType); - break; - } - default: - break; + { + SuspendLock suspendLock(*this); + switch (method) { + case MacroActionHttp::Method::GET: + response = cli.Get(path, params, headers); + break; + case MacroActionHttp::Method::POST: { + const auto pathWithParam = + httplib::append_query_params(path, params); + response = cli.Post(pathWithParam, headers, body, + contentType); + break; + } + case MacroActionHttp::Method::PUT: { + const auto pathWithParam = + httplib::append_query_params(path, params); + response = cli.Put(pathWithParam, headers, body, + contentType); + break; + } + case MacroActionHttp::Method::PATCH: { + const auto pathWithParam = + httplib::append_query_params(path, params); + response = cli.Patch(pathWithParam, headers, body, + contentType); + break; + } + case MacroActionHttp::Method::DELETE: { + const auto pathWithParam = + httplib::append_query_params(path, params); + response = cli.Delete(pathWithParam, headers, body, + contentType); + break; + } + default: + break; + } } if (VerboseLoggingEnabled() && !response) { diff --git a/plugins/scripting/macro-action-script.cpp b/plugins/scripting/macro-action-script.cpp index 9659fa70..2dab2d87 100644 --- a/plugins/scripting/macro-action-script.cpp +++ b/plugins/scripting/macro-action-script.cpp @@ -35,6 +35,7 @@ bool MacroActionScript::PerformAction() return true; } + SuspendLock suspendLock(static_cast(*this)); (void)SendTriggerSignal(); return true; } diff --git a/plugins/scripting/macro-condition-script.cpp b/plugins/scripting/macro-condition-script.cpp index fabc3d15..2d8fa8da 100644 --- a/plugins/scripting/macro-condition-script.cpp +++ b/plugins/scripting/macro-condition-script.cpp @@ -36,6 +36,7 @@ bool MacroConditionScript::CheckCondition() return false; } + SuspendLock suspendLock(static_cast(*this)); return SendTriggerSignal(); }