Merge pull request #13783 from jordan-woyak/schedule-event-from-thread-fix

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

3 files changed, 12 insertions, 13 deletions

diff --git a/Source/Core/Core/CoreTiming.cpp b/Source/Core/Core/CoreTiming.cpp
index 261fa454fd..d7ef5b6092 100644
--- a/Source/Core/Core/CoreTiming.cpp
+++ b/Source/Core/Core/CoreTiming.cpp
@@ -282,7 +282,7 @@ void CoreTimingManager::ScheduleEvent(s64 cycles_into_future, EventType* event_t
     }
 
     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()
 {
-  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++;
-    m_event_queue.emplace_back(std::move(ev));
+    ev.time += m_globals.global_timer;
+
     std::ranges::push_heap(m_event_queue, std::ranges::greater{});
   }
 }
diff --git a/Source/Core/Core/CoreTiming.h b/Source/Core/Core/CoreTiming.h
index 8761a45c95..84311291ec 100644
--- a/Source/Core/Core/CoreTiming.h
+++ b/Source/Core/Core/CoreTiming.h
@@ -186,6 +186,9 @@ private:
   std::vector<Event> m_event_queue;
   u64 m_event_fifo_id = 0;
   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;
 
   float m_last_oc_factor = 0.0f;
diff --git a/Source/UnitTests/Core/CoreTimingTest.cpp b/Source/UnitTests/Core/CoreTimingTest.cpp
index 67a4f4c716..3b57020ece 100644
--- a/Source/UnitTests/Core/CoreTimingTest.cpp
+++ b/Source/UnitTests/Core/CoreTimingTest.cpp
@@ -305,17 +305,9 @@ TEST(CoreTiming, ScheduleIntoPast)
 
   AdvanceAndCheck(system, 0, MAX_SLICE_LENGTH, 1000);  // Run cb_chain into late cb_a
 
-  // Schedule late 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.
+  // Schedule directly into the past from wrong thread.
   Core::UndeclareAsCPUThread();
-  auto& core_timing_globals = core_timing.GetGlobals();
-  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_timing.ScheduleEvent(-1000, cb_b, CB_IDS[1], CoreTiming::FromThread::NON_CPU);
   Core::DeclareAsCPUThread();
   AdvanceAndCheck(system, 1, MAX_SLICE_LENGTH, MAX_SLICE_LENGTH + 1000);