Skip to content

Commit

Permalink
DolphinQt: Fix the panic alert deadlock, GPU thread edition
Browse files Browse the repository at this point in the history
The fix in ef77872 worked for panic alerts from
the CPU thread, but there were still problems with
panic alerts from the GPU thread in dual core mode.
This change attempts to fix those.
  • Loading branch information
JosJuice committed Jan 13, 2022
1 parent 07fd174 commit 3367e5e
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 21 deletions.
22 changes: 14 additions & 8 deletions Source/Core/Core/Core.cpp
Expand Up @@ -127,6 +127,7 @@ static std::queue<HostJob> s_host_jobs_queue;
static Common::Event s_cpu_thread_job_finished;

static thread_local bool tls_is_cpu_thread = false;
static thread_local bool tls_is_gpu_thread = false;

static void EmuThread(std::unique_ptr<BootParameters> boot, WindowSystemInfo wsi);

Expand Down Expand Up @@ -203,14 +204,7 @@ bool IsCPUThread()

bool IsGPUThread()
{
if (Core::System::GetInstance().IsDualCoreMode())
{
return (s_emu_thread.joinable() && (s_emu_thread.get_id() == std::this_thread::get_id()));
}
else
{
return IsCPUThread();
}
return tls_is_gpu_thread;
}

bool WantsDeterminism()
Expand Down Expand Up @@ -313,6 +307,16 @@ void UndeclareAsCPUThread()
tls_is_cpu_thread = false;
}

void DeclareAsGPUThread()
{
tls_is_gpu_thread = true;
}

void UndeclareAsGPUThread()
{
tls_is_gpu_thread = false;
}

// For the CPU Thread only.
static void CPUSetInitialExecutionState(bool force_paused = false)
{
Expand Down Expand Up @@ -459,6 +463,8 @@ static void EmuThread(std::unique_ptr<BootParameters> boot, WindowSystemInfo wsi

Common::SetCurrentThreadName("Emuthread - Starting");

DeclareAsGPUThread();

// For a time this acts as the CPU thread...
DeclareAsCPUThread();
s_frame_step = false;
Expand Down
2 changes: 2 additions & 0 deletions Source/Core/Core/Core.h
Expand Up @@ -98,6 +98,8 @@ void Shutdown();

void DeclareAsCPUThread();
void UndeclareAsCPUThread();
void DeclareAsGPUThread();
void UndeclareAsGPUThread();

std::string StopMessage(bool main_thread, std::string_view message);

Expand Down
23 changes: 21 additions & 2 deletions Source/Core/DolphinQt/Host.cpp
Expand Up @@ -3,6 +3,8 @@

#include "DolphinQt/Host.h"

#include <functional>

#include <QAbstractEventDispatcher>
#include <QApplication>
#include <QLocale>
Expand Down Expand Up @@ -85,6 +87,19 @@ void Host::SetMainWindowHandle(void* handle)
m_main_window_handle = handle;
}

static void RunWithGPUThreadInactive(std::function<void()> f)
{
// If we are the GPU thread in dual core mode, we cannot safely call
// RunAsCPUThread, since RunAsCPUThread will need to pause the GPU thread
// and the GPU thread is waiting for us to finish. Fortunately, if we know
// that the GPU thread is waiting for us, we can just run f directly.

if (Core::IsGPUThread())
f();
else
Core::RunAsCPUThread(std::move(f));
}

bool Host::GetRenderFocus()
{
#ifdef _WIN32
Expand All @@ -107,10 +122,12 @@ void Host::SetRenderFocus(bool focus)
{
m_render_focus = focus;
if (g_renderer && m_render_fullscreen && g_ActiveConfig.ExclusiveFullscreenEnabled())
Core::RunAsCPUThread([focus] {
{
RunWithGPUThreadInactive([focus] {
if (!Config::Get(Config::MAIN_RENDER_TO_MAIN))
g_renderer->SetFullscreen(focus);
});
}
}

void Host::SetRenderFullFocus(bool focus)
Expand Down Expand Up @@ -138,7 +155,9 @@ void Host::SetRenderFullscreen(bool fullscreen)

if (g_renderer && g_renderer->IsFullscreen() != fullscreen &&
g_ActiveConfig.ExclusiveFullscreenEnabled())
Core::RunAsCPUThread([fullscreen] { g_renderer->SetFullscreen(fullscreen); });
{
RunWithGPUThreadInactive([fullscreen] { g_renderer->SetFullscreen(fullscreen); });
}
}

void Host::ResizeSurface(int new_width, int new_height)
Expand Down
25 changes: 18 additions & 7 deletions Source/Core/DolphinQt/Main.cpp
Expand Up @@ -41,20 +41,31 @@ static bool QtMsgAlertHandler(const char* caption, const char* text, bool yes_no
Common::MsgType style)
{
const bool called_from_cpu_thread = Core::IsCPUThread();
const bool called_from_gpu_thread = Core::IsGPUThread();

std::optional<bool> r = RunOnObject(QApplication::instance(), [&] {
Common::ScopeGuard scope_guard(&Core::UndeclareAsCPUThread);
Common::ScopeGuard cpu_scope_guard(&Core::UndeclareAsCPUThread);
Common::ScopeGuard gpu_scope_guard(&Core::UndeclareAsGPUThread);

if (!called_from_cpu_thread)
cpu_scope_guard.Dismiss();
if (!called_from_gpu_thread)
gpu_scope_guard.Dismiss();

if (called_from_cpu_thread)
{
// Temporarily declare this as the CPU thread to avoid getting a deadlock if any DolphinQt
// code calls RunAsCPUThread while the CPU thread is blocked on this function returning.
// Notably, if the panic alert steals focus from RenderWidget, Host::SetRenderFocus gets
// called, which can attempt to use RunAsCPUThread to get us out of exclusive fullscreen.
// 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();
}
else
if (called_from_gpu_thread)
{
scope_guard.Dismiss();
// 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();
}

ModalMessageBox message_box(QApplication::activeWindow(), Qt::ApplicationModal);
Expand Down
8 changes: 4 additions & 4 deletions Source/Core/DolphinQt/RenderWidget.cpp
Expand Up @@ -421,10 +421,10 @@ bool RenderWidget::event(QEvent* event)

if (Config::Get(Config::MAIN_PAUSE_ON_FOCUS_LOST) && Core::GetState() == Core::State::Running)
{
// If we are declared as the CPU thread, it means that the real CPU thread is waiting
// for us to finish showing a panic alert (with that panic alert likely being the cause
// of this event), so trying to pause the real CPU thread would cause a deadlock
if (!Core::IsCPUThread())
// If we are declared as the CPU or GPU thread, it means that the real CPU or GPU thread
// is waiting for us to finish showing a panic alert (with that panic alert likely being
// the cause of this event), so trying to pause the core would cause a deadlock
if (!Core::IsCPUThread() && !Core::IsGPUThread())
Core::SetState(Core::State::Paused);
}

Expand Down

0 comments on commit 3367e5e

Please sign in to comment.