Fix leak in the bookkeeping of GDScript lambdas

godotengine · Nov 21, 2023 · 1ed6919 · 1ed6919
1 parent ac29e8e
commit 1ed6919
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 12 deletions.
diff --git a/modules/gdscript/gdscript.cpp b/modules/gdscript/gdscript.cpp
@@ -1394,7 +1394,7 @@ String GDScript::debug_get_script_name(const Ref<Script> &p_script) {
 thread_local GDScript::UpdatableFuncPtr GDScript::func_ptrs_to_update_thread_local;
 GDScript::UpdatableFuncPtr *GDScript::func_ptrs_to_update_main_thread = &func_ptrs_to_update_thread_local;
 
-GDScript::UpdatableFuncPtrElement *GDScript::_add_func_ptr_to_update(GDScriptFunction **p_func_ptr_ptr) {
+List<GDScript::UpdatableFuncPtrElement>::Element *GDScript::_add_func_ptr_to_update(GDScriptFunction **p_func_ptr_ptr) {
 	MutexLock lock(func_ptrs_to_update_mutex);
 
 	List<UpdatableFuncPtrElement>::Element *result = func_ptrs_to_update_elems.push_back(UpdatableFuncPtrElement());
@@ -1405,26 +1405,28 @@ GDScript::UpdatableFuncPtrElement *GDScript::_add_func_ptr_to_update(GDScriptFun
 		result->get().mutex = &func_ptrs_to_update_thread_local.mutex;
 
 		if (likely(func_ptrs_to_update_thread_local.initialized)) {
-			return &result->get();
+			return result;
 		}
 
 		func_ptrs_to_update_thread_local.initialized = true;
 	}
 
 	func_ptrs_to_update.push_back(&func_ptrs_to_update_thread_local);
 
-	return &result->get();
+	return result;
 }
 
-void GDScript::_remove_func_ptr_to_update(const UpdatableFuncPtrElement *p_func_ptr_element) {
+void GDScript::_remove_func_ptr_to_update(List<UpdatableFuncPtrElement>::Element *p_func_ptr_element) {
 	// None of these checks should ever fail, unless there's a bug.
 	// They can be removed once we are sure they never catch anything.
 	// Left here now due to extra safety needs late in the release cycle.
 	ERR_FAIL_NULL(p_func_ptr_element);
-	MutexLock lock(*p_func_ptr_element->mutex);
-	ERR_FAIL_NULL(p_func_ptr_element->element);
-	ERR_FAIL_NULL(p_func_ptr_element->mutex);
-	p_func_ptr_element->element->erase();
+	MutexLock lock(func_ptrs_to_update_thread_local.mutex);
+	ERR_FAIL_NULL(p_func_ptr_element->get().element);
+	ERR_FAIL_NULL(p_func_ptr_element->get().mutex);
+	MutexLock lock2(*p_func_ptr_element->get().mutex);
+	p_func_ptr_element->get().element->erase();
+	p_func_ptr_element->erase();
 }
 
 void GDScript::_fixup_thread_function_bookkeeping() {

diff --git a/modules/gdscript/gdscript.h b/modules/gdscript/gdscript.h
@@ -134,8 +134,8 @@ class GDScript : public Script {
 	List<UpdatableFuncPtrElement> func_ptrs_to_update_elems;
 	Mutex func_ptrs_to_update_mutex;
 
-	UpdatableFuncPtrElement *_add_func_ptr_to_update(GDScriptFunction **p_func_ptr_ptr);
-	static void _remove_func_ptr_to_update(const UpdatableFuncPtrElement *p_func_ptr_element);
+	List<UpdatableFuncPtrElement>::Element *_add_func_ptr_to_update(GDScriptFunction **p_func_ptr_ptr);
+	static void _remove_func_ptr_to_update(List<UpdatableFuncPtrElement>::Element *p_func_ptr_element);
 
 	static void _fixup_thread_function_bookkeeping();
 

diff --git a/modules/gdscript/gdscript_lambda_callable.h b/modules/gdscript/gdscript_lambda_callable.h
@@ -45,7 +45,7 @@ class GDScriptLambdaCallable : public CallableCustom {
 	GDScriptFunction *function = nullptr;
 	Ref<GDScript> script;
 	uint32_t h;
-	GDScript::UpdatableFuncPtrElement *updatable_func_ptr_element = nullptr;
+	List<GDScript::UpdatableFuncPtrElement>::Element *updatable_func_ptr_element = nullptr;
 
 	Vector<Variant> captures;
 
@@ -72,7 +72,7 @@ class GDScriptLambdaSelfCallable : public CallableCustom {
 	Ref<RefCounted> reference; // For objects that are RefCounted, keep a reference.
 	Object *object = nullptr; // For non RefCounted objects, use a direct pointer.
 	uint32_t h;
-	GDScript::UpdatableFuncPtrElement *updatable_func_ptr_element = nullptr;
+	List<GDScript::UpdatableFuncPtrElement>::Element *updatable_func_ptr_element = nullptr;
 
 	Vector<Variant> captures;