Fix use condition_variable after delete

godotengine · Jul 27, 2024 · 022285a · 022285a
1 parent e343dbb
commit 022285a
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 17 deletions.
diff --git a/core/io/resource_loader.cpp b/core/io/resource_loader.cpp
@@ -48,6 +48,31 @@
 #define print_lt(m_text)
 #endif
 
+template <typename MutexT, typename OnWakeupT>
+void ResourceLoader::WaitBarrier::wait(MutexLock<MutexT> &p_lock, OnWakeupT p_on_wakeup) {
+	if (!notified) {
+		if (!cond_var) {
+			cond_var = memnew(ConditionVariable);
+		}
+		++wait_count;
+		do {
+			cond_var->wait(p_lock);
+			p_on_wakeup();
+		} while (!notified);
+		if (!--wait_count) {
+			memdelete(cond_var);
+			cond_var = nullptr;
+		}
+	}
+}
+
+void ResourceLoader::WaitBarrier::notify() {
+	if (cond_var && !notified) {
+		cond_var->notify_all();
+	}
+	notified = true;
+}
+
 Ref<ResourceFormatLoader> ResourceLoader::loader[ResourceLoader::MAX_LOADERS];
 
 int ResourceLoader::loader_count = 0;
@@ -337,11 +362,7 @@ void ResourceLoader::_thread_load_function(void *p_userdata) {
 		load_task.status = THREAD_LOAD_LOADED;
 	}
 
-	if (load_task.cond_var) {
-		load_task.cond_var->notify_all();
-		memdelete(load_task.cond_var);
-		load_task.cond_var = nullptr;
-	}
+	load_task.wait_barrier.notify();
 
 	bool ignoring = load_task.cache_mode == ResourceFormatLoader::CACHE_MODE_IGNORE || load_task.cache_mode == ResourceFormatLoader::CACHE_MODE_IGNORE_DEEP;
 	bool replacing = load_task.cache_mode == ResourceFormatLoader::CACHE_MODE_REPLACE || load_task.cache_mode == ResourceFormatLoader::CACHE_MODE_REPLACE_DEEP;
@@ -718,13 +739,9 @@ Ref<Resource> ResourceLoader::_load_complete_inner(LoadToken &p_load_token, Erro
 					}
 				} else {
 					// Loading thread is main or user thread.
-					if (!load_task.cond_var) {
-						load_task.cond_var = memnew(ConditionVariable);
-					}
-					do {
-						load_task.cond_var->wait(p_thread_load_lock);
+					load_task.wait_barrier.wait(p_thread_load_lock, [&] {
 						DEV_ASSERT(thread_load_tasks.has(p_load_token.local_path) && p_load_token.get_reference_count());
-					} while (load_task.cond_var);
+					});
 				}
 			} else {
 				if (loader_is_wtp) {
@@ -1152,11 +1169,7 @@ void ResourceLoader::clear_thread_load_tasks() {
 		if (thread_load_tasks.size()) {
 			for (KeyValue<String, ResourceLoader::ThreadLoadTask> &E : thread_load_tasks) {
 				if (E.value.status == THREAD_LOAD_IN_PROGRESS) {
-					if (E.value.cond_var) {
-						E.value.cond_var->notify_all();
-						memdelete(E.value.cond_var);
-						E.value.cond_var = nullptr;
-					}
+					E.value.wait_barrier.notify();
 					none_running = false;
 				}
 			}

diff --git a/core/io/resource_loader.h b/core/io/resource_loader.h
@@ -162,11 +162,22 @@ class ResourceLoader {
 
 	static Ref<ResourceFormatLoader> _find_custom_resource_format_loader(const String &path);
 
+	class WaitBarrier {
+		ConditionVariable *cond_var = nullptr;
+		uint32_t wait_count = 0;
+		bool notified = false;
+
+	public:
+		template <typename MutexT, typename OnWakeupT>
+		void wait(MutexLock<MutexT> &p_lock, OnWakeupT p_on_wakeup);
+		void notify();
+	};
+
 	struct ThreadLoadTask {
 		WorkerThreadPool::TaskID task_id = 0; // Used if run on a worker thread from the pool.
 		Thread::ID thread_id = 0; // Used if running on an user thread (e.g., simple non-threaded load).
 		bool awaited = false; // If it's in the pool, this helps not awaiting from more than one dependent thread.
-		ConditionVariable *cond_var = nullptr; // In not in the worker pool or already awaiting, this is used as a secondary awaiting mechanism.
+		WaitBarrier wait_barrier; // In not in the worker pool or already awaiting, this is used as a secondary awaiting mechanism.
 		LoadToken *load_token = nullptr;
 		String local_path;
 		String remapped_path;