Mono: Lifetime fixes for CSharpInstance and instance binding data

Avoid CSharpInstance from accessing its state after self destructing (by deleting the Reference owner).
It's now safe to replace the script instance without leaking or crashing.

Also fixed godot_icall_Object_weakref return reference being freed before returning.

1 files changed, 44 insertions, 35 deletions

diff --git a/modules/mono/mono_gd/gd_mono_utils.cpp b/modules/mono/mono_gd/gd_mono_utils.cpp
index 804bec811e..ac6ccac3a7 100644
--- a/modules/mono/mono_gd/gd_mono_utils.cpp
+++ b/modules/mono/mono_gd/gd_mono_utils.cpp
@@ -265,61 +265,70 @@ void clear_cache() {
 }
 
 MonoObject *unmanaged_get_managed(Object *unmanaged) {
-	if (unmanaged) {
-		if (unmanaged->get_script_instance()) {
-			CSharpInstance *cs_instance = CAST_CSHARP_INSTANCE(unmanaged->get_script_instance());
 
-			if (cs_instance) {
-				return cs_instance->get_mono_object();
-			}
+	if (!unmanaged)
+		return NULL;
+
+	if (unmanaged->get_script_instance()) {
+		CSharpInstance *cs_instance = CAST_CSHARP_INSTANCE(unmanaged->get_script_instance());
+
+		if (cs_instance) {
+			return cs_instance->get_mono_object();
 		}
+	}
+
+	// If the owner does not have a CSharpInstance...
+
+	void *data = unmanaged->get_script_instance_binding(CSharpLanguage::get_singleton()->get_language_index());
+
+	if (!data)
+		return NULL;
 
-		// If the owner does not have a CSharpInstance...
+	CSharpScriptBinding &script_binding = ((Map<Object *, CSharpScriptBinding>::Element *)data)->value();
 
-		void *data = unmanaged->get_script_instance_binding(CSharpLanguage::get_singleton()->get_language_index());
+	if (!script_binding.inited) {
+		// Already had a binding that needs to be setup
+		CSharpLanguage::get_singleton()->setup_csharp_script_binding(script_binding, unmanaged);
 
-		if (data) {
-			CSharpScriptBinding &script_binding = ((Map<Object *, CSharpScriptBinding>::Element *)data)->value();
+		if (!script_binding.inited)
+			return NULL;
+	}
 
-			Ref<MonoGCHandle> &gchandle = script_binding.gchandle;
-			ERR_FAIL_COND_V(gchandle.is_null(), NULL);
+	Ref<MonoGCHandle> &gchandle = script_binding.gchandle;
+	ERR_FAIL_COND_V(gchandle.is_null(), NULL);
 
-			MonoObject *target = gchandle->get_target();
+	MonoObject *target = gchandle->get_target();
 
-			if (target)
-				return target;
+	if (target)
+		return target;
 
-			CSharpLanguage::get_singleton()->release_script_gchandle(gchandle);
+	CSharpLanguage::get_singleton()->release_script_gchandle(gchandle);
 
-			// Create a new one
+	// Create a new one
 
 #ifdef DEBUG_ENABLED
-			CRASH_COND(script_binding.type_name == StringName());
-			CRASH_COND(script_binding.wrapper_class == NULL);
+	CRASH_COND(script_binding.type_name == StringName());
+	CRASH_COND(script_binding.wrapper_class == NULL);
 #endif
 
-			MonoObject *mono_object = GDMonoUtils::create_managed_for_godot_object(script_binding.wrapper_class, script_binding.type_name, unmanaged);
-			ERR_FAIL_NULL_V(mono_object, NULL);
-
-			gchandle->set_handle(MonoGCHandle::new_strong_handle(mono_object), MonoGCHandle::STRONG_HANDLE);
+	MonoObject *mono_object = GDMonoUtils::create_managed_for_godot_object(script_binding.wrapper_class, script_binding.type_name, unmanaged);
+	ERR_FAIL_NULL_V(mono_object, NULL);
 
-			// Tie managed to unmanaged
-			Reference *ref = Object::cast_to<Reference>(unmanaged);
+	gchandle->set_handle(MonoGCHandle::new_strong_handle(mono_object), MonoGCHandle::STRONG_HANDLE);
 
-			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)
+	// Tie managed to unmanaged
+	Reference *ref = Object::cast_to<Reference>(unmanaged);
 
-				ref->reference();
-			}
+	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)
 
-			return mono_object;
-		}
+		ref->reference();
 	}
 
-	return NULL;
+	return mono_object;
 }
 
 void set_main_thread(MonoThread *p_thread) {