godot

godot copied to clipboard

Closes #24311

It can't be that simple, can it? 🤔

I rebased and the code is not valid anymore :/ It works, but sanitizer is not happy about it.

I suspect that the same GDScriptInstance::~GDScriptInstance() is ran twice. Try adding a destructing variable, like I did in GDScript::~GDScript().

I suspect that the same GDScriptInstance::~GDScriptInstance() is ran twice. Try adding a destructing variable, like I did in GDScript::~GDScript().

Curious to know how that would happen. I figured destructors would only ever run once!

I just tested adding destructing to GDScriptInstance but the issues persist unfortunately.

This patch makes the build pass the sanitizer.

diff --git a/modules/gdscript/gdscript.cpp b/modules/gdscript/gdscript.cpp
index 72d5bb23c5..dc87560fdd 100644
--- a/modules/gdscript/gdscript.cpp
+++ b/modules/gdscript/gdscript.cpp
@@ -1925,10 +1925,14 @@ GDScriptInstance::~GDScriptInstance() {
 
 	while (SelfList<GDScriptFunctionState> *E = pending_func_states.first()) {
 		// Order matters since clearing the stack may already cause
-		// the GDSCriptFunctionState to be destroyed and thus removed from the list.
+		// the GDScriptFunctionState to be destroyed and thus removed from the list.
 		pending_func_states.remove(E);
-		E->self()->_clear_connections();
-		E->self()->_clear_stack();
+		GDScriptFunctionState *state = E->self();
+		state->_clear_connections();
+		state = Object::cast_to<GDScriptFunctionState>(state);
+		if (state != nullptr) {
+			state->_clear_stack();
+		}
 	}
 
 	if (script.is_valid() && owner) {

Weird. My local build pass the clang test sanitizers.

My hunch says that this is more sane than casting AND it should fix the macos SIGSEGV.

diff --git a/modules/gdscript/gdscript.cpp b/modules/gdscript/gdscript.cpp
index df57749f76..fc3d42b1b0 100644
--- a/modules/gdscript/gdscript.cpp
+++ b/modules/gdscript/gdscript.cpp
@@ -1484,9 +1484,9 @@ GDScript::~GDScript() {
 			// the GDScriptFunctionState to be destroyed and thus removed from the list.
 			pending_func_states.remove(E);
 			GDScriptFunctionState *state = E->self();
+			ObjectID state_id = state->get_instance_id();
 			state->_clear_connections();
-			state = Object::cast_to<GDScriptFunctionState>(state);
-			if (state != nullptr) {
+			if (ObjectDB::get_instance(state_id) != nullptr) {
 				state->_clear_stack();
 			}
 		}
@@ -1932,9 +1932,9 @@ GDScriptInstance::~GDScriptInstance() {
 		// the GDSCriptFunctionState to be destroyed and thus removed from the list.
 		pending_func_states.remove(E);
 		GDScriptFunctionState *state = E->self();
+		ObjectID state_id = state->get_instance_id();
 		state->_clear_connections();
-		state = Object::cast_to<GDScriptFunctionState>(state);
-		if (state != nullptr) {
+		if (ObjectDB::get_instance(state_id) != nullptr) {
 			state->_clear_stack();
 		}
 	}

Thanks!