1 files changed, 66 insertions, 5 deletions

diff --git a/modules/mono/csharp_script.cpp b/modules/mono/csharp_script.cpp
index d82e78d080..3678a82bee 100644
--- a/modules/mono/csharp_script.cpp
+++ b/modules/mono/csharp_script.cpp
@@ -159,6 +159,19 @@ void CSharpLanguage::finish() {
 	// Clear here, after finalizing all domains to make sure there is nothing else referencing the elements.
 	script_bindings.clear();
 
+#ifdef DEBUG_ENABLED
+	for (List<ObjectID>::Element *E = unsafely_referenced_objects.front(); E; E = E->next()) {
+		const ObjectID &id = E->get();
+		Object *obj = ObjectDB::get_instance(id);
+
+		if (obj) {
+			ERR_PRINTS("Leaked unsafe reference to object: " + obj->get_class() + ":" + itos(id));
+		} else {
+			ERR_PRINTS("Leaked unsafe reference to deleted object: " + itos(id));
+		}
+	}
+#endif
+
 	finalizing = false;
 }
 
@@ -615,6 +628,23 @@ Vector<ScriptLanguage::StackInfo> CSharpLanguage::stack_trace_get_info(MonoObjec
 }
 #endif
 
+void CSharpLanguage::post_unsafe_reference(Object *p_obj) {
+#ifdef DEBUG_ENABLED
+	ObjectID id = p_obj->get_instance_id();
+	ERR_FAIL_COND_MSG(unsafely_referenced_objects.find(id), "Multiple unsafe references for object: " + p_obj->get_class() + ":" + itos(id));
+	unsafely_referenced_objects.push_back(id);
+#endif
+}
+
+void CSharpLanguage::pre_unsafe_unreference(Object *p_obj) {
+#ifdef DEBUG_ENABLED
+	ObjectID id = p_obj->get_instance_id();
+	List<ObjectID>::Element *elem = unsafely_referenced_objects.find(id);
+	ERR_FAIL_NULL(elem);
+	unsafely_referenced_objects.erase(elem);
+#endif
+}
+
 void CSharpLanguage::frame() {
 
 	if (gdmono && gdmono->is_runtime_initialized() && gdmono->get_core_api_assembly() != NULL) {
@@ -1286,6 +1316,7 @@ bool CSharpLanguage::setup_csharp_script_binding(CSharpScriptBinding &r_script_b
 		// See: godot_icall_Reference_Dtor(MonoObject *p_obj, Object *p_ptr)
 
 		ref->reference();
+		CSharpLanguage::get_singleton()->post_unsafe_reference(ref);
 	}
 
 	return true;
@@ -1736,9 +1767,12 @@ bool CSharpInstance::_reference_owner_unsafe() {
 	// See: _unreference_owner_unsafe()
 
 	// May not me referenced yet, so we must use init_ref() instead of reference()
-	bool success = Object::cast_to<Reference>(owner)->init_ref();
-	unsafe_referenced = success;
-	return success;
+	if (static_cast<Reference *>(owner)->init_ref()) {
+		CSharpLanguage::get_singleton()->post_unsafe_reference(owner);
+		unsafe_referenced = true;
+	}
+
+	return unsafe_referenced;
 }
 
 bool CSharpInstance::_unreference_owner_unsafe() {
@@ -1759,6 +1793,7 @@ bool CSharpInstance::_unreference_owner_unsafe() {
 	// See: _reference_owner_unsafe()
 
 	// Destroying the owner here means self destructing, so we defer the owner destruction to the caller.
+	CSharpLanguage::get_singleton()->pre_unsafe_unreference(owner);
 	return static_cast<Reference *>(owner)->unreference();
 }
 
@@ -2099,6 +2134,17 @@ CSharpInstance::~CSharpInstance() {
 
 		// Transfer ownership to an "instance binding"
 
+		Reference *ref_owner = static_cast<Reference *>(owner);
+
+		// We will unreference the owner before referencing it again, so we need to keep it alive
+		Ref<Reference> scope_keep_owner_alive(ref_owner);
+		(void)scope_keep_owner_alive;
+
+		// Unreference the owner here, before the new "instance binding" references it.
+		// Otherwise, the unsafe reference debug checks will incorrectly detect a bug.
+		bool die = _unreference_owner_unsafe();
+		CRASH_COND(die == true); // `owner_keep_alive` holds a reference, so it can't die
+
 		void *data = owner->get_script_instance_binding(CSharpLanguage::get_singleton()->get_language_index());
 		CRASH_COND(data == NULL);
 
@@ -2114,8 +2160,10 @@ CSharpInstance::~CSharpInstance() {
 			}
 		}
 
-		bool die = _unreference_owner_unsafe();
-		CRASH_COND(die == true); // The "instance binding" should be holding a reference
+#ifdef DEBUG_ENABLED
+		// The "instance binding" holds a reference so the refcount should be at least 2 before `scope_keep_owner_alive` goes out of scope
+		CRASH_COND(ref_owner->reference_get_count() <= 1);
+#endif
 	}
 
 	if (script.is_valid() && owner) {
@@ -2243,7 +2291,11 @@ bool CSharpScript::_update_exports() {
 		MonoException *ctor_exc = NULL;
 		ctor->invoke(tmp_object, NULL, &ctor_exc);
 
+		Object *tmp_native = GDMonoMarshal::unbox<Object *>(CACHED_FIELD(GodotObject, ptr)->get_value(tmp_object));
+
 		if (ctor_exc) {
+			// TODO: Should we free 'tmp_native' if the exception was thrown after its creation?
+
 			MonoGCHandle::free_handle(tmp_pinned_gchandle);
 			tmp_object = NULL;
 
@@ -2322,6 +2374,15 @@ bool CSharpScript::_update_exports() {
 
 		MonoGCHandle::free_handle(tmp_pinned_gchandle);
 		tmp_object = NULL;
+
+		if (tmp_native && !Object::cast_to<Reference>(tmp_native)) {
+			Node *node = Object::cast_to<Node>(tmp_native);
+			if (node && node->is_inside_tree()) {
+				ERR_PRINTS("Temporary instance was added to the scene tree.");
+			} else {
+				memdelete(tmp_native);
+			}
+		}
 	}
 
 	placeholder_fallback_enabled = false;