Vulkan/Debug: Various fixes / improvements to Thread

src/Pipeline/SpirvShaderDebugger.cpp

@@ -856,10 +856,10 @@ public:
 	static State *create(const Debugger *debugger, const char *name);
 	static void destroy(State *);
 
-	State(const Debugger *debugger, const char *stackBase, vk::dbg::Context::Lock &lock);
+	State(const Debugger *debugger, const char *stackBase);
 	~State();
 
-	void enter(vk::dbg::Context::Lock &lock, const char *name);
+	void enter(const char *name);
 	void exit();
 	void updateActiveLaneMask(int lane, bool enabled);
 	void updateLocation(bool isStep, vk::dbg::File::ID file, int line, int column);
@@ -905,8 +905,7 @@ public:
 
 SpirvShader::Impl::Debugger::State *SpirvShader::Impl::Debugger::State::create(const Debugger *debugger, const char *name)
 {
-	auto lock = debugger->ctx->lock();
-	return new State(debugger, name, lock);
+	return new State(debugger, name);
 }
 
 void SpirvShader::Impl::Debugger::State::destroy(State *state)
@@ -914,13 +913,13 @@ void SpirvShader::Impl::Debugger::State::destroy(State *state)
 	delete state;
 }
 
-SpirvShader::Impl::Debugger::State::State(const Debugger *debugger, const char *stackBase, vk::dbg::Context::Lock &lock)
+SpirvShader::Impl::Debugger::State::State(const Debugger *debugger, const char *stackBase)
     : debugger(debugger)
