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

Previously, PerformanceTracker registered a callback to be updated on
emulation state changes. PerformanceTrackers live in a global variable
(g_perf_metrics) within libvideocommon. The callback was stored in a
global variable in libcore. This created a race condition at shutdown
between these libraries, when the PerfTracker's destructor tried to
unregister the callback.
Notify the PerfTracker directly from libcore, without callbacks, since
Core.cpp already references g_perf_metrics explicitly. Also rename
Core::CallOnStateChangedCallbacks to NotifyStateChanged to better
reflect what it's doing.
This commit is contained in:
Mihai Brodschi 2025-04-27 17:33:23 +03:00
parent 8ee64a84c7
commit bad78cfed4
8 changed files with 25 additions and 20 deletions

View file

@ -278,7 +278,7 @@ void Stop(Core::System& system) // - Hammertime!
s_state.store(State::Stopping); s_state.store(State::Stopping);
CallOnStateChangedCallbacks(State::Stopping); NotifyStateChanged(State::Stopping);
// Dump left over jobs // Dump left over jobs
HostDispatchJobs(system); 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, static void EmuThread(Core::System& system, std::unique_ptr<BootParameters> boot,
WindowSystemInfo wsi) WindowSystemInfo wsi)
{ {
CallOnStateChangedCallbacks(State::Starting); NotifyStateChanged(State::Starting);
Common::ScopeGuard flag_guard{[] { Common::ScopeGuard flag_guard{[] {
s_state.store(State::Uninitialized); s_state.store(State::Uninitialized);
CallOnStateChangedCallbacks(State::Uninitialized); NotifyStateChanged(State::Uninitialized);
INFO_LOG_FMT(CONSOLE, "Stop\t\t---- Shutdown complete ----"); 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 // 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. // unwanted updates, such as the Pause/Play button flickering between states on frame advance.
if (report_state_change) if (report_state_change)
CallOnStateChangedCallbacks(GetState(system)); NotifyStateChanged(GetState(system));
} }
State GetState(Core::System& system) State GetState(Core::System& system)
@ -877,7 +877,7 @@ void Callback_NewField(Core::System& system)
{ {
s_frame_step = false; s_frame_step = false;
system.GetCPU().Break(); system.GetCPU().Break();
CallOnStateChangedCallbacks(Core::GetState(system)); NotifyStateChanged(Core::GetState(system));
} }
} }
@ -942,13 +942,14 @@ bool RemoveOnStateChangedCallback(int* handle)
return false; return false;
} }
void CallOnStateChangedCallbacks(Core::State state) void NotifyStateChanged(Core::State state)
{ {
for (const StateChangedCallbackFunc& on_state_changed_callback : s_on_state_changed_callbacks) for (const StateChangedCallbackFunc& on_state_changed_callback : s_on_state_changed_callbacks)
{ {
if (on_state_changed_callback) if (on_state_changed_callback)
on_state_changed_callback(state); on_state_changed_callback(state);
} }
g_perf_metrics.OnEmulationStateChanged(state);
} }
void UpdateWantDeterminism(Core::System& system, bool initial) void UpdateWantDeterminism(Core::System& system, bool initial)

View file

@ -167,7 +167,7 @@ using StateChangedCallbackFunc = std::function<void(Core::State)>;
int AddOnStateChangedCallback(StateChangedCallbackFunc callback); int AddOnStateChangedCallback(StateChangedCallbackFunc callback);
// Also invalidates the handle // Also invalidates the handle
bool RemoveOnStateChangedCallback(int* 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] // Run on the Host thread when the factors change. [NOT THREADSAFE]
void UpdateWantDeterminism(Core::System& system, bool initial = false); void UpdateWantDeterminism(Core::System& system, bool initial = false);

View file

