From 8f3f14d323032e511c5b40bd14247f28440a2e86 Mon Sep 17 00:00:00 2001 From: Sour Date: Fri, 21 Jan 2022 21:19:44 -0500 Subject: [PATCH] Fixed more issues with debugger when reloading games, etc. --- Core/Debugger/Debugger.cpp | 5 ++ Core/Debugger/Debugger.h | 1 + Core/Shared/Emulator.cpp | 102 ++++++++++++---------------- Core/Shared/Emulator.h | 8 +-- Utilities/Utilities.vcxproj | 1 + Utilities/Utilities.vcxproj.filters | 3 + Utilities/safe_ptr.h | 52 ++++++++++++++ 7 files changed, 110 insertions(+), 62 deletions(-) create mode 100644 Utilities/safe_ptr.h diff --git a/Core/Debugger/Debugger.cpp b/Core/Debugger/Debugger.cpp index 99e9efa2..9aea8b69 100644 --- a/Core/Debugger/Debugger.cpp +++ b/Core/Debugger/Debugger.cpp @@ -373,6 +373,11 @@ void Debugger::BreakRequest(bool release) } } +void Debugger::ResetSuspendCounter() +{ + _suspendRequestCount = 0; +} + void Debugger::SuspendDebugger(bool release) { if(release) { diff --git a/Core/Debugger/Debugger.h b/Core/Debugger/Debugger.h index 502a0a91..3080b32b 100644 --- a/Core/Debugger/Debugger.h +++ b/Core/Debugger/Debugger.h @@ -104,6 +104,7 @@ public: bool HasBreakRequest(); void BreakRequest(bool release); + void ResetSuspendCounter(); void SuspendDebugger(bool release); void BreakImmediately(BreakSource source); diff --git a/Core/Shared/Emulator.cpp b/Core/Shared/Emulator.cpp index ab5d66ce..77bfa8bd 100644 --- a/Core/Shared/Emulator.cpp +++ b/Core/Shared/Emulator.cpp @@ -258,26 +258,6 @@ void Emulator::RunSingleFrame() _controlManager->UpdateControlDevices();*/ } -shared_ptr Emulator::SafeGetDebugger() -{ - auto lock = _debuggerLock.AcquireSafe(); - shared_ptr debugger = _debugger; - return debugger; -} - -void Emulator::SafeResetDebugger(Debugger* dbg = nullptr) -{ - if(_emulationThreadId == std::this_thread::get_id()) { - auto dbgLock = _debuggerLock.AcquireSafe(); - _debugger.reset(dbg); - } else { - //Need to pause emulator to change _debugger (when not called from the emulation thread) - auto emuLock = AcquireLock(); - auto dbgLock = _debuggerLock.AcquireSafe(); - _debugger.reset(dbg); - } -} - void Emulator::Stop(bool sendNotification) { BlockDebuggerRequests(); @@ -286,11 +266,7 @@ void Emulator::Stop(bool sendNotification) _notificationManager->SendNotification(ConsoleNotificationType::BeforeGameUnload); - shared_ptr debugger = SafeGetDebugger(); - if(debugger) { - debugger->SuspendDebugger(false); - debugger->Run(); - } + ResetDebugger(); if(_emuThread) { _emuThread->join(); @@ -306,10 +282,6 @@ void Emulator::Stop(bool sendNotification) _notificationManager->SendNotification(ConsoleNotificationType::BeforeEmulationStop); } - //Make sure we release both pointers to destroy the debugger before everything else - _debugger.reset(); - debugger.reset(); - _videoDecoder->StopThread(); _videoRenderer->StopThread(); _rewindManager.reset(); @@ -359,12 +331,15 @@ bool Emulator::LoadRom(VirtualFile romFile, VirtualFile patchFile, bool stopRom, return false; } - BlockDebuggerRequests(); - - //Keep a reference to the original debugger, and reset the member to avoid any calls to the debugger by the init process - shared_ptr debugger = SafeGetDebugger(); + //Keep a reference to the original debugger + shared_ptr debugger = _debugger.lock(); bool debuggerActive = debugger != nullptr; - SafeResetDebugger(); + + //Unset _debugger to ensure nothing calls the debugger while initializing the new rom + { + auto dbgLock = _debuggerLock.AcquireSafe(); + ResetDebugger(); + } if(patchFile.IsValid()) { if(romFile.ApplyPatch(patchFile)) { @@ -415,7 +390,8 @@ bool Emulator::LoadRom(VirtualFile romFile, VirtualFile patchFile, bool stopRom, if(result != LoadRomResult::Success) { MessageManager::DisplayMessage("Error", "CouldNotLoadFile", romFile.GetFileName()); - _debugger = debugger; + _debugger.reset(debugger); + debugger->ResetSuspendCounter(); _allowDebuggerRequest = true; return false; } @@ -605,7 +581,7 @@ double Emulator::GetFrameDelay() void Emulator::PauseOnNextFrame() { - shared_ptr debugger = _debugger; + shared_ptr debugger = _debugger.lock(); if(debugger) { switch(GetConsoleType()) { case ConsoleType::Snes: @@ -629,7 +605,7 @@ void Emulator::PauseOnNextFrame() void Emulator::Pause() { - shared_ptr debugger = _debugger; + shared_ptr debugger = _debugger.lock(); if(debugger) { switch(GetConsoleType()) { case ConsoleType::Snes: @@ -652,7 +628,7 @@ void Emulator::Pause() void Emulator::Resume() { - shared_ptr debugger = _debugger; + shared_ptr debugger = _debugger.lock(); if(debugger) { debugger->Run(); } else { @@ -662,7 +638,7 @@ void Emulator::Resume() bool Emulator::IsPaused() { - shared_ptr debugger = _debugger; + shared_ptr debugger = _debugger.lock(); if(debugger) { return debugger->IsExecutionStopped(); } else { @@ -701,7 +677,7 @@ EmulatorLock Emulator::AcquireLock() void Emulator::Lock() { - shared_ptr debugger = _debugger; + shared_ptr debugger = _debugger.lock(); if(debugger) { debugger->SuspendDebugger(false); } @@ -712,7 +688,7 @@ void Emulator::Lock() void Emulator::Unlock() { - shared_ptr debugger = _debugger; + shared_ptr debugger = _debugger.lock(); if(debugger) { debugger->SuspendDebugger(true); } @@ -737,7 +713,7 @@ void Emulator::WaitForLock() //Spin wait until we are allowed to start again while(_lockCounter > 0 && !_stopFlag) {} - shared_ptr debugger = _debugger; + shared_ptr debugger = _debugger.lock(); if(debugger) { while(debugger->HasBreakRequest() && !_stopFlag) {} } @@ -870,24 +846,41 @@ void Emulator::BlockDebuggerRequests() Emulator::DebuggerRequest Emulator::GetDebugger(bool autoInit) { if(IsRunning() && _allowDebuggerRequest) { - auto lock = _debuggerLock.AcquireSafe(); - if(IsRunning() && _allowDebuggerRequest) { - if(!_debugger && autoInit) { - InitDebugger(); - } - return DebuggerRequest(this); + if(!_debugger && autoInit) { + InitDebugger(); } + return DebuggerRequest(this); } return DebuggerRequest(nullptr); } +void Emulator::ResetDebugger(Debugger* dbg) +{ + BlockDebuggerRequests(); + + shared_ptr currentDbg = _debugger.lock(); + if(currentDbg) { + currentDbg->SuspendDebugger(false); + } + + if(_emulationThreadId == std::this_thread::get_id()) { + _debugger.reset(dbg); + } else { + //Need to pause emulator to change _debugger (when not called from the emulation thread) + auto emuLock = AcquireLock(); + _debugger.reset(dbg); + } + + _allowDebuggerRequest = true; +} + void Emulator::InitDebugger() { if(!_debugger) { //Lock to make sure we don't try to start debuggers in 2 separate threads at once auto lock = _debuggerLock.AcquireSafe(); if(!_debugger) { - SafeResetDebugger(new Debugger(this, _console.get())); + ResetDebugger(new Debugger(this, _console.get())); } } } @@ -898,23 +891,16 @@ void Emulator::StopDebugger() _paused = IsPaused(); if(_debugger) { - BlockDebuggerRequests(); - auto lock = _debuggerLock.AcquireSafe(); if(_debugger) { - _debugger->SuspendDebugger(false); - Lock(); - _debugger.reset(); - Unlock(); + ResetDebugger(); } - - _allowDebuggerRequest = true; } } bool Emulator::IsDebugging() { - return SafeGetDebugger() != nullptr; + return !!_debugger; } thread::id Emulator::GetEmulationThreadId() diff --git a/Core/Shared/Emulator.h b/Core/Shared/Emulator.h index 746b0a1f..02820f83 100644 --- a/Core/Shared/Emulator.h +++ b/Core/Shared/Emulator.h @@ -6,6 +6,7 @@ #include "Core/Shared/Interfaces/IConsole.h" #include "Core/Shared/Audio/AudioPlayerTypes.h" #include "Utilities/Timer.h" +#include "Utilities/safe_ptr.h" #include "Utilities/SimpleLock.h" #include "Utilities/VirtualFile.h" @@ -57,7 +58,7 @@ private: shared_ptr _shortcutKeyHandler; shared_ptr _rewindManager; - shared_ptr _debugger; + safe_ptr _debugger; shared_ptr _systemActionManager; const unique_ptr _settings; @@ -108,8 +109,7 @@ private: void RunFrameWithRunAhead(); void BlockDebuggerRequests(); - shared_ptr SafeGetDebugger(); - void SafeResetDebugger(Debugger* dbg); + void ResetDebugger(Debugger* dbg = nullptr); public: class DebuggerRequest @@ -123,7 +123,7 @@ public: { if(emu) { _emu = emu; - _debugger = _emu->_debugger; + _debugger = _emu->_debugger.lock(); _emu->_debugRequestCount++; } } diff --git a/Utilities/Utilities.vcxproj b/Utilities/Utilities.vcxproj index 8ec6f9a5..6d71ecfa 100644 --- a/Utilities/Utilities.vcxproj +++ b/Utilities/Utilities.vcxproj @@ -465,6 +465,7 @@ + diff --git a/Utilities/Utilities.vcxproj.filters b/Utilities/Utilities.vcxproj.filters index e4abfe0b..7322415b 100644 --- a/Utilities/Utilities.vcxproj.filters +++ b/Utilities/Utilities.vcxproj.filters @@ -229,6 +229,9 @@ Misc + + Misc + diff --git a/Utilities/safe_ptr.h b/Utilities/safe_ptr.h new file mode 100644 index 00000000..1ecf790e --- /dev/null +++ b/Utilities/safe_ptr.h @@ -0,0 +1,52 @@ +#include "stdafx.h" + +template +class safe_ptr +{ +private: + T* _ptr = nullptr; + shared_ptr _shared; + SimpleLock _lock; + +public: + safe_ptr(T* ptr = nullptr) + { + reset(ptr); + } + + ~safe_ptr() + { + //shared_ptr will free the instance as needed + } + + shared_ptr lock() + { + auto lock = _lock.AcquireSafe(); + return _shared; + } + + void reset(T* ptr = nullptr) + { + auto lock = _lock.AcquireSafe(); + _ptr = ptr; + _shared.reset(ptr); + } + + void reset(shared_ptr ptr) + { + auto lock = _lock.AcquireSafe(); + _ptr = ptr.get(); + _shared = ptr; + } + + operator bool() const + { + return _ptr != nullptr; + } + + T* operator->() const + { + //Accessing the pointer this way is fast, but not thread-safe + return _ptr; + } +};