From bfe66ab7cd73279d078d73b925573745031a9e62 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pedro=20J=2E=20Est=C3=A9banez?= <pedrojrulez@gmail.com>
Date: Wed, 22 Nov 2023 23:20:49 +0100
Subject: Fixup thread-owned lambda bookkeeping on thread exit (take 2)

---
 modules/gdscript/gdscript.cpp | 87 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 74 insertions(+), 13 deletions(-)

(limited to 'modules/gdscript/gdscript.cpp')

diff --git a/modules/gdscript/gdscript.cpp b/modules/gdscript/gdscript.cpp
index a2be6a86d7..05c2558417 100644
--- a/modules/gdscript/gdscript.cpp
+++ b/modules/gdscript/gdscript.cpp
@@ -1391,36 +1391,54 @@ String GDScript::debug_get_script_name(const Ref<Script> &p_script) {
 }
 #endif
 
-thread_local GDScript::UpdatableFuncPtr GDScript::func_ptrs_to_update_thread_local;
+GDScript::UpdatableFuncPtr GDScript::func_ptrs_to_update_main_thread;
+thread_local GDScript::UpdatableFuncPtr *GDScript::func_ptrs_to_update_thread_local = nullptr;
 
 GDScript::UpdatableFuncPtrElement GDScript::_add_func_ptr_to_update(GDScriptFunction **p_func_ptr_ptr) {
 	UpdatableFuncPtrElement result = {};
 
 	{
-		MutexLock lock(func_ptrs_to_update_thread_local.mutex);
-		result.element = func_ptrs_to_update_thread_local.ptrs.push_back(p_func_ptr_ptr);
-		result.mutex = &func_ptrs_to_update_thread_local.mutex;
+		MutexLock lock(func_ptrs_to_update_thread_local->mutex);
+		result.element = func_ptrs_to_update_thread_local->ptrs.push_back(p_func_ptr_ptr);
+		result.func_ptr = func_ptrs_to_update_thread_local;
 
-		if (likely(func_ptrs_to_update_thread_local.initialized)) {
+		if (likely(func_ptrs_to_update_thread_local->initialized)) {
 			return result;
 		}
 
-		func_ptrs_to_update_thread_local.initialized = true;
+		func_ptrs_to_update_thread_local->initialized = true;
 	}
 
 	MutexLock lock(func_ptrs_to_update_mutex);
-	func_ptrs_to_update.push_back(&func_ptrs_to_update_thread_local);
+	func_ptrs_to_update.push_back(func_ptrs_to_update_thread_local);
+	func_ptrs_to_update_thread_local->rc++;
 
 	return result;
 }
 
-void GDScript::_remove_func_ptr_to_update(const UpdatableFuncPtrElement p_func_ptr_element) {
+void GDScript::_remove_func_ptr_to_update(const UpdatableFuncPtrElement &p_func_ptr_element) {
 	ERR_FAIL_NULL(p_func_ptr_element.element);
-	ERR_FAIL_NULL(p_func_ptr_element.mutex);
-	MutexLock lock(*p_func_ptr_element.mutex);
+	ERR_FAIL_NULL(p_func_ptr_element.func_ptr);
+	MutexLock lock(p_func_ptr_element.func_ptr->mutex);
 	p_func_ptr_element.element->erase();
 }
 
+void GDScript::_fixup_thread_function_bookkeeping() {
+	// Transfer the ownership of these update items to the main thread,
+	// because the current one is dying, leaving theirs orphan, dangling.
+
+	DEV_ASSERT(!Thread::is_main_thread());
+
+	MutexLock lock(func_ptrs_to_update_main_thread.mutex);
+	MutexLock lock2(func_ptrs_to_update_thread_local->mutex);
+
+	while (!func_ptrs_to_update_thread_local->ptrs.is_empty()) {
+		List<GDScriptFunction **>::Element *E = func_ptrs_to_update_thread_local->ptrs.front();
+		E->transfer_to_back(&func_ptrs_to_update_main_thread.ptrs);
+		func_ptrs_to_update_thread_local->transferred = true;
+	}
+}
+
 void GDScript::clear(ClearData *p_clear_data) {
 	if (clearing) {
 		return;
@@ -1441,9 +1459,27 @@ void GDScript::clear(ClearData *p_clear_data) {
 	{
 		MutexLock outer_lock(func_ptrs_to_update_mutex);
 		for (UpdatableFuncPtr *updatable : func_ptrs_to_update) {
-			MutexLock inner_lock(updatable->mutex);
-			for (GDScriptFunction **func_ptr_ptr : updatable->ptrs) {
-				*func_ptr_ptr = nullptr;
+			bool destroy = false;
+			{
+				MutexLock inner_lock(updatable->mutex);
+				if (updatable->transferred) {
+					func_ptrs_to_update_main_thread.mutex.lock();
+				}
+				for (GDScriptFunction **func_ptr_ptr : updatable->ptrs) {
+					*func_ptr_ptr = nullptr;
+				}
+				DEV_ASSERT(updatable->rc != 0);
+				updatable->rc--;
+				if (updatable->rc == 0) {
+					destroy = true;
+				}
+				if (updatable->transferred) {
+					func_ptrs_to_update_main_thread.mutex.unlock();
+				}
+			}
+			if (destroy) {
+				DEV_ASSERT(updatable != &func_ptrs_to_update_main_thread);
+				memdelete(updatable);
 			}
 		}
 	}
@@ -2065,6 +2101,27 @@ void GDScriptLanguage::remove_named_global_constant(const StringName &p_name) {
 	named_globals.erase(p_name);
 }
 
+void GDScriptLanguage::thread_enter() {
+	GDScript::func_ptrs_to_update_thread_local = memnew(GDScript::UpdatableFuncPtr);
+}
+
+void GDScriptLanguage::thread_exit() {
+	GDScript::_fixup_thread_function_bookkeeping();
+
+	bool destroy = false;
+	{
+		MutexLock lock(GDScript::func_ptrs_to_update_thread_local->mutex);
+		DEV_ASSERT(GDScript::func_ptrs_to_update_thread_local->rc != 0);
+		GDScript::func_ptrs_to_update_thread_local->rc--;
+		if (GDScript::func_ptrs_to_update_thread_local->rc == 0) {
+			destroy = true;
+		}
+	}
+	if (destroy) {
+		memdelete(GDScript::func_ptrs_to_update_thread_local);
+	}
+}
+
 void GDScriptLanguage::init() {
 	//populate global constants
 	int gcc = CoreConstants::get_global_constant_count();
@@ -2097,6 +2154,8 @@ void GDScriptLanguage::init() {
 		_add_global(E.name, E.ptr);
 	}
 
+	GDScript::func_ptrs_to_update_thread_local = &GDScript::func_ptrs_to_update_main_thread;
+
 #ifdef TESTS_ENABLED
 	GDScriptTests::GDScriptTestRunner::handle_cmdline();
 #endif
@@ -2146,6 +2205,8 @@ void GDScriptLanguage::finish() {
 	}
 	script_list.clear();
 	function_list.clear();
+
+	DEV_ASSERT(GDScript::func_ptrs_to_update_main_thread.rc == 1);
 }
 
 void GDScriptLanguage::profiling_start() {
-- 
cgit v1.2.3