Debugger: Lock startup/shutdown for threadsafety.

Otherwise things can get freed while we're trying to inspect them.
hrydgard · Jun 6, 2018 · e313a9b · e313a9b
1 parent 77609bb
commit e313a9b
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 23 deletions.
diff --git 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<Core_ShutdownFunc> shutdownFuncs;
+static std::set<CoreLifecycleFunc> 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
@@ -38,9 +38,18 @@ void Core_UpdateSingleStep();
 // Changes every time we enter stepping.
 int Core_GetSteppingCounter();
 
-typedef void (* Core_ShutdownFunc)();
+enum class CoreLifecycle {
-void Core_ListenShutdown(Core_ShutdownFunc func);
+	STARTING,
-void Core_NotifyShutdown();
+	// 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/HLE/sceIo.cpp b/Core/HLE/sceIo.cpp
@@ -496,10 +496,12 @@ static void __IoManagerThread() {
 	}
 }
 
-static void __IoWakeManager() {
+static void __IoWakeManager(CoreLifecycle stage) {
 	// Ping the thread so that it knows to check coreState.
-	ioManagerThreadEnabled = false;
+	if (stage == CoreLifecycle::STOPPING) {
-	ioManager.FinishEventLoop();
+		ioManagerThreadEnabled = false;
+		ioManager.FinishEventLoop();
+	}
 }
 
 static void __IoVblank() {
@@ -586,7 +588,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
@@ -317,6 +317,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) {
@@ -331,6 +332,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;
@@ -361,6 +363,9 @@ bool PSP_InitUpdate(std::string *error_string) {
 
 	pspIsInited = GPU_IsReady();
 	pspIsIniting = !pspIsInited;
+	if (pspIsInited) {
+		Core_NotifyLifecycle(CoreLifecycle::START_COMPLETE);
+	}
 	return pspIsInited;
 }
 
@@ -386,17 +391,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.
+	if (pspIsIniting)
-	pspIsQuitting = true;
+		Core_NotifyLifecycle(CoreLifecycle::START_COMPLETE);
-	if (coreState == CORE_RUNNING)
+	Core_NotifyLifecycle(CoreLifecycle::STOPPING);
-		Core_UpdateState(CORE_ERROR);
-	Core_NotifyShutdown();
 	CPU_Shutdown();
 	GPU_Shutdown();
 	g_paramSFO.Clear();
@@ -406,6 +414,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
@@ -197,10 +197,12 @@ void ResumeFromStepping() {
 	SetPauseAction(PAUSE_CONTINUE, false);
 }
 
-void ForceUnpause() {
+void ForceUnpause(CoreLifecycle stage) {
-	SetPauseAction(PAUSE_CONTINUE, false);
+	if (stage == CoreLifecycle::STOPPING) {
-	actionComplete = true;
+		SetPauseAction(PAUSE_CONTINUE, false);
-	actionWait.notify_all();
+		actionComplete = true;
+		actionWait.notify_all();
+	}
 }
 
 }  // namespace
diff --git a/GPU/Debugger/Stepping.h b/GPU/Debugger/Stepping.h
@@ -20,6 +20,7 @@
 #include <functional>
 
 #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
@@ -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;