Merge pull request #24098 from neikeq/kk

Fix crash due to ~CSharpInstance() being called on freed instance

3 files changed, 21 insertions, 11 deletions

diff --git a/modules/mono/csharp_script.cpp b/modules/mono/csharp_script.cpp
index e3df0246ab..3c818898e6 100644
--- a/modules/mono/csharp_script.cpp
+++ b/modules/mono/csharp_script.cpp
@@ -993,7 +993,7 @@ void CSharpLanguage::set_language_index(int p_idx) {
 
 void CSharpLanguage::release_script_gchandle(Ref<MonoGCHandle> &p_gchandle) {
 
-	if (!p_gchandle->is_released()) { // Do not locking unnecessarily
+	if (!p_gchandle->is_released()) { // Do not lock unnecessarily
 		SCOPED_MUTEX_LOCK(get_singleton()->script_gchandle_release_mutex);
 		p_gchandle->release();
 	}
@@ -1003,7 +1003,7 @@ void CSharpLanguage::release_script_gchandle(MonoObject *p_expected_obj, Ref<Mon
 
 	uint32_t pinned_gchandle = MonoGCHandle::new_strong_handle_pinned(p_expected_obj); // We might lock after this, so pin it
 
-	if (!p_gchandle->is_released()) { // Do not locking unnecessarily
+	if (!p_gchandle->is_released()) { // Do not lock unnecessarily
 		SCOPED_MUTEX_LOCK(get_singleton()->script_gchandle_release_mutex);
 
 		MonoObject *target = p_gchandle->get_target();
@@ -1754,7 +1754,8 @@ CSharpInstance::CSharpInstance() :
 		base_ref(false),
 		ref_dying(false),
 		unsafe_referenced(false),
-		predelete_notified(false) {
+		predelete_notified(false),
+		destructing_script_instance(false) {
 }
 
 CSharpInstance::~CSharpInstance() {
@@ -1771,7 +1772,9 @@ CSharpInstance::~CSharpInstance() {
 
 			if (mono_object) {
 				MonoException *exc = NULL;
+				destructing_script_instance = true;
 				GDMonoUtils::dispose(mono_object, &exc);
+				destructing_script_instance = false;
 
 				if (exc) {
 					GDMonoUtils::set_pending_exception(exc);
diff --git a/modules/mono/csharp_script.h b/modules/mono/csharp_script.h
index 269d7ebe42..501e0d9d6d 100644
--- a/modules/mono/csharp_script.h
+++ b/modules/mono/csharp_script.h
@@ -198,6 +198,7 @@ class CSharpInstance : public ScriptInstance {
 	bool ref_dying;
 	bool unsafe_referenced;
 	bool predelete_notified;
+	bool destructing_script_instance;
 
 	Ref<CSharpScript> script;
 	Ref<MonoGCHandle> gchandle;
@@ -218,6 +219,8 @@ class CSharpInstance : public ScriptInstance {
 public:
 	MonoObject *get_mono_object() const;
 
+	_FORCE_INLINE_ bool is_destructing_script_instance() { return destructing_script_instance; }
+
 	virtual bool set(const StringName &p_name, const Variant &p_value);
 	virtual bool get(const StringName &p_name, Variant &r_ret) const;
 	virtual void get_property_list(List<PropertyInfo> *p_properties) const;
diff --git a/modules/mono/glue/base_object_glue.cpp b/modules/mono/glue/base_object_glue.cpp
index d718c3cc61..58916c5283 100644
--- a/modules/mono/glue/base_object_glue.cpp
+++ b/modules/mono/glue/base_object_glue.cpp
@@ -54,8 +54,10 @@ void godot_icall_Object_Disposed(MonoObject *p_obj, Object *p_ptr) {
 	if (p_ptr->get_script_instance()) {
 		CSharpInstance *cs_instance = CAST_CSHARP_INSTANCE(p_ptr->get_script_instance());
 		if (cs_instance) {
-			cs_instance->mono_object_disposed(p_obj);
-			p_ptr->set_script_instance(NULL);
+			if (!cs_instance->is_destructing_script_instance()) {
+				cs_instance->mono_object_disposed(p_obj);
+				p_ptr->set_script_instance(NULL);
+			}
 			return;
 		}
 	}
@@ -82,12 +84,14 @@ void godot_icall_Reference_Disposed(MonoObject *p_obj, Object *p_ptr, bool p_is_
 	if (ref->get_script_instance()) {
 		CSharpInstance *cs_instance = CAST_CSHARP_INSTANCE(ref->get_script_instance());
 		if (cs_instance) {
-			bool r_owner_deleted;
-			cs_instance->mono_object_disposed_baseref(p_obj, p_is_finalizer, r_owner_deleted);
-			if (!r_owner_deleted && !p_is_finalizer) {
-				// If the native instance is still alive and Dispose() was called
-				// (instead of the finalizer), then we remove the script instance.
-				ref->set_script_instance(NULL);
+			if (!cs_instance->is_destructing_script_instance()) {
+				bool r_owner_deleted;
+				cs_instance->mono_object_disposed_baseref(p_obj, p_is_finalizer, r_owner_deleted);
+				if (!r_owner_deleted && !p_is_finalizer) {
+					// If the native instance is still alive and Dispose() was called
+					// (instead of the finalizer), then we remove the script instance.
+					ref->set_script_instance(NULL);
+				}
 			}
 			return;
 		}