From 6d1c0dda9137eeb32f79ef9048957702c6f1360e Mon Sep 17 00:00:00 2001 From: WarmUpTill Date: Wed, 17 Jan 2024 20:58:23 +0100 Subject: [PATCH] Fix input dialogs causing crash on shutdown --- lib/advanced-scene-switcher.cpp | 23 ++------- lib/macro/macro-action-variable.cpp | 29 ++++++++---- lib/utils/non-modal-dialog.cpp | 73 +++++++++++++++++------------ lib/utils/non-modal-dialog.hpp | 9 ++-- 4 files changed, 75 insertions(+), 59 deletions(-) diff --git a/lib/advanced-scene-switcher.cpp b/lib/advanced-scene-switcher.cpp index 42f7aee7..439f63f8 100644 --- a/lib/advanced-scene-switcher.cpp +++ b/lib/advanced-scene-switcher.cpp @@ -470,7 +470,7 @@ void SwitcherData::Start() } } -bool CloseAllInputDialogs(); +void CloseAllInputDialogs(); void StopAndClearAllActionQueues(); void SwitcherData::Stop() @@ -482,23 +482,10 @@ void SwitcherData::Stop() GetMacroWaitCV().notify_all(); GetMacroTransitionCV().notify_all(); StopAndClearAllActionQueues(); - - // Not waiting if a dialog was closed is a workaround to avoid - // deadlocks when a variable input dialog is opened while Stop() - // is called. - // - // The QEventLoop handling the dialog would not exit until Stop() - // would return, which itself would wait for the switcher thread - // to complete, which itself is blocked by the variable input - // dialog. - // - // Must be reworked in the future, as this will not go well when - // OBS is shutting down and a dialog is still opened. - if (!CloseAllInputDialogs()) { - th->wait(); - delete th; - th = nullptr; - } + CloseAllInputDialogs(); + th->wait(); + delete th; + th = nullptr; writeToStatusFile("Advanced Scene Switcher stopped"); } diff --git a/lib/macro/macro-action-variable.cpp b/lib/macro/macro-action-variable.cpp index fe138d1c..312c8656 100644 --- a/lib/macro/macro-action-variable.cpp +++ b/lib/macro/macro-action-variable.cpp @@ -181,18 +181,23 @@ void MacroActionVariable::SetToSceneItemName(Variable *var) } struct AskForInputParams { + AskForInputParams(const QString &prompt_, const QString &placeholder_) + : prompt(prompt_), placeholder(placeholder_){}; QString prompt; QString placeholder; std::optional result; + std::atomic_bool resultReady = {false}; }; static void askForInput(void *param) { - auto parameters = static_cast(param); + auto parameters = + *static_cast *>(param); auto dialog = new NonModalMessageDialog( - parameters->prompt, NonModalMessageDialog::Type::INPUT); + parameters->prompt, NonModalMessageDialog::Type::INPUT, false); dialog->SetInput(parameters->placeholder); parameters->result = dialog->GetInput(); + parameters->resultReady = true; } bool MacroActionVariable::PerformAction() @@ -264,7 +269,7 @@ bool MacroActionVariable::PerformAction() return true; } case Type::USER_INPUT: { - AskForInputParams params{ + auto params = std::make_shared( _useCustomPrompt ? QString::fromStdString(_inputPrompt) : QString(obs_module_text( @@ -273,13 +278,19 @@ bool MacroActionVariable::PerformAction() var->Name())), _useCustomPrompt && _useInputPlaceholder ? QString::fromStdString(_inputPlaceholder) - : "", - {}}; - obs_queue_task(OBS_TASK_UI, askForInput, ¶ms, true); - if (!params.result.has_value()) { - return false; + : ""); + obs_queue_task(OBS_TASK_UI, askForInput, ¶ms, false); + while (!params->resultReady) { + if (GetMacro()->GetStop()) { + return false; + } + std::this_thread::sleep_for( + std::chrono::milliseconds(100)); } - var->SetValue(*params.result); + if (!params->result.has_value()) { + return true; + } + var->SetValue(*params->result); return true; } case Type::ENV_VARIABLE: { diff --git a/lib/utils/non-modal-dialog.cpp b/lib/utils/non-modal-dialog.cpp index f11a009d..9d8d4597 100644 --- a/lib/utils/non-modal-dialog.cpp +++ b/lib/utils/non-modal-dialog.cpp @@ -1,24 +1,40 @@ #include "non-modal-dialog.hpp" #include "obs-module-helper.hpp" -#include -#include -#include -#include +#include +#include #include +#include +#include +#include +#include +#include namespace advss { +static std::mutex mutex; +static std::vector dialogs; + QWidget *GetSettingsWindow(); NonModalMessageDialog::NonModalMessageDialog(const QString &message, - bool question) - : NonModalMessageDialog(message, question ? Type::QUESTION : Type::INFO) + bool question, + bool focusAdvssWindow) + : NonModalMessageDialog(message, question ? Type::QUESTION : Type::INFO, + focusAdvssWindow) { } -NonModalMessageDialog::NonModalMessageDialog(const QString &message, Type type) - : QDialog(GetSettingsWindow() +NonModalMessageDialog::~NonModalMessageDialog() +{ + std::lock_guard lock(mutex); + dialogs.erase(std::remove(dialogs.begin(), dialogs.end(), this), + dialogs.end()); +} + +NonModalMessageDialog::NonModalMessageDialog(const QString &message, Type type, + bool focusAdvssWindow) + : QDialog(focusAdvssWindow && GetSettingsWindow() ? GetSettingsWindow() : static_cast( obs_frontend_get_main_window())), @@ -65,6 +81,9 @@ NonModalMessageDialog::NonModalMessageDialog(const QString &message, Type type) } } setLayout(layout); + + std::lock_guard lock(mutex); + dialogs.emplace_back(this); } QMessageBox::StandardButton NonModalMessageDialog::ShowMessage() @@ -83,7 +102,8 @@ std::optional NonModalMessageDialog::GetInput() _inputEdit->setPlainText(_inputEdit->toPlainText()); exec(); - this->deleteLater(); + deleteLater(); + if (_answer == QMessageBox::Yes) { return _input.toStdString(); } @@ -115,28 +135,23 @@ void NonModalMessageDialog::InputChanged() updateGeometry(); } -bool CloseAllInputDialogs() +void CloseAllInputDialogs() { - auto window = - static_cast(obs_frontend_get_main_window()); - if (!window) { - return false; + std::lock_guard lock(mutex); + if (dialogs.empty()) { + return; } - - bool closedAtLeastOneDialog = false; - QList widgets = window->findChildren(); - for (auto &widget : widgets) { - auto dialog = qobject_cast(widget); - if (!dialog) { - continue; - } - if (dialog->GetType() != NonModalMessageDialog::Type::INPUT) { - continue; - } - dialog->close(); - closedAtLeastOneDialog = true; - } - return closedAtLeastOneDialog; + obs_queue_task( + OBS_TASK_UI, + [](void *) { + for (const auto dialog : dialogs) { + if (dialog->GetType() == + NonModalMessageDialog::Type::INPUT) { + dialog->close(); + } + } + }, + nullptr, true); } } // namespace advss diff --git a/lib/utils/non-modal-dialog.hpp b/lib/utils/non-modal-dialog.hpp index d4684e98..c057b27b 100644 --- a/lib/utils/non-modal-dialog.hpp +++ b/lib/utils/non-modal-dialog.hpp @@ -7,7 +7,7 @@ namespace advss { -bool CloseAllInputDialogs(); +void CloseAllInputDialogs(); class NonModalMessageDialog : public QDialog { Q_OBJECT @@ -15,8 +15,11 @@ class NonModalMessageDialog : public QDialog { public: enum class Type { INFO, QUESTION, INPUT }; - NonModalMessageDialog(const QString &message, Type); - NonModalMessageDialog(const QString &message, bool question); + NonModalMessageDialog(const QString &message, Type, + bool focusAdvssWindow = false); + NonModalMessageDialog(const QString &message, bool question, + bool focusAdvssWindow = false); + ~NonModalMessageDialog(); QMessageBox::StandardButton ShowMessage(); std::optional GetInput(); Type GetType() const { return _type; }