From 06dd0ba3b467487a697d0bcb66d929e2a1516620 Mon Sep 17 00:00:00 2001 From: comex Date: Tue, 21 Apr 2015 14:46:44 -0400 Subject: [PATCH] Exit ReadDataFromFifoOnCPU, PushFifoAuxBuffer early if shutting down (GpuRunningState=false) This was causing a race condition where the "absurdly large aux buffer" panic alert would be triggered in the last bit of fifo processing on the CPU thread in deterministic mode (i.e. netplay). SyncGPU is supposed to move the auxiliary queue data to the beginning of the containing buffer so we don't have to deal with wraparound; if GpuRunningState is false, however, it just returns, because it's set to false by another thread - thus it doesn't know whether RunGpuLoop is still executing (in which case it can't just reset the pointers, because it may still be using the buffer) or not (in which case the condition variable it normally waits for to avoid the previous problem will never be signaled). However, SyncGPU's caller PushFifoAuxBuffer wasn't aware of this, so if the buffer was filling at just the right time, it'd stay full and that function would complain that it was about to overflow it. Similar problem with ReadDataFromFifoOnCPU afaik. Fix this by returning early from those as well; other callers of SyncGPU should be safe. A *slightly* cleaner alternative would be giving the CPU thread a way to tell when RunGpuLoop has actually exited, but whatever, this works. --- Source/Core/VideoCommon/Fifo.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/Source/Core/VideoCommon/Fifo.cpp b/Source/Core/VideoCommon/Fifo.cpp index c5e49fb583..0b29f2da0a 100644 --- a/Source/Core/VideoCommon/Fifo.cpp +++ b/Source/Core/VideoCommon/Fifo.cpp @@ -145,7 +145,7 @@ void EmulatorState(bool running) void SyncGPU(SyncGPUReason reason, bool may_move_read_ptr) { - if (g_use_deterministic_gpu_thread && GpuRunningState) + if (g_use_deterministic_gpu_thread) { std::unique_lock lk(s_video_buffer_lock); u8* write_ptr = s_video_buffer_write_ptr; @@ -185,6 +185,11 @@ void PushFifoAuxBuffer(void* ptr, size_t size) if (size > (size_t) (s_fifo_aux_data + FIFO_SIZE - s_fifo_aux_write_ptr)) { SyncGPU(SYNC_GPU_AUX_SPACE, /* may_move_read_ptr */ false); + if (!GpuRunningState) + { + // GPU is shutting down + return; + } if (size > (size_t) (s_fifo_aux_data + FIFO_SIZE - s_fifo_aux_write_ptr)) { // That will sync us up to the last 32 bytes, so this short region @@ -235,6 +240,12 @@ static void ReadDataFromFifoOnCPU(u32 readPtr) // We can't wrap around while the GPU is working on the data. // This should be very rare due to the reset in SyncGPU. SyncGPU(SYNC_GPU_WRAPAROUND); + if (!GpuRunningState) + { + // GPU is shutting down + return; + } + if (s_video_buffer_pp_read_ptr != s_video_buffer_read_ptr) { PanicAlert("desynced read pointers");