@ -348,7 +348,7 @@ void CPUManager::Break()
void CPUManager::Continue() void CPUManager::Continue()
{ {
SetStepping(false); 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) bool CPUManager::PauseAndLock(bool do_lock, bool unpause_on_unlock, bool control_adjacent)

View file

@ -868,7 +868,7 @@ static void Step()
{ {
auto& system = Core::System::GetInstance(); auto& system = Core::System::GetInstance();
system.GetCPU().SetStepping(true); system.GetCPU().SetStepping(true);
Core::CallOnStateChangedCallbacks(Core::State::Paused); Core::NotifyStateChanged(Core::State::Paused);
} }
static bool AddBreakpoint(BreakpointType type, u32 addr, u32 len) static bool AddBreakpoint(BreakpointType type, u32 addr, u32 len)

View file

@ -35,6 +35,12 @@ void PerformanceMetrics::CountVBlank()
m_vps_counter.Count(); 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) void PerformanceMetrics::CountThrottleSleep(DT sleep)
{ {
m_time_sleeping += sleep; m_time_sleeping += sleep;

View file

@ -7,6 +7,7 @@
#include <deque> #include <deque>
#include "Common/CommonTypes.h" #include "Common/CommonTypes.h"
#include "Core/Core.h"
#include "VideoCommon/PerformanceTracker.h" #include "VideoCommon/PerformanceTracker.h"
namespace Core namespace Core
@ -29,6 +30,7 @@ public:
void CountFrame(); void CountFrame();
void CountVBlank(); void CountVBlank();
void OnEmulationStateChanged(Core::State state);
// Call from CPU thread. // Call from CPU thread.
void CountThrottleSleep(DT sleep); void CountThrottleSleep(DT sleep);

View file

@ -23,17 +23,9 @@ PerformanceTracker::PerformanceTracker(const std::optional<std::string> log_name
const std::optional<DT> sample_window_duration) const std::optional<DT> sample_window_duration)
: m_log_name{log_name}, m_sample_window_duration{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(); Reset();
} }
PerformanceTracker::~PerformanceTracker()
{
Core::RemoveOnStateChangedCallback(&m_on_state_changed_handle);
}
void PerformanceTracker::Reset() void PerformanceTracker::Reset()
{ {
m_raw_dts.Clear(); m_raw_dts.Clear();
@ -135,6 +127,11 @@ DT PerformanceTracker::GetLastRawDt() const
return m_last_raw_dt; return m_last_raw_dt;
} }
void PerformanceTracker::InvalidateLastTime()
{
m_is_last_time_sane = false;
}
void PerformanceTracker::ImPlotPlotLines(const char* label) const void PerformanceTracker::ImPlotPlotLines(const char* label) const
{ {
// "quality" graph uses twice as many points. // "quality" graph uses twice as many points.

View file

@ -16,7 +16,7 @@ class PerformanceTracker
public: public:
PerformanceTracker(const std::optional<std::string> log_name = std::nullopt, PerformanceTracker(const std::optional<std::string> log_name = std::nullopt,
const std::optional<DT> sample_window_duration = std::nullopt); const std::optional<DT> sample_window_duration = std::nullopt);
~PerformanceTracker(); ~PerformanceTracker() = default;
PerformanceTracker(const PerformanceTracker&) = delete; PerformanceTracker(const PerformanceTracker&) = delete;
PerformanceTracker& operator=(const PerformanceTracker&) = delete; PerformanceTracker& operator=(const PerformanceTracker&) = delete;
@ -39,6 +39,7 @@ public:
DT GetDtAvg() const; DT GetDtAvg() const;
DT GetDtStd() const; DT GetDtStd() const;
DT GetLastRawDt() const; DT GetLastRawDt() const;
void InvalidateLastTime();
private: private:
void LogRenderTimeToFile(DT val); void LogRenderTimeToFile(DT val);
@ -47,8 +48,6 @@ private:
void PushFront(DT value); void PushFront(DT value);
void PopBack(); void PopBack();
int m_on_state_changed_handle;
// Name of log file and file stream // Name of log file and file stream
std::optional<std::string> m_log_name; std::optional<std::string> m_log_name;
std::ofstream m_bench_file; std::ofstream m_bench_file;