From 55b2e58a98697f9d65fa1dd41cd4ae8184213f43 Mon Sep 17 00:00:00 2001 From: Ignacio Etcheverry Date: Thu, 23 Jan 2020 21:23:32 +0100 Subject: [PATCH] Mono/C#: Lighten up unsafe reference checks Because of the weird case with multi-threading and ResourceLoader, it can be the case that a resource is GCed while being referenced again in the main thread. In such cases, a new unsafe reference is created before the finalizer thread removes the previous one. --- modules/mono/csharp_script.cpp | 27 ++++++++++++++++++++++----- modules/mono/csharp_script.h | 5 +++-- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/modules/mono/csharp_script.cpp b/modules/mono/csharp_script.cpp index 210267e68181..3d868b08393b 100644 --- a/modules/mono/csharp_script.cpp +++ b/modules/mono/csharp_script.cpp @@ -160,7 +160,7 @@ void CSharpLanguage::finish() { script_bindings.clear(); #ifdef DEBUG_ENABLED - for (List::Element *E = unsafely_referenced_objects.front(); E; E = E->next()) { + for (Map::Element *E = unsafe_object_references.front(); E; E = E->next()) { const ObjectID &id = E->get(); Object *obj = ObjectDB::get_instance(id); @@ -632,18 +632,20 @@ Vector CSharpLanguage::stack_trace_get_info(MonoObjec void CSharpLanguage::post_unsafe_reference(Object *p_obj) { #ifdef DEBUG_ENABLED + SCOPED_MUTEX_LOCK(unsafe_object_references_lock); 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); + unsafe_object_references[id]++; #endif } void CSharpLanguage::pre_unsafe_unreference(Object *p_obj) { #ifdef DEBUG_ENABLED + SCOPED_MUTEX_LOCK(unsafe_object_references_lock); ObjectID id = p_obj->get_instance_id(); - List::Element *elem = unsafely_referenced_objects.find(id); + Map::Element *elem = unsafe_object_references.find(id); ERR_FAIL_NULL(elem); - unsafely_referenced_objects.erase(elem); + if (--elem->value() == 0) + unsafe_object_references.erase(elem); #endif } @@ -1246,6 +1248,14 @@ CSharpLanguage::CSharpLanguage() { language_bind_mutex = Mutex::create(); #endif +#ifdef DEBUG_ENABLED +#ifdef NO_THREADS + unsafe_object_references_lock = NULL; +#else + unsafe_object_references_lock = Mutex::create(); +#endif +#endif + lang_idx = -1; scripts_metadata_invalidated = true; @@ -1274,6 +1284,13 @@ CSharpLanguage::~CSharpLanguage() { script_gchandle_release_mutex = NULL; } +#ifdef DEBUG_ENABLED + if (unsafe_object_references_lock) { + memdelete(unsafe_object_references_lock); + unsafe_object_references_lock = NULL; + } +#endif + singleton = NULL; } diff --git a/modules/mono/csharp_script.h b/modules/mono/csharp_script.h index 30f56e00bd95..f244bc411939 100644 --- a/modules/mono/csharp_script.h +++ b/modules/mono/csharp_script.h @@ -308,8 +308,9 @@ class CSharpLanguage : public ScriptLanguage { Map script_bindings; #ifdef DEBUG_ENABLED - // List of unsafely referenced objects - List unsafely_referenced_objects; + // List of unsafe object references + Map unsafe_object_references; + Mutex *unsafe_object_references_lock; #endif struct StringNameCache {