Merge pull request #15472 from unknownbrackets/waitable

ThreadManager: Improve waitable destruction

Common/Thread/Waitable.h

@@ -1,7 +1,8 @@
 #pragma once
 
-#include <mutex>
+#include <atomic>
 #include <condition_variable>
+#include <mutex>
 
 #include "Common/Thread/ThreadManager.h"
 
@@ -11,22 +12,29 @@ public:
 		triggered_ = false;
 	}
 
+	~LimitedWaitable() {
+		// Make sure no one is still waiting, and any notify lock is released.
+		Notify();
+	}
+
 	void Wait() override {
+		if (triggered_)
+			return;
+
 		std::unique_lock<std::mutex> lock(mutex_);
-		if (!triggered_) {
-			cond_.wait(lock, [&] { return triggered_.load(); });
-		}
+		cond_.wait(lock, [&] { return triggered_.load(); });
 	}
 
 	bool WaitFor(double budget) {
+		if (triggered_)
+			return true;
+
 		uint32_t us = budget > 0 ? (uint32_t)(budget * 1000000.0) : 0;
+		if (us == 0)
+			return false;
+
 		std::unique_lock<std::mutex> lock(mutex_);
-		if (!triggered_) {
-			if (us == 0)
-				return false;
-			cond_.wait_for(lock, std::chrono::microseconds(us), [&] { return triggered_.load(); });
-		}
-		return triggered_;
+		return cond_.wait_for(lock, std::chrono::microseconds(us), [&] { return triggered_.load(); });
 	}
 
 	void Notify() {

Core/TextureReplacer.cpp

@@ -765,13 +765,8 @@ private:
 
 bool ReplacedTexture::IsReady(double budget) {
 	lastUsed_ = time_now_d();
-	if (threadWaitable_) {
-		if (!threadWaitable_->WaitFor(budget)) {
-			return false;
-		} else {
-			threadWaitable_->WaitAndRelease();
-			threadWaitable_ = nullptr;
-		}
+	if (threadWaitable_ && !threadWaitable_->WaitFor(budget)) {
+		return false;
 	}
 
 	// Loaded already, or not yet on a thread?
@@ -786,9 +781,6 @@ bool ReplacedTexture::IsReady(double budget) {
 		g_threadManager.EnqueueTask(new ReplacedTextureTask(*this, threadWaitable_));
 
 		if (threadWaitable_->WaitFor(budget)) {
-			threadWaitable_->WaitAndRelease();
-			threadWaitable_ = nullptr;
-
 			// If we finished all the levels, we're done.
 			return !levelData_.empty();
 		}
@@ -802,12 +794,19 @@ bool ReplacedTexture::IsReady(double budget) {
 }
 
 void ReplacedTexture::Prepare() {
+	std::unique_lock<std::mutex> lock(mutex_);
+	if (cancelPrepare_)
+		return;
+
 	levelData_.resize(MaxLevel() + 1);
 	for (int i = 0; i <= MaxLevel(); ++i) {
 		if (cancelPrepare_)
 			break;
 		PrepareData(i);
 	}
+
+	if (!cancelPrepare_ && threadWaitable_)
+		threadWaitable_->Notify();
 }
 
 void ReplacedTexture::PrepareData(int level) {
@@ -909,7 +908,7 @@ void ReplacedTexture::PrepareData(int level) {
 }
 
 void ReplacedTexture::PurgeIfOlder(double t) {
-	if (lastUsed_ < t && !threadWaitable_) {
+	if (lastUsed_ < t && (!threadWaitable_ || threadWaitable_->WaitFor(0.0))) {
 		levelData_.clear();
 	}
 }
@@ -917,6 +916,8 @@ void ReplacedTexture::PurgeIfOlder(double t) {
 ReplacedTexture::~ReplacedTexture() {
 	if (threadWaitable_) {
 		cancelPrepare_ = true;
+
+		std::unique_lock<std::mutex> lock(mutex_);
 		threadWaitable_->WaitAndRelease();
 		threadWaitable_ = nullptr;
 	}

Core/TextureReplacer.h

@@ -17,6 +17,7 @@
 
 #pragma once
 
+#include <mutex>
 #include <string>
 #include <unordered_map>
 #include <vector>
@@ -170,6 +171,7 @@ protected:
 	ReplacedTextureAlpha alphaStatus_;
 	double lastUsed_ = 0.0;
 	LimitedWaitable *threadWaitable_ = nullptr;
+	std::mutex mutex_;
 	bool cancelPrepare_ = false;
 
 	friend TextureReplacer;