Movie: Fix crash when starting input recording on OpenGL single core #12279
Conversation
Use RunOnCPUThread instead of RunAsCPUThread in BeginRecordingInput. Most OpenGL functions require an OpenGL context to have been created on that thread before calling the function; when that isn't the case they return invalid results which can cause crashes when passed into other functions. Dolphin creates the OpenGL context in the EmuThread which then becomes either the CPU-GPU thread or the Video thread for single and dual core respectively. OpenGL functions must therefore be called from that thread. Movie::BeginRecordingInput is called from the Host thread and runs a block of code which ultimately creates a savestate, which in turn embeds the framebuffer which requires calling various OpenGL functions. In single core the use of RunAsCPUThread leads to this all happening on the Host thread, eventually leading to invalid OpenGL calls and a crash. In Dual core the crash is avoided because VideoBackendBase::DoState uses the AsyncRequests::DO_SAVE_STATE event which causes VideoCommon_DoState and its subsequent OpenGL calls to safely run on the Video thread. This commit uses RunOnCPUThread instead of RunAsCPUThread, which causes the subsequent code to run on the CPU-GPU thread in single core which has the valid OpenGL context and so doesn't crash.
|
This logic seems reasonable to me and your explanation makes sense.
As an aside, I find this surprising just reading the documentation for the function. Not sure if the documentation should be updated or if there's a bug here. |
It's working as intended but I agree the documentation isn't very clear. RunAsCPUThread doesn't change which c++ thread the code is running on. Instead if it's run from the Host thread it pauses the actual CPU thread (to avoid thread safety issues), uses DeclareAsCPUThread to cause future calls to IsCPUThread to return true, runs the code, then undeclares itself and resumes the actual CPU thread. This normally works fine because most calls only care about not racing the CPU thread rather than actually being on it, and the ones that do use RunOnCPUThread instead. |
Dentomologist
deleted the
opengl_single_core_fix_start_movie_recording_crash
branch
Fix Dolphin crashing when selecting "Start Recording Input" when using OpenGL and single core.
Uses RunOnCPUThread instead of RunAsCPUThread in BeginRecordingInput to ensure OpenGL functions are accessed from a thread with an OpenGL context created.
Background: Most OpenGL functions require an OpenGL context to have been created on that thread before calling the function; when that isn't the case they return invalid results which can cause crashes when passed into other functions.
Dolphin creates the OpenGL context in the EmuThread which then becomes either the CPU-GPU thread or the Video thread for single and dual core respectively. OpenGL functions must therefore be called from that thread.
Movie::BeginRecordingInput was called from the Host thread and runs a block of code which ultimately creates a savestate, which in turn embeds the framebuffer which requires calling various OpenGL functions.
In single core the use of RunAsCPUThread lead to this all happening on the Host thread, eventually leading to invalid OpenGL calls and a crash.
In Dual core the crash is avoided because VideoBackendBase::DoState uses the AsyncRequests::DO_SAVE_STATE event which causes VideoCommon_DoState and its subsequent OpenGL calls to safely run on the Video thread.
This PR uses RunOnCPUThread instead of RunAsCPUThread, which causes the subsequent code to run on the CPU-GPU thread in single core which has the valid OpenGL context and so doesn't crash.
Resolves https://bugs.dolphin-emu.org/issues/12554.