-    , thread(lock.currentThread())
+    , thread(debugger->ctx->lock().currentThread())
     , shadow(new uint8_t[debugger->shadow.size])
     , initialThreadDepth(thread->depth())
 {
-	enter(lock, stackBase);
+	enter(stackBase);
 
 	thread->update(true, [&](vk::dbg::Frame &frame) {
 		globals.locals = frame.locals;
@@ -942,9 +941,9 @@ SpirvShader::Impl::Debugger::State::~State()
 	}
 }
 
-void SpirvShader::Impl::Debugger::State::enter(vk::dbg::Context::Lock &lock, const char *name)
+void SpirvShader::Impl::Debugger::State::enter(const char *name)
 {
-	thread->enter(lock, debugger->spirvFile, name);
+	thread->enter(debugger->spirvFile, name);
 }
 
 void SpirvShader::Impl::Debugger::State::exit()
@@ -1058,8 +1057,7 @@ void SpirvShader::Impl::Debugger::State::setScope(debug::SourceScope *newSrcScop
 
 	if(hasDebuggerScope(srcScope->scope))
 	{
-		auto lock = debugger->ctx->lock();
-		auto thread = lock.currentThread();
+		auto thread = debugger->ctx->lock().currentThread();
 
 		debug::Function *oldFunction = oldSrcScope ? debug::find<debug::Function>(oldSrcScope->scope) : nullptr;
 		debug::Function *newFunction = newSrcScope ? debug::find<debug::Function>(newSrcScope->scope) : nullptr;
@@ -1067,7 +1065,7 @@ void SpirvShader::Impl::Debugger::State::setScope(debug::SourceScope *newSrcScop
 		if(oldFunction != newFunction)
 		{
 			if(oldFunction) { thread->exit(); }
-			if(newFunction) { thread->enter(lock, newFunction->source->dbgFile, newFunction->name); }
+			if(newFunction) { thread->enter(newFunction->source->dbgFile, newFunction->name); }
 		}
 
 		auto dbgScope = getScopes(srcScope->scope);

src/Vulkan/Debug/Thread.cpp

@@ -23,7 +23,7 @@ namespace dbg {
 
 Thread::Thread(ID id, Context *ctx)
     : id(id)
-    , broadcast(ctx->serverEventBroadcast())
+    , ctx(ctx)
 {}
 
 void Thread::setName(const std::string &name)
@@ -46,7 +46,7 @@ void Thread::onLocationUpdate(marl::lock &lock)
 	{
 		if(location.file->hasBreakpoint(location.line))
 		{
-			broadcast->onLineBreakpointHit(id);
+			ctx->serverEventBroadcast()->onLineBreakpointHit(id);
 			state_ = State::Paused;
 		}
 	}
@@ -61,12 +61,23 @@ void Thread::onLocationUpdate(marl::lock &lock)
 
 		case State::Stepping:
 		{
-			if(!pauseAtFrame || pauseAtFrame == frames.back())
+			bool pause = false;
+
+			{
+				auto frame = pauseAtFrame.lock();
+				pause = !frame;             // Pause if there's no pause-at-frame...
+				if(frame == frames.back())  // ... or if we've reached the pause-at-frame
+				{
+					pause = true;
+					pauseAtFrame.reset();
+				}
+			}
+
+			if(pause)
 			{
-				broadcast->onThreadStepped(id);
+				ctx->serverEventBroadcast()->onThreadStepped(id);
 				state_ = State::Paused;
 				lock.wait(stateCV, [this]() REQUIRES(mutex) { return state_ != State::Paused; });
-				pauseAtFrame = 0;
 			}
 			break;
 		}
@@ -76,18 +87,30 @@ void Thread::onLocationUpdate(marl::lock &lock)
 	}
 }
 
-void Thread::enter(Context::Lock &ctxlck, const std::shared_ptr<File> &file, const std::string &function)
+void Thread::enter(const std::shared_ptr<File> &file, const std::string &function, const UpdateFrame &f)
 {
-	auto frame = ctxlck.createFrame(file, function);
-	auto isFunctionBreakpoint = ctxlck.isFunctionBreakpoint(function);
+	std::shared_ptr<Frame> frame;
+	bool isFunctionBreakpoint;
+	{
+		auto lock = ctx->lock();
+		frame = lock.createFrame(file, function);
+		isFunctionBreakpoint = lock.isFunctionBreakpoint(function);
+	}
 
+	{
 		marl::lock lock(mutex);
 		frames.push_back(frame);
+
+		if(f) { f(*frame); }
+
 		if(isFunctionBreakpoint)
 		{
-		broadcast->onFunctionBreakpointHit(id);
+			ctx->serverEventBroadcast()->onFunctionBreakpointHit(id);
 			state_ = State::Paused;
 		}
+
+		onLocationUpdate(lock);
+	}
 }
 
 void Thread::exit()
@@ -96,7 +119,7 @@ void Thread::exit()
 	frames.pop_back();
 }
 
-void Thread::update(bool isStep, std::function<void(Frame &)> f)
+void Thread::update(bool isStep, const UpdateFrame &f)
 {
 	marl::lock lock(mutex);
 	auto &frame = *frames.back();
@@ -180,7 +203,7 @@ void Thread::stepOut()
 {
 	marl::lock lock(mutex);
 	state_ = State::Stepping;
-	pauseAtFrame = (frames.size() > 1) ? frames[frames.size() - 1] : nullptr;
+	pauseAtFrame = (frames.size() > 1) ? frames[frames.size() - 2] : nullptr;
 	stateCV.notify_all();
 }
 

src/Vulkan/Debug/Thread.hpp

@@ -107,6 +107,8 @@ class Thread
 public:
 	using ID = dbg::ID<Thread>;
 
+	using UpdateFrame = std::function<void(Frame &)>;
+
 	// The current execution state.
 	enum class State
 	{
@@ -124,8 +126,8 @@ public:
 	std::string name() const;
 
 	// enter() pushes the thread's stack with a new frame created with the given
-	// file and function.
-	void enter(Context::Lock &lock, const std::shared_ptr<File> &file, const std::string &function);
+	// file and function, then calls f to modify the new frame of the stack.
+	void enter(const std::shared_ptr<File> &file, const std::string &function, const UpdateFrame &f = nullptr);
 
 	// exit() pops the thread's stack frame.
 	void exit();
@@ -150,7 +152,7 @@ public:
 	// from full line updates. Note that we cannot simply examine line position
 	// changes as single-line loops such as `while(true) { foo(); }` would not
 	// be correctly steppable.
-	void update(bool isStep, std::function<void(Frame &)> f);
+	void update(bool isStep, const UpdateFrame &f);
 
 	// resume() resumes execution of the thread by unblocking a call to
 	// update() and setting the thread's state to State::Running.
@@ -181,7 +183,7 @@ public:
 	const ID id;
 
 private:
-	ServerEventListener *const broadcast;
+	Context *const ctx;
 
 	void onLocationUpdate(marl::lock &lock) REQUIRES(mutex);
 
@@ -189,7 +191,7 @@ private:
 	std::string name_ GUARDED_BY(mutex);
 	std::vector<std::shared_ptr<Frame>> frames GUARDED_BY(mutex);
 	State state_ GUARDED_BY(mutex) = State::Running;
-	std::shared_ptr<Frame> pauseAtFrame GUARDED_BY(mutex);
+	std::weak_ptr<Frame> pauseAtFrame GUARDED_BY(mutex);
 
 	std::condition_variable stateCV;
 };

src/Vulkan/VkCommandBuffer.cpp

@@ -1810,11 +1810,9 @@ void CommandBuffer::submit(CommandBuffer::ExecutionState &executionState)
 	auto debuggerContext = device->getDebuggerContext();
 	if(debuggerContext)
 	{
-		auto lock = debuggerContext->lock();
-		debuggerThread = lock.currentThread();
+		debuggerThread = debuggerContext->lock().currentThread();
 		debuggerThread->setName("vkQueue processor");
-		debuggerThread->enter(lock, debuggerFile, "vkCommandBuffer::submit");
-		lock.unlock();
+		debuggerThread->enter(debuggerFile, "vkCommandBuffer::submit");
 	}
 	defer(if(debuggerThread) { debuggerThread->exit(); });
 	int line = 1;