Merge pull request #18741 from hrydgard/more-beta-fixes

Fix another game-shutdown race condition

Common/GPU/Vulkan/VulkanRenderManager.cpp

@@ -342,49 +342,50 @@ bool VulkanRenderManager::CreateBackbuffers() {
 
 	// Start the thread(s).
 	if (HasBackbuffers()) {
-		runCompileThread_ = true;  // For controlling the compiler thread's exit
-		compileBlocked_ = false;
-
-		if (useRenderThread_) {
-			INFO_LOG(G3D, "Starting Vulkan submission thread");
-			thread_ = std::thread(&VulkanRenderManager::ThreadFunc, this);
-		}
-		INFO_LOG(G3D, "Starting Vulkan compiler thread");
-		compileThread_ = std::thread(&VulkanRenderManager::CompileThreadFunc, this);
-
-		if (measurePresentTime_ && vulkan_->Extensions().KHR_present_wait && vulkan_->GetPresentMode() == VK_PRESENT_MODE_FIFO_KHR) {
-			INFO_LOG(G3D, "Starting Vulkan present wait thread");
-			presentWaitThread_ = std::thread(&VulkanRenderManager::PresentWaitThreadFunc, this);
-		}
+		StartThreads();
 	}
 	return true;
 }
 
-// Called from main thread.
-void VulkanRenderManager::StopThread() {
+void VulkanRenderManager::StartThreads() {
+	runCompileThread_ = true;  // For controlling the compiler thread's exit
+
 	if (useRenderThread_) {
-		_dbg_assert_(thread_.joinable());
+		INFO_LOG(G3D, "Starting Vulkan submission thread");
+		renderThread_ = std::thread(&VulkanRenderManager::RenderThreadFunc, this);
+	}
+	INFO_LOG(G3D, "Starting Vulkan compiler thread");
+	compileThread_ = std::thread(&VulkanRenderManager::CompileThreadFunc, this);
+
+	if (measurePresentTime_ && vulkan_->Extensions().KHR_present_wait && vulkan_->GetPresentMode() == VK_PRESENT_MODE_FIFO_KHR) {
+		INFO_LOG(G3D, "Starting Vulkan present wait thread");
+		presentWaitThread_ = std::thread(&VulkanRenderManager::PresentWaitThreadFunc, this);
+	}
+}
+
+// Called from main thread.
+void VulkanRenderManager::StopThreads() {
+	if (useRenderThread_) {
+		_dbg_assert_(renderThread_.joinable());
 		// Tell the render thread to quit when it's done.
 		VKRRenderThreadTask *task = new VKRRenderThreadTask(VKRRunType::EXIT);
 		task->frame = vulkan_->GetCurFrame();
-		std::unique_lock<std::mutex> lock(pushMutex_);
-		renderThreadQueue_.push(task);
+		{
+			std::unique_lock<std::mutex> lock(pushMutex_);
+			renderThreadQueue_.push(task);
+		}
 		pushCondVar_.notify_one();
+		// Once the render thread encounters the above exit task, it'll exit.
+		renderThread_.join();
 	}
 
 	// Compiler and present thread still relies on this.
 	runCompileThread_ = false;
-	compileBlocked_ = true;
 
 	if (presentWaitThread_.joinable()) {
 		presentWaitThread_.join();
 	}
 
-	// Stop the thread.
-	if (useRenderThread_) {
-		thread_.join();
-	}
-
 	for (int i = 0; i < vulkan_->GetInflightFrames(); i++) {
 		auto &frameData = frameData_[i];
 		// Zero the queries so we don't try to pull them later.
@@ -393,22 +394,17 @@ void VulkanRenderManager::StopThread() {
 
 	INFO_LOG(G3D, "Vulkan submission thread joined. Frame=%d", vulkan_->GetCurFrame());
 
-	if (compileThread_.joinable()) {
-		// Lock to avoid race conditions. Not sure if needed.
-		{
-			std::lock_guard<std::mutex> guard(compileMutex_);
-			compileCond_.notify_all();
-		}
-		compileThread_.join();
-	}
+	_assert_(compileThread_.joinable());
+	compileCond_.notify_all();
+	compileThread_.join();
+
 	INFO_LOG(G3D, "Vulkan compiler thread joined. Now wait for any straggling compile tasks.");
 	CreateMultiPipelinesTask::WaitForAll();
-
 	_dbg_assert_(steps_.empty());
 }
 
 void VulkanRenderManager::DestroyBackbuffers() {
-	StopThread();
+	StopThreads();
 	vulkan_->WaitUntilQueueIdle();
 
 	queueRunner_.DestroyBackBuffers();
@@ -487,35 +483,16 @@ void VulkanRenderManager::CompileThreadFunc() {
 			g_threadManager.EnqueueTask(task);
 		}
 
-		// Hold off just a bit before we check again, to allow bunches of pipelines to collect.
-		sleep_ms(1);
-
 		if (!runCompileThread_) {
 			break;
 		}
+
+		// Hold off just a bit before we check again, to allow bunches of pipelines to collect.
+		sleep_ms(1);
 	}
 }
 
-void VulkanRenderManager::DrainAndBlockCompileQueue() {
-	compileBlocked_ = true;
-	runCompileThread_ = false;
-	compileCond_.notify_all();
-	compileThread_.join();
-
-	_assert_(compileQueue_.empty());
-
-	// At this point, no more tasks can be queued to the threadpool. So wait for them all to go away.
-	CreateMultiPipelinesTask::WaitForAll();
-}
-
-void VulkanRenderManager::ReleaseCompileQueue() {
-	compileBlocked_ = false;
-	runCompileThread_ = true;
-	INFO_LOG(G3D, "Restarting Vulkan compiler thread");
-	compileThread_ = std::thread(&VulkanRenderManager::CompileThreadFunc, this);
-}
-
-void VulkanRenderManager::ThreadFunc() {
+void VulkanRenderManager::RenderThreadFunc() {
 	SetCurrentThreadName("VulkanRenderMan");
 	while (true) {
 		_dbg_assert_(useRenderThread_);
@@ -781,7 +758,6 @@ VKRGraphicsPipeline *VulkanRenderManager::CreateGraphicsPipeline(VKRGraphicsPipe
 			VKRRenderPassStoreAction::STORE, VKRRenderPassStoreAction::DONT_CARE, VKRRenderPassStoreAction::DONT_CARE,
 		};
 		VKRRenderPass *compatibleRenderPass = queueRunner_.GetRenderPass(key);
-		_dbg_assert_(!compileBlocked_);
 		std::lock_guard<std::mutex> lock(compileMutex_);
 		bool needsCompile = false;
 		for (size_t i = 0; i < (size_t)RenderPassType::TYPE_COUNT; i++) {
@@ -854,7 +830,6 @@ void VulkanRenderManager::EndCurRenderStep() {
 	VkSampleCountFlagBits sampleCount = curRenderStep_->render.framebuffer ? curRenderStep_->render.framebuffer->sampleCount : VK_SAMPLE_COUNT_1_BIT;
 
 	compileMutex_.lock();
-	_dbg_assert_(!compileBlocked_);
 	bool needsCompile = false;
 	for (VKRGraphicsPipeline *pipeline : pipelinesToCheck_) {
 		if (!pipeline) {

Common/GPU/Vulkan/VulkanRenderManager.h

@@ -524,27 +524,25 @@ public:
 		return outOfDateFrames_ > VulkanContext::MAX_INFLIGHT_FRAMES;
 	}
 
-	void Invalidate(InvalidationFlags flags);
-
 	VulkanBarrierBatch &PostInitBarrier() {
 		return postInitBarrier_;
 	}
 
 	void ResetStats();
-	void DrainAndBlockCompileQueue();
-	void ReleaseCompileQueue();
+
+	void StartThreads();
+	void StopThreads();
 
 private:
 	void EndCurRenderStep();
 
-	void ThreadFunc();
+	void RenderThreadFunc();
 	void CompileThreadFunc();
 
 	void Run(VKRRenderThreadTask &task);
 
 	// Bad for performance but sometimes necessary for synchronous CPU readbacks (screenshots and whatnot).
 	void FlushSync();
-	void StopThread();
 
 	void PresentWaitThreadFunc();
 	void PollPresentTiming();
@@ -587,7 +585,7 @@ private:
 
 	// Execution time state
 	VulkanContext *vulkan_;
-	std::thread thread_;
+	std::thread renderThread_;
 	VulkanQueueRunner queueRunner_;
 
 	// For pushing data on the queue.
@@ -607,7 +605,6 @@ private:
 	std::condition_variable compileCond_;
 	std::mutex compileMutex_;
 	std::vector<CompileQueueEntry> compileQueue_;
-	std::atomic<bool> compileBlocked_{};  // Only for asserting on, now.
 
 	// Thread for measuring presentation delay.
 	std::thread presentWaitThread_;

GPU/Vulkan/GPU_Vulkan.cpp

@@ -168,7 +168,7 @@ GPU_Vulkan::~GPU_Vulkan() {
 	if (draw_) {
 		VulkanRenderManager *rm = (VulkanRenderManager *)draw_->GetNativeObject(Draw::NativeObject::RENDER_MANAGER);
 		// This now also does a hard sync with the render thread, so that we can safely delete our pipeline layout below.
-		rm->DrainAndBlockCompileQueue();
+		rm->StopThreads();
 	}
 
 	SaveCache(shaderCachePath_);
@@ -185,7 +185,7 @@ GPU_Vulkan::~GPU_Vulkan() {
 	// other managers are deleted in ~GPUCommonHW.
 	if (draw_) {
 		VulkanRenderManager *rm = (VulkanRenderManager *)draw_->GetNativeObject(Draw::NativeObject::RENDER_MANAGER);
-		rm->ReleaseCompileQueue();
+		rm->StartThreads();
 	}
 }
 
@@ -426,7 +426,7 @@ void GPU_Vulkan::DeviceLost() {
 	Draw::DrawContext *draw = draw_;
 	if (draw) {
 		VulkanRenderManager *rm = (VulkanRenderManager *)draw->GetNativeObject(Draw::NativeObject::RENDER_MANAGER);
-		rm->DrainAndBlockCompileQueue();
+		rm->StopThreads();
 	}
 
 	if (shaderCachePath_.Valid()) {
@@ -439,7 +439,7 @@ void GPU_Vulkan::DeviceLost() {
 
 	if (draw) {
 		VulkanRenderManager *rm = (VulkanRenderManager *)draw->GetNativeObject(Draw::NativeObject::RENDER_MANAGER);
-		rm->ReleaseCompileQueue();
+		rm->StartThreads();
 	}
 }