piwalker/dolphin
1
0
Fork 0
mirror of https://github.com/dolphin-emu/dolphin.git synced 2025-07-24 14:49:42 -06:00
Code Issues Packages Projects Releases Wiki Activity

CoreTiming: Fix unsafe usage of m_globals.global_timer in ScheduleEvent from non-CPU thread.

Browse Source
This commit is contained in:
Jordan Woyak
2025-06-29 20:23:41 -05:00
parent 9a0d4501f8
commit c597c70316
3 changed files with 12 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
Source/Core/Core/CoreTiming.cpp
View File

@ -282,7 +282,7 @@ void CoreTimingManager::ScheduleEvent(s64 cycles_into_future, EventType* event_t
} }
std::lock_guard lk(m_ts_write_lock); std::lock_guard lk(m_ts_write_lock);
m_ts_queue.Push(Event{m_globals.global_timer + cycles_into_future, 0, userdata, event_type}); m_ts_queue.Push(Event{cycles_into_future, 0, userdata, event_type});
} }
} }
@ -319,10 +319,14 @@ void CoreTimingManager::ForceExceptionCheck(s64 cycles)
void CoreTimingManager::MoveEvents() void CoreTimingManager::MoveEvents()
{ {
for (Event ev; m_ts_queue.Pop(ev);) while (!m_ts_queue.Empty())
{ {
auto& ev = m_event_queue.emplace_back(m_ts_queue.Front());
m_ts_queue.Pop();
ev.fifo_order = m_event_fifo_id++; ev.fifo_order = m_event_fifo_id++;
m_event_queue.emplace_back(std::move(ev)); ev.time += m_globals.global_timer;
std::ranges::push_heap(m_event_queue, std::ranges::greater{}); std::ranges::push_heap(m_event_queue, std::ranges::greater{});
} }
} }

3
Source/Core/Core/CoreTiming.h
View File

@ -186,6 +186,9 @@ private:
std::vector<Event> m_event_queue; std::vector<Event> m_event_queue;
u64 m_event_fifo_id = 0; u64 m_event_fifo_id = 0;
std::mutex m_ts_write_lock; std::mutex m_ts_write_lock;
// Event objects created from other threads.
// The time value of each Event here is a cycles_into_future value.
Common::SPSCQueue<Event> m_ts_queue; Common::SPSCQueue<Event> m_ts_queue;
float m_last_oc_factor = 0.0f; float m_last_oc_factor = 0.0f;

12
Source/UnitTests/Core/CoreTimingTest.cpp
View File

@ -305,17 +305,9 @@ TEST(CoreTiming, ScheduleIntoPast)
AdvanceAndCheck(system, 0, MAX_SLICE_LENGTH, 1000); // Run cb_chain into late cb_a AdvanceAndCheck(system, 0, MAX_SLICE_LENGTH, 1000); // Run cb_chain into late cb_a
// Schedule late from wrong thread // Schedule directly into the past from wrong thread.
// The problem with scheduling CPU events from outside the CPU Thread is that g_global_timer
// is not reliable outside the CPU Thread. It's possible for the other thread to sample the
// global timer right before the timer is updated by Advance() then submit a new event using
// the stale value, i.e. effectively half-way through the previous slice.
// NOTE: We're only testing that the scheduler doesn't break, not whether this makes sense.
Core::UndeclareAsCPUThread(); Core::UndeclareAsCPUThread();
auto& core_timing_globals = core_timing.GetGlobals(); core_timing.ScheduleEvent(-1000, cb_b, CB_IDS[1], CoreTiming::FromThread::NON_CPU);
core_timing_globals.global_timer -= 1000;
core_timing.ScheduleEvent(0, cb_b, CB_IDS[1], CoreTiming::FromThread::NON_CPU);
core_timing_globals.global_timer += 1000;
Core::DeclareAsCPUThread(); Core::DeclareAsCPUThread();
AdvanceAndCheck(system, 1, MAX_SLICE_LENGTH, MAX_SLICE_LENGTH + 1000); AdvanceAndCheck(system, 1, MAX_SLICE_LENGTH, MAX_SLICE_LENGTH + 1000);