Use script instance binding for objects constructed from C#

Only possible if the object class is a "native type". If the object class is a user class (that derives a "native type") then a script is needed.
Since CSharpLanguage does cleanup of script instance bindings when finished, cases like #25621 will no longer cause problems.

Fixed ~Object() trying to free script instance bindings after the language has already been removed, which would result in a NULL dereference.

core/object.cpp

@@ -1929,6 +1929,13 @@ bool Object::has_script_instance_binding(int p_script_language_index) {
 	return _script_instance_bindings[p_script_language_index] != NULL;
 }
 
+void Object::set_script_instance_binding(int p_script_language_index, void *p_data) {
+#ifdef DEBUG_ENABLED
+	CRASH_COND(_script_instance_bindings[p_script_language_index] != NULL);
+#endif
+	_script_instance_bindings[p_script_language_index] = p_data;
+}
+
 Object::Object() {
 
 	_class_ptr = NULL;
@@ -1992,9 +1999,11 @@ Object::~Object() {
 	_instance_ID = 0;
 	_predelete_ok = 2;
 
-	for (int i = 0; i < MAX_SCRIPT_INSTANCE_BINDINGS; i++) {
-		if (_script_instance_bindings[i]) {
-			ScriptServer::get_language(i)->free_instance_binding_data(_script_instance_bindings[i]);
+	if (!ScriptServer::are_languages_finished()) {
+		for (int i = 0; i < MAX_SCRIPT_INSTANCE_BINDINGS; i++) {
+			if (_script_instance_bindings[i]) {
+				ScriptServer::get_language(i)->free_instance_binding_data(_script_instance_bindings[i]);
+			}
 		}
 	}
 }

core/object.h

@@ -730,6 +730,7 @@ public:
 	//used by script languages to store binding data
 	void *get_script_instance_binding(int p_script_language_index);
 	bool has_script_instance_binding(int p_script_language_index);
+	void set_script_instance_binding(int p_script_language_index, void *p_data);
 
 	void clear_internal_resource_paths();
 

core/script_language.cpp

@@ -37,6 +37,7 @@ int ScriptServer::_language_count = 0;
 
 bool ScriptServer::scripting_enabled = true;
 bool ScriptServer::reload_scripts_on_save = false;
+bool ScriptServer::languages_finished = false;
 ScriptEditRequestFunction ScriptServer::edit_request_func = NULL;
 
 void Script::_notification(int p_what) {
@@ -130,6 +131,7 @@ void ScriptServer::finish_languages() {
 		_languages[i]->finish();
 	}
 	global_classes_clear();
+	languages_finished = true;
 }
 
 void ScriptServer::set_reload_scripts_on_save(bool p_enable) {

core/script_language.h

@@ -54,6 +54,7 @@ class ScriptServer {
 	static int _language_count;
 	static bool scripting_enabled;
 	static bool reload_scripts_on_save;
+	static bool languages_finished;
 
 	struct GlobalScriptClass {
 		StringName language;
@@ -91,6 +92,8 @@ public:
 
 	static void init_languages();
 	static void finish_languages();
+
+	static bool are_languages_finished() { return languages_finished; }
 };
 
 class ScriptInstance;

modules/mono/csharp_script.cpp

@@ -1113,19 +1113,23 @@ bool CSharpLanguage::setup_csharp_script_binding(CSharpScriptBinding &r_script_b
 
 void *CSharpLanguage::alloc_instance_binding_data(Object *p_object) {
 
+	SCOPED_MUTEX_LOCK(language_bind_mutex);
+
+	Map<Object *, CSharpScriptBinding>::Element *match = script_bindings.find(p_object);
+	if (match)
+		return (void *)match;
+
 	CSharpScriptBinding script_binding;
 
 	if (!setup_csharp_script_binding(script_binding, p_object))
 		return NULL;
 
-	void *data;
+	return (void *)insert_script_binding(p_object, script_binding);
+}
 
-	{
-		SCOPED_MUTEX_LOCK(language_bind_mutex);
-		data = (void *)script_bindings.insert(p_object, script_binding);
-	}
+Map<Object *, CSharpScriptBinding>::Element *CSharpLanguage::insert_script_binding(Object *p_object, const CSharpScriptBinding &p_script_binding) {
 
-	return data;
+	return script_bindings.insert(p_object, p_script_binding);
 }
 
 void CSharpLanguage::free_instance_binding_data(void *p_data) {
@@ -2279,17 +2283,18 @@ void CSharpScript::_bind_methods() {
 	ClassDB::bind_vararg_method(METHOD_FLAGS_DEFAULT, "new", &CSharpScript::_new, MethodInfo(Variant::OBJECT, "new"));
 }
 
-Ref<CSharpScript> CSharpScript::create_for_managed_type(GDMonoClass *p_class) {
+Ref<CSharpScript> CSharpScript::create_for_managed_type(GDMonoClass *p_class, GDMonoClass *p_native) {
 
 	// This method should not fail
 
 	CRASH_COND(!p_class);
 
+	// TODO: Cache the 'CSharpScript' associated with this 'p_class' instead of allocating a new one every time
 	Ref<CSharpScript> script = memnew(CSharpScript);
 
 	script->name = p_class->get_name();
 	script->script_class = p_class;
-	script->native = GDMonoUtils::get_class_native_base(script->script_class);
+	script->native = p_native;
 
 	CRASH_COND(script->native == NULL);
 

modules/mono/csharp_script.h

@@ -135,7 +135,7 @@ class CSharpScript : public Script {
 
 	// Do not use unless you know what you are doing
 	friend void GDMonoInternals::tie_managed_to_unmanaged(MonoObject *, Object *);
-	static Ref<CSharpScript> create_for_managed_type(GDMonoClass *p_class);
+	static Ref<CSharpScript> create_for_managed_type(GDMonoClass *p_class, GDMonoClass *p_native);
 
 protected:
 	static void _bind_methods();
@@ -312,6 +312,8 @@ class CSharpLanguage : public ScriptLanguage {
 public:
 	StringNameCache string_names;
 
+	Mutex *get_language_bind_mutex() { return language_bind_mutex; }
+
 	_FORCE_INLINE_ int get_language_index() { return lang_idx; }
 	void set_language_index(int p_idx);
 
@@ -406,6 +408,7 @@ public:
 	virtual void refcount_incremented_instance_binding(Object *p_object);
 	virtual bool refcount_decremented_instance_binding(Object *p_object);
 
+	Map<Object *, CSharpScriptBinding>::Element *insert_script_binding(Object *p_object, const CSharpScriptBinding &p_script_binding);
 	bool setup_csharp_script_binding(CSharpScriptBinding &r_script_binding, Object *p_object);
 
 #ifdef DEBUG_ENABLED

modules/mono/editor/bindings_generator.cpp

@@ -849,6 +849,8 @@ Error BindingsGenerator::_generate_cs_type(const TypeInterface &itype, const Str
 		}
 	}
 
+	// TODO: BINDINGS_NATIVE_NAME_FIELD should be StringName, once we support it in C#
+
 	if (itype.is_singleton) {
 		// Add the type name and the singleton pointer as static fields
 

modules/mono/mono_gd/gd_mono_internals.cpp

@@ -34,6 +34,8 @@
 #include "../mono_gc_handle.h"
 #include "../utils/macros.h"
 #include "../utils/thread_local.h"
+#include "gd_mono_class.h"
+#include "gd_mono_marshal.h"
 #include "gd_mono_utils.h"
 
 #include <mono/metadata/exception.h>
@@ -55,10 +57,49 @@ void tie_managed_to_unmanaged(MonoObject *managed, Object *unmanaged) {
 
 	CRASH_COND(!klass);
 
+	GDMonoClass *native = GDMonoUtils::get_class_native_base(klass);
+
+	CRASH_COND(native == NULL);
+
+	if (native == klass) {
+		// If it's just a wrapper Godot class and not a custom inheriting class, then attach a
+		// script binding instead. One of the advantages of this is that if a script is attached
+		// later and it's not a C# script, then the managed object won't have to be disposed.
+		// Another reason for doing this is that this instance could outlive CSharpLanguage, which would
+		// be problematic when using a script. See: https://github.com/godotengine/godot/issues/25621
+
+		CSharpScriptBinding script_binding;
+
+		script_binding.inited = true;
+		script_binding.type_name = NATIVE_GDMONOCLASS_NAME(klass);
+		script_binding.wrapper_class = klass;
+		script_binding.gchandle = MonoGCHandle::create_strong(managed);
+
+		Reference *ref = Object::cast_to<Reference>(unmanaged);
+		if (ref) {
+			// Unsafe refcount increment. The managed instance also counts as a reference.
+			// This way if the unmanaged world has no references to our owner
+			// but the managed instance is alive, the refcount will be 1 instead of 0.
+			// See: godot_icall_Reference_Dtor(MonoObject *p_obj, Object *p_ptr)
+
+			ref->reference();
+		}
+
+		// The object was just created, no script instance binding should have been attached
+		CRASH_COND(unmanaged->has_script_instance_binding(CSharpLanguage::get_singleton()->get_language_index()));
+
+		void *data = (void *)CSharpLanguage::get_singleton()->insert_script_binding(unmanaged, script_binding);
+
+		// Should be thread safe because the object was just created and nothing else should be referencing it
+		unmanaged->set_script_instance_binding(CSharpLanguage::get_singleton()->get_language_index(), data);
+
+		return;
+	}
+
 	Ref<MonoGCHandle> gchandle = ref ? MonoGCHandle::create_weak(managed) :
 									   MonoGCHandle::create_strong(managed);
 
-	Ref<CSharpScript> script = CSharpScript::create_for_managed_type(klass);
+	Ref<CSharpScript> script = CSharpScript::create_for_managed_type(klass, native);
 
 	CRASH_COND(script.is_null());
 

modules/mono/mono_gd/gd_mono_utils.cpp

@@ -39,6 +39,7 @@
 
 #include "../csharp_script.h"
 #include "../utils/macros.h"
+#include "../utils/mutex_utils.h"
 #include "gd_mono.h"
 #include "gd_mono_class.h"
 #include "gd_mono_marshal.h"
@@ -281,17 +282,19 @@ MonoObject *unmanaged_get_managed(Object *unmanaged) {
 
 	void *data = unmanaged->get_script_instance_binding(CSharpLanguage::get_singleton()->get_language_index());
 
-	if (!data)
-		return NULL;
+	ERR_FAIL_NULL_V(data, NULL);
 
 	CSharpScriptBinding &script_binding = ((Map<Object *, CSharpScriptBinding>::Element *)data)->value();
 
 	if (!script_binding.inited) {
-		// Already had a binding that needs to be setup
-		CSharpLanguage::get_singleton()->setup_csharp_script_binding(script_binding, unmanaged);
+		SCOPED_MUTEX_LOCK(CSharpLanguage::get_singleton()->get_language_bind_mutex());
+
+		if (!script_binding.inited) { // Other thread may have set it up
+			// Already had a binding that needs to be setup
+			CSharpLanguage::get_singleton()->setup_csharp_script_binding(script_binding, unmanaged);
 
-		if (!script_binding.inited)
-			return NULL;
+			ERR_FAIL_COND_V(!script_binding.inited, NULL);
+		}
 	}
 
 	Ref<MonoGCHandle> &gchandle = script_binding.gchandle;