diff --git a/Core/Core.cpp b/Core/Core.cpp index 0c5b37ddc185..722fed54d2fc 100644 --- a/Core/Core.cpp +++ b/Core/Core.cpp @@ -55,7 +55,7 @@ static std::condition_variable m_InactiveCond; static std::mutex m_hInactiveMutex; static bool singleStepPending = false; static int steppingCounter = 0; -static std::set shutdownFuncs; +static std::set shutdownFuncs; static bool windowHidden = false; static double lastActivity = 0.0; static double lastKeepAwake = 0.0; @@ -76,13 +76,13 @@ void Core_NotifyActivity() { lastActivity = time_now_d(); } -void Core_ListenShutdown(Core_ShutdownFunc func) { +void Core_ListenLifecycle(CoreLifecycleFunc func) { shutdownFuncs.insert(func); } -void Core_NotifyShutdown() { +void Core_NotifyLifecycle(CoreLifecycle stage) { for (auto it = shutdownFuncs.begin(); it != shutdownFuncs.end(); ++it) { - (*it)(); + (*it)(stage); } } @@ -98,7 +98,6 @@ void Core_Halt(const char *msg) { void Core_Stop() { Core_UpdateState(CORE_POWERDOWN); - Core_NotifyShutdown(); m_StepCond.notify_all(); } diff --git a/Core/Core.h b/Core/Core.h index 1c98ccdc4966..fe881f2aa5cb 100644 --- a/Core/Core.h +++ b/Core/Core.h @@ -38,9 +38,18 @@ void Core_UpdateSingleStep(); // Changes every time we enter stepping. int Core_GetSteppingCounter(); -typedef void (* Core_ShutdownFunc)(); -void Core_ListenShutdown(Core_ShutdownFunc func); -void Core_NotifyShutdown(); +enum class CoreLifecycle { + STARTING, + // Note: includes failure cases. Guaranteed call after STARTING. + START_COMPLETE, + STOPPING, + // Guaranteed call after STOPPING. + STOPPED, +}; + +typedef void (* CoreLifecycleFunc)(CoreLifecycle stage); +void Core_ListenLifecycle(CoreLifecycleFunc func); +void Core_NotifyLifecycle(CoreLifecycle stage); void Core_Halt(const char *msg); bool Core_IsStepping(); diff --git a/Core/Debugger/WebSocket.cpp b/Core/Debugger/WebSocket.cpp index e99253b81353..5b323bce4bce 100644 --- a/Core/Debugger/WebSocket.cpp +++ b/Core/Debugger/WebSocket.cpp @@ -68,12 +68,43 @@ static volatile bool stopRequested = false; static std::mutex stopLock; static std::condition_variable stopCond; +// Prevent threading surprises and obscure crashes by locking startup/shutdown. +static bool lifecycleLockSetup = false; +static std::mutex lifecycleLock; + static void UpdateConnected(int delta) { std::lock_guard guard(stopLock); debuggersConnected += delta; stopCond.notify_all(); } +static void WebSocketNotifyLifecycle(CoreLifecycle stage) { + switch (stage) { + case CoreLifecycle::STARTING: + case CoreLifecycle::STOPPING: + if (debuggersConnected > 0) { + DEBUG_LOG(SYSTEM, "Waiting for debugger to complete on shutdown"); + } + lifecycleLock.lock(); + break; + + case CoreLifecycle::START_COMPLETE: + case CoreLifecycle::STOPPED: + lifecycleLock.unlock(); + if (debuggersConnected > 0) { + DEBUG_LOG(SYSTEM, "Debugger ready for shutdown"); + } + break; + } +} + +static void SetupDebuggerLock() { + if (!lifecycleLockSetup) { + Core_ListenLifecycle(&WebSocketNotifyLifecycle); + lifecycleLockSetup = true; + } +} + void HandleDebuggerRequest(const http::Request &request) { net::WebSocketServer *ws = net::WebSocketServer::CreateAsUpgrade(request, "debugger.ppsspp.org"); if (!ws) @@ -81,6 +112,7 @@ void HandleDebuggerRequest(const http::Request &request) { setCurrentThreadName("Debugger"); UpdateConnected(1); + SetupDebuggerLock(); LogBroadcaster logger; GameBroadcaster game; @@ -89,6 +121,7 @@ void HandleDebuggerRequest(const http::Request &request) { std::unordered_map eventHandlers; std::vector subscriberData; for (auto info : subscribers) { + std::lock_guard guard(lifecycleLock); subscriberData.push_back(info.init(eventHandlers)); } @@ -109,6 +142,7 @@ void HandleDebuggerRequest(const http::Request &request) { DebuggerRequest req(event, ws, root); auto eventFunc = eventHandlers.find(event); if (eventFunc != eventHandlers.end()) { + std::lock_guard guard(lifecycleLock); eventFunc->second(req); req.Finish(); } else { @@ -120,6 +154,7 @@ void HandleDebuggerRequest(const http::Request &request) { }); while (ws->Process(1.0f / 60.0f)) { + std::lock_guard guard(lifecycleLock); // These send events that aren't just responses to requests. logger.Broadcast(ws); game.Broadcast(ws); @@ -130,6 +165,7 @@ void HandleDebuggerRequest(const http::Request &request) { } } + std::lock_guard guard(lifecycleLock); for (size_t i = 0; i < subscribers.size(); ++i) { if (subscribers[i].shutdown) { subscribers[i].shutdown(subscriberData[i]); diff --git a/Core/HLE/sceIo.cpp b/Core/HLE/sceIo.cpp index 8935e0dc3c4e..4ab0f25fb662 100644 --- a/Core/HLE/sceIo.cpp +++ b/Core/HLE/sceIo.cpp @@ -497,10 +497,12 @@ static void __IoManagerThread() { } } -static void __IoWakeManager() { +static void __IoWakeManager(CoreLifecycle stage) { // Ping the thread so that it knows to check coreState. - ioManagerThreadEnabled = false; - ioManager.FinishEventLoop(); + if (stage == CoreLifecycle::STOPPING) { + ioManagerThreadEnabled = false; + ioManager.FinishEventLoop(); + } } static void __IoVblank() { @@ -587,7 +589,7 @@ void __IoInit() { ioManagerThreadEnabled = g_Config.bSeparateIOThread; ioManager.SetThreadEnabled(ioManagerThreadEnabled); if (ioManagerThreadEnabled) { - Core_ListenShutdown(&__IoWakeManager); + Core_ListenLifecycle(&__IoWakeManager); ioManagerThread = new std::thread(&__IoManagerThread); ioManagerThread->detach(); } diff --git a/Core/System.cpp b/Core/System.cpp index 873e5d2c77be..9f5c0128bf4f 100644 --- a/Core/System.cpp +++ b/Core/System.cpp @@ -306,6 +306,7 @@ bool PSP_InitStart(const CoreParameter &coreParam, std::string *error_string) { INFO_LOG(BOOT, "PPSSPP %s", PPSSPP_GIT_VERSION); #endif + Core_NotifyLifecycle(CoreLifecycle::STARTING); GraphicsContext *temp = coreParameter.graphicsContext; coreParameter = coreParam; if (coreParameter.graphicsContext == nullptr) { @@ -320,6 +321,7 @@ bool PSP_InitStart(const CoreParameter &coreParam, std::string *error_string) { *error_string = coreParameter.errorString; bool success = coreParameter.fileToStart != ""; if (!success) { + Core_NotifyLifecycle(CoreLifecycle::START_COMPLETE); pspIsIniting = false; } return success; @@ -346,6 +348,9 @@ bool PSP_InitUpdate(std::string *error_string) { } pspIsInited = success && GPU_IsReady(); pspIsIniting = success && !pspIsInited; + if (!pspIsIniting) { + Core_NotifyLifecycle(CoreLifecycle::START_COMPLETE); + } return !success || pspIsInited; } @@ -375,17 +380,20 @@ void PSP_Shutdown() { return; } + // Make sure things know right away that PSP memory, etc. is going away. + pspIsQuitting = true; + if (coreState == CORE_RUNNING) + Core_UpdateState(CORE_ERROR); + #ifndef MOBILE_DEVICE if (g_Config.bFuncHashMap) { MIPSAnalyst::StoreHashMap(); } #endif - // Make sure things know right away that PSP memory, etc. is going away. - pspIsQuitting = true; - if (coreState == CORE_RUNNING) - Core_UpdateState(CORE_ERROR); - Core_NotifyShutdown(); + if (pspIsIniting) + Core_NotifyLifecycle(CoreLifecycle::START_COMPLETE); + Core_NotifyLifecycle(CoreLifecycle::STOPPING); CPU_Shutdown(); GPU_Shutdown(); g_paramSFO.Clear(); @@ -395,6 +403,7 @@ void PSP_Shutdown() { pspIsIniting = false; pspIsQuitting = false; g_Config.unloadGameConfig(); + Core_NotifyLifecycle(CoreLifecycle::STOPPED); } void PSP_BeginHostFrame() { diff --git a/GPU/Debugger/Stepping.cpp b/GPU/Debugger/Stepping.cpp index c765d15b7688..ea93778cff42 100644 --- a/GPU/Debugger/Stepping.cpp +++ b/GPU/Debugger/Stepping.cpp @@ -197,10 +197,12 @@ void ResumeFromStepping() { SetPauseAction(PAUSE_CONTINUE, false); } -void ForceUnpause() { - SetPauseAction(PAUSE_CONTINUE, false); - actionComplete = true; - actionWait.notify_all(); +void ForceUnpause(CoreLifecycle stage) { + if (stage == CoreLifecycle::STOPPING) { + SetPauseAction(PAUSE_CONTINUE, false); + actionComplete = true; + actionWait.notify_all(); + } } } // namespace diff --git a/GPU/Debugger/Stepping.h b/GPU/Debugger/Stepping.h index cb651af23428..3e9b5086ccbc 100644 --- a/GPU/Debugger/Stepping.h +++ b/GPU/Debugger/Stepping.h @@ -20,6 +20,7 @@ #include #include "Common/CommonTypes.h" +#include "Core/Core.h" #include "GPU/Common/GPUDebugInterface.h" namespace GPUStepping { @@ -37,5 +38,5 @@ namespace GPUStepping { bool GPU_SetCmdValue(u32 op); void ResumeFromStepping(); - void ForceUnpause(); + void ForceUnpause(CoreLifecycle stage); }; diff --git a/Windows/GEDebugger/GEDebugger.cpp b/Windows/GEDebugger/GEDebugger.cpp index 2b1639971f7e..e77238ba3b94 100644 --- a/Windows/GEDebugger/GEDebugger.cpp +++ b/Windows/GEDebugger/GEDebugger.cpp @@ -67,7 +67,7 @@ void CGEDebugger::Init() { CGEDebugger::CGEDebugger(HINSTANCE _hInstance, HWND _hParent) : Dialog((LPCSTR)IDD_GEDEBUGGER, _hInstance, _hParent) { GPUBreakpoints::Init(); - Core_ListenShutdown(ForceUnpause); + Core_ListenLifecycle(ForceUnpause); // minimum size = a little more than the default RECT windowRect;