From 3233083f63dc668b8dd21290d25a511212f114d8 Mon Sep 17 00:00:00 2001
From: Ignacio Etcheverry <ignalfonsore@gmail.com>
Date: Sun, 3 Feb 2019 06:35:22 +0100
Subject: 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.
---
 modules/mono/mono_gd/gd_mono_utils.cpp | 79 +++++++++++++++++++---------------
 1 file changed, 44 insertions(+), 35 deletions(-)

(limited to 'modules/mono/mono_gd/gd_mono_utils.cpp')

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) {
-- 
cgit v1.2.3