Merge pull request #13535 from m-brodschi/mihaib/fix-shutdown-crash

Core, VideoCommon: Fix crash at shutdown due to destructor order

8 files changed, 25 insertions, 20 deletions

diff --git a/Source/Core/Core/Core.cpp b/Source/Core/Core/Core.cpp
index 323ea899d1..ead9670fcf 100644
--- a/Source/Core/Core/Core.cpp
+++ b/Source/Core/Core/Core.cpp
@@ -278,7 +278,7 @@ void Stop(Core::System& system)  // - Hammertime!
 
   s_state.store(State::Stopping);
 
-  CallOnStateChangedCallbacks(State::Stopping);
+  NotifyStateChanged(State::Stopping);
 
   // Dump left over jobs
   HostDispatchJobs(system);
@@ -467,11 +467,11 @@ static void FifoPlayerThread(Core::System& system, const std::optional<std::stri
 static void EmuThread(Core::System& system, std::unique_ptr<BootParameters> boot,
                       WindowSystemInfo wsi)
 {
-  CallOnStateChangedCallbacks(State::Starting);
+  NotifyStateChanged(State::Starting);
   Common::ScopeGuard flag_guard{[] {
     s_state.store(State::Uninitialized);
 
-    CallOnStateChangedCallbacks(State::Uninitialized);
+    NotifyStateChanged(State::Uninitialized);
 
     INFO_LOG_FMT(CONSOLE, "Stop\t\t---- Shutdown complete ----");
   }};
@@ -711,7 +711,7 @@ void SetState(Core::System& system, State state, bool report_state_change,
   // Certain callers only change the state momentarily. Sending a callback for them causes
   // unwanted updates, such as the Pause/Play button flickering between states on frame advance.
   if (report_state_change)
-    CallOnStateChangedCallbacks(GetState(system));
+    NotifyStateChanged(GetState(system));
 }
 
 State GetState(Core::System& system)
@@ -877,7 +877,7 @@ void Callback_NewField(Core::System& system)
     {
       s_frame_step = false;
       system.GetCPU().Break();
-      CallOnStateChangedCallbacks(Core::GetState(system));
+      NotifyStateChanged(Core::GetState(system));
     }
   }
 
@@ -942,13 +942,14 @@ bool RemoveOnStateChangedCallback(int* handle)
   return false;
 }
 
-void CallOnStateChangedCallbacks(Core::State state)
+void NotifyStateChanged(Core::State state)
 {
   for (const StateChangedCallbackFunc& on_state_changed_callback : s_on_state_changed_callbacks)
   {
     if (on_state_changed_callback)
       on_state_changed_callback(state);
   }
+  g_perf_metrics.OnEmulationStateChanged(state);
 }
 
 void UpdateWantDeterminism(Core::System& system, bool initial)
diff --git a/Source/Core/Core/Core.h b/Source/Core/Core/Core.h
index eca18cf3be..3e946a8140 100644
--- a/Source/Core/Core/Core.h
+++ b/Source/Core/Core/Core.h
@@ -167,7 +167,7 @@ using StateChangedCallbackFunc = std::function<void(Core::State)>;
 int AddOnStateChangedCallback(StateChangedCallbackFunc callback);
 // Also invalidates the handle
 bool RemoveOnStateChangedCallback(int* handle);
-void CallOnStateChangedCallbacks(Core::State state);
+void NotifyStateChanged(Core::State state);
 
 // Run on the Host thread when the factors change. [NOT THREADSAFE]
 void UpdateWantDeterminism(Core::System& system, bool initial = false);
diff --git a/Source/Core/Core/HW/CPU.cpp b/Source/Core/Core/HW/CPU.cpp
index 5a21e5e3c8..9ead105322 100644
--- a/Source/Core/Core/HW/CPU.cpp
+++ b/Source/Core/Core/HW/CPU.cpp
@@ -348,7 +348,7 @@ void CPUManager::Break()
 void CPUManager::Continue()
 {
   SetStepping(false);
-  Core::CallOnStateChangedCallbacks(Core::State::Running);
+  Core::NotifyStateChanged(Core::State::Running);
 }
 
 bool CPUManager::PauseAndLock(bool do_lock, bool unpause_on_unlock, bool control_adjacent)
diff --git a/Source/Core/Core/PowerPC/GDBStub.cpp b/Source/Core/Core/PowerPC/GDBStub.cpp
index c9875a99db..a339ad251a 100644
--- a/Source/Core/Core/PowerPC/GDBStub.cpp
+++ b/Source/Core/Core/PowerPC/GDBStub.cpp
@@ -868,7 +868,7 @@ static void Step()
 {
   auto& system = Core::System::GetInstance();
   system.GetCPU().SetStepping(true);
-  Core::CallOnStateChangedCallbacks(Core::State::Paused);
+  Core::NotifyStateChanged(Core::State::Paused);
 }
 
 static bool AddBreakpoint(BreakpointType type, u32 addr, u32 len)
