DolphinQt: Improve the earlier panic alert deadlock fix

If the purpose of calling SetFullscreen using RunAsCPUThread is
to make sure that the GPU thread is paused, the fix in ef77872
is faulty when dual core is used and a panic alert comes from
the CPU thread. This change re-adds synchronization for that case.
This commit is contained in:
JosJuice 2020-04-25 10:35:14 +02:00
parent 3367e5e026
commit d445d2ad36
2 changed files with 35 additions and 15 deletions

View File

@ -36,6 +36,7 @@
#include "UICommon/DiscordPresence.h" #include "UICommon/DiscordPresence.h"
#include "VideoCommon/Fifo.cpp"
#include "VideoCommon/RenderBase.h" #include "VideoCommon/RenderBase.h"
#include "VideoCommon/VideoConfig.h" #include "VideoCommon/VideoConfig.h"
@ -89,16 +90,41 @@ void Host::SetMainWindowHandle(void* handle)
static void RunWithGPUThreadInactive(std::function<void()> f) static void RunWithGPUThreadInactive(std::function<void()> f)
{ {
// If we are the GPU thread in dual core mode, we cannot safely call // Potentially any thread which shows panic alerts can be blocked on this returning.
// RunAsCPUThread, since RunAsCPUThread will need to pause the GPU thread // This means that, in order to avoid deadlocks, we need to be careful with how we
// and the GPU thread is waiting for us to finish. Fortunately, if we know // synchronize with other threads. Note that the panic alert handler temporarily declares
// that the GPU thread is waiting for us, we can just run f directly. // us as the CPU and/or GPU thread if the panic alert was requested by that thread.
// TODO: What about the unlikely case where the GPU thread calls the panic alert handler
// while the panic alert handler is processing a panic alert from the CPU thread?
if (Core::IsGPUThread()) if (Core::IsGPUThread())
{
// If we are the GPU thread, we can't call Core::PauseAndLock without getting a deadlock,
// since it would try to pause the GPU thread while that thread is waiting for us.
// However, since we know that the GPU thread is inactive, we can just run f directly.
f(); f();
}
else if (Core::IsCPUThread())
{
// If we are the CPU thread in dual core mode, we can't call Core::PauseAndLock, for the
// same reason as above. Instead, we use Fifo::PauseAndLock to pause the GPU thread only.
// (Note that this case cannot be reached in single core mode, because in single core mode,
// the CPU and GPU threads are the same thread, and we already checked for the GPU thread.)
const bool was_running = Core::GetState() == Core::State::Running;
Fifo::PauseAndLock(true, was_running);
f();
Fifo::PauseAndLock(false, was_running);
}
else else
{
// If we reach here, we can call Core::PauseAndLock (which we do using RunAsCPUThread).
Core::RunAsCPUThread(std::move(f)); Core::RunAsCPUThread(std::move(f));
} }
}
bool Host::GetRenderFocus() bool Host::GetRenderFocus()
{ {

View File

@ -44,6 +44,11 @@ static bool QtMsgAlertHandler(const char* caption, const char* text, bool yes_no
const bool called_from_gpu_thread = Core::IsGPUThread(); const bool called_from_gpu_thread = Core::IsGPUThread();
std::optional<bool> r = RunOnObject(QApplication::instance(), [&] { std::optional<bool> r = RunOnObject(QApplication::instance(), [&] {
// If we were called from the CPU/GPU thread, set us as the CPU/GPU thread.
// This information is used in order to avoid deadlocks when calling e.g.
// Host::SetRenderFocus or Core::RunAsCPUThread. (Host::SetRenderFocus
// can get called automatically when a dialog steals the focus.)
Common::ScopeGuard cpu_scope_guard(&Core::UndeclareAsCPUThread); Common::ScopeGuard cpu_scope_guard(&Core::UndeclareAsCPUThread);
Common::ScopeGuard gpu_scope_guard(&Core::UndeclareAsGPUThread); Common::ScopeGuard gpu_scope_guard(&Core::UndeclareAsGPUThread);
@ -53,20 +58,9 @@ static bool QtMsgAlertHandler(const char* caption, const char* text, bool yes_no
gpu_scope_guard.Dismiss(); gpu_scope_guard.Dismiss();
if (called_from_cpu_thread) if (called_from_cpu_thread)
{
// If the panic alert that we are about to create steals the focus from RenderWidget,
// Host::SetRenderFocus gets called, which can attempt to use RunAsCPUThread to get us out
// of exclusive fullscreen. If we don't declare ourselves as the CPU thread, RunAsCPUThread
// calls PauseAndLock, which causes a deadlock if the CPU thread is waiting on us returning.
Core::DeclareAsCPUThread(); Core::DeclareAsCPUThread();
}
if (called_from_gpu_thread) if (called_from_gpu_thread)
{
// We also need to avoid getting a deadlock when the GPU thread is waiting on us returning.
// Declaring ourselves as the GPU thread does not alter the behavior of RunAsCPUThread or
// PauseAndLock, but it does make Host::SetRenderFocus not call RunAsCPUThread.
Core::DeclareAsGPUThread(); Core::DeclareAsGPUThread();
}
ModalMessageBox message_box(QApplication::activeWindow(), Qt::ApplicationModal); ModalMessageBox message_box(QApplication::activeWindow(), Qt::ApplicationModal);
message_box.setWindowTitle(QString::fromUtf8(caption)); message_box.setWindowTitle(QString::fromUtf8(caption));