diff --git a/Source/Core/VideoCommon/PerformanceMetrics.cpp b/Source/Core/VideoCommon/PerformanceMetrics.cpp
index 8d18b846ef..520f9d8b95 100644
--- a/Source/Core/VideoCommon/PerformanceMetrics.cpp
+++ b/Source/Core/VideoCommon/PerformanceMetrics.cpp
@@ -35,6 +35,12 @@ void PerformanceMetrics::CountVBlank()
   m_vps_counter.Count();
 }
 
+void PerformanceMetrics::OnEmulationStateChanged([[maybe_unused]] Core::State state)
+{
+  m_fps_counter.InvalidateLastTime();
+  m_vps_counter.InvalidateLastTime();
+}
+
 void PerformanceMetrics::CountThrottleSleep(DT sleep)
 {
   m_time_sleeping += sleep;
diff --git a/Source/Core/VideoCommon/PerformanceMetrics.h b/Source/Core/VideoCommon/PerformanceMetrics.h
index 7beac394dc..bca6372d18 100644
--- a/Source/Core/VideoCommon/PerformanceMetrics.h
+++ b/Source/Core/VideoCommon/PerformanceMetrics.h
@@ -7,6 +7,7 @@
 #include <deque>
 
 #include "Common/CommonTypes.h"
+#include "Core/Core.h"
 #include "VideoCommon/PerformanceTracker.h"
 
 namespace Core
@@ -29,6 +30,7 @@ public:
 
   void CountFrame();
   void CountVBlank();
+  void OnEmulationStateChanged(Core::State state);
 
   // Call from CPU thread.
   void CountThrottleSleep(DT sleep);
diff --git a/Source/Core/VideoCommon/PerformanceTracker.cpp b/Source/Core/VideoCommon/PerformanceTracker.cpp
index f61f8fe63e..7113f388d1 100644
--- a/Source/Core/VideoCommon/PerformanceTracker.cpp
+++ b/Source/Core/VideoCommon/PerformanceTracker.cpp
@@ -23,17 +23,9 @@ PerformanceTracker::PerformanceTracker(const std::optional<std::string> log_name
                                        const std::optional<DT> sample_window_duration)
     : m_log_name{log_name}, m_sample_window_duration{sample_window_duration}
 {
-  m_on_state_changed_handle =
-      Core::AddOnStateChangedCallback([this](Core::State state) { m_is_last_time_sane = false; });
-
   Reset();
 }
 
-PerformanceTracker::~PerformanceTracker()
-{
-  Core::RemoveOnStateChangedCallback(&m_on_state_changed_handle);
-}
-
 void PerformanceTracker::Reset()
 {
   m_raw_dts.Clear();
@@ -135,6 +127,11 @@ DT PerformanceTracker::GetLastRawDt() const
   return m_last_raw_dt;
 }
 
+void PerformanceTracker::InvalidateLastTime()
+{
+  m_is_last_time_sane = false;
+}
+
 void PerformanceTracker::ImPlotPlotLines(const char* label) const
 {
   // "quality" graph uses twice as many points.
diff --git a/Source/Core/VideoCommon/PerformanceTracker.h b/Source/Core/VideoCommon/PerformanceTracker.h
index fac6435101..358de676b4 100644
--- a/Source/Core/VideoCommon/PerformanceTracker.h
+++ b/Source/Core/VideoCommon/PerformanceTracker.h
@@ -16,7 +16,7 @@ class PerformanceTracker
 public:
   PerformanceTracker(const std::optional<std::string> log_name = std::nullopt,
                      const std::optional<DT> sample_window_duration = std::nullopt);
-  ~PerformanceTracker();
+  ~PerformanceTracker() = default;
 
   PerformanceTracker(const PerformanceTracker&) = delete;
   PerformanceTracker& operator=(const PerformanceTracker&) = delete;
@@ -39,6 +39,7 @@ public:
   DT GetDtAvg() const;
   DT GetDtStd() const;
   DT GetLastRawDt() const;
+  void InvalidateLastTime();
 
 private:
   void LogRenderTimeToFile(DT val);
@@ -47,8 +48,6 @@ private:
   void PushFront(DT value);
   void PopBack();
 
-  int m_on_state_changed_handle;
-
   // Name of log file and file stream
   std::optional<std::string> m_log_name;
   std::ofstream m_bench_file;