From 0b814ea78d6b065e8e0785155207bd80b3d845c8 Mon Sep 17 00:00:00 2001 From: Ignacio Etcheverry Date: Wed, 11 Dec 2019 17:08:40 +0100 Subject: [PATCH] Mono/C#: Optimize the way we store GC handles for scripts Don't store GC handles for C# script instances and instance bindings as 'Ref'; store the raw data instead. Initially this was not possible as we needed to store a Variant, but this had not been the case for a looong time yet the stored type was never updated. --- modules/mono/csharp_script.cpp | 109 ++++++++++----------- modules/mono/csharp_script.h | 16 ++- modules/mono/glue/base_object_glue.cpp | 8 +- modules/mono/managed_callable.cpp | 18 ++-- modules/mono/managed_callable.h | 6 +- modules/mono/mono_gc_handle.cpp | 67 +++++-------- modules/mono/mono_gc_handle.h | 93 ++++++++++++------ modules/mono/mono_gd/gd_mono_cache.cpp | 4 +- modules/mono/mono_gd/gd_mono_cache.h | 2 +- modules/mono/mono_gd/gd_mono_internals.cpp | 4 +- modules/mono/mono_gd/gd_mono_utils.cpp | 23 ++++- modules/mono/mono_gd/gd_mono_utils.h | 5 + modules/mono/signal_awaiter_utils.cpp | 16 ++- modules/mono/signal_awaiter_utils.h | 3 +- 14 files changed, 212 insertions(+), 162 deletions(-) diff --git a/modules/mono/csharp_script.cpp b/modules/mono/csharp_script.cpp index c3608693f1..cb17defc4a 100644 --- a/modules/mono/csharp_script.cpp +++ b/modules/mono/csharp_script.cpp @@ -150,8 +150,8 @@ void CSharpLanguage::finish() { for (Map::Element *E = script_bindings.front(); E; E = E->next()) { CSharpScriptBinding &script_binding = E->value(); - if (script_binding.gchandle.is_valid()) { - script_binding.gchandle->release(); + if (!script_binding.gchandle.is_released()) { + script_binding.gchandle.release(); script_binding.inited = false; } } @@ -676,7 +676,7 @@ void CSharpLanguage::pre_unsafe_unreference(Object *p_obj) { void CSharpLanguage::frame() { if (gdmono && gdmono->is_runtime_initialized() && gdmono->get_core_api_assembly() != NULL) { - const Ref &task_scheduler_handle = GDMonoCache::cached_data.task_scheduler_handle; + const Ref &task_scheduler_handle = GDMonoCache::cached_data.task_scheduler_handle; if (task_scheduler_handle.is_valid()) { MonoObject *task_scheduler = task_scheduler_handle->get_target(); @@ -804,7 +804,7 @@ void CSharpLanguage::reload_assemblies(bool p_soft_reload) { for (SelfList *elem = ManagedCallable::instances.first(); elem; elem = elem->next()) { ManagedCallable *managed_callable = elem->self(); - MonoDelegate *delegate = (MonoDelegate *)managed_callable->delegate_handle->get_target(); + MonoDelegate *delegate = (MonoDelegate *)managed_callable->delegate_handle.get_target(); Array serialized_data; MonoObject *managed_serialized_data = GDMonoMarshal::variant_to_mono_object(serialized_data); @@ -1278,6 +1278,7 @@ bool CSharpLanguage::debug_break(const String &p_error, bool p_allow_continue) { void CSharpLanguage::_on_scripts_domain_unloaded() { for (Map::Element *E = script_bindings.front(); E; E = E->next()) { CSharpScriptBinding &script_binding = E->value(); + script_binding.gchandle.release(); script_binding.inited = false; } @@ -1286,7 +1287,7 @@ void CSharpLanguage::_on_scripts_domain_unloaded() { for (SelfList *elem = ManagedCallable::instances.first(); elem; elem = elem->next()) { ManagedCallable *managed_callable = elem->self(); - managed_callable->delegate_handle.unref(); + managed_callable->delegate_handle.release(); managed_callable->delegate_invoke = NULL; } } @@ -1328,33 +1329,33 @@ void CSharpLanguage::set_language_index(int p_idx) { lang_idx = p_idx; } -void CSharpLanguage::release_script_gchandle(Ref &p_gchandle) { +void CSharpLanguage::release_script_gchandle(MonoGCHandleData &p_gchandle) { - if (!p_gchandle->is_released()) { // Do not lock unnecessarily + if (!p_gchandle.is_released()) { // Do not lock unnecessarily MutexLock lock(get_singleton()->script_gchandle_release_mutex); - p_gchandle->release(); + p_gchandle.release(); } } -void CSharpLanguage::release_script_gchandle(MonoObject *p_expected_obj, Ref &p_gchandle) { +void CSharpLanguage::release_script_gchandle(MonoObject *p_expected_obj, MonoGCHandleData &p_gchandle) { - uint32_t pinned_gchandle = MonoGCHandle::new_strong_handle_pinned(p_expected_obj); // We might lock after this, so pin it + uint32_t pinned_gchandle = GDMonoUtils::new_strong_gchandle_pinned(p_expected_obj); // We might lock after this, so pin it - if (!p_gchandle->is_released()) { // Do not lock unnecessarily + if (!p_gchandle.is_released()) { // Do not lock unnecessarily MutexLock lock(get_singleton()->script_gchandle_release_mutex); - MonoObject *target = p_gchandle->get_target(); + MonoObject *target = p_gchandle.get_target(); // We release the gchandle if it points to the MonoObject* we expect (otherwise it was // already released and could have been replaced) or if we can't get its target MonoObject* // (which doesn't necessarily mean it was released, and we want it released in order to // avoid locking other threads unnecessarily). if (target == p_expected_obj || target == NULL) { - p_gchandle->release(); + p_gchandle.release(); } } - MonoGCHandle::free_handle(pinned_gchandle); + GDMonoUtils::free_gchandle(pinned_gchandle); } CSharpLanguage::CSharpLanguage() { @@ -1399,7 +1400,7 @@ bool CSharpLanguage::setup_csharp_script_binding(CSharpScriptBinding &r_script_b r_script_binding.inited = true; r_script_binding.type_name = type_name; r_script_binding.wrapper_class = type_class; // cache - r_script_binding.gchandle = MonoGCHandle::create_strong(mono_object); + r_script_binding.gchandle = MonoGCHandleData::new_strong_handle(mono_object); r_script_binding.owner = p_object; // Tie managed to unmanaged @@ -1464,10 +1465,11 @@ void CSharpLanguage::free_instance_binding_data(void *p_data) { if (script_binding.inited) { // Set the native instance field to IntPtr.Zero, if not yet garbage collected. // This is done to avoid trying to dispose the native instance from Dispose(bool). - MonoObject *mono_object = script_binding.gchandle->get_target(); + MonoObject *mono_object = script_binding.gchandle.get_target(); if (mono_object) { CACHED_FIELD(GodotObject, ptr)->set_value_raw(mono_object, NULL); } + script_binding.gchandle.release(); } script_bindings.erase(data); @@ -1487,26 +1489,26 @@ void CSharpLanguage::refcount_incremented_instance_binding(Object *p_object) { CRASH_COND(!data); CSharpScriptBinding &script_binding = ((Map::Element *)data)->get(); - Ref &gchandle = script_binding.gchandle; + MonoGCHandleData &gchandle = script_binding.gchandle; if (!script_binding.inited) return; - if (ref_owner->reference_get_count() > 1 && gchandle->is_weak()) { // The managed side also holds a reference, hence 1 instead of 0 + if (ref_owner->reference_get_count() > 1 && gchandle.is_weak()) { // The managed side also holds a reference, hence 1 instead of 0 GD_MONO_SCOPE_THREAD_ATTACH; // The reference count was increased after the managed side was the only one referencing our owner. // This means the owner is being referenced again by the unmanaged side, // so the owner must hold the managed side alive again to avoid it from being GCed. - MonoObject *target = gchandle->get_target(); + MonoObject *target = gchandle.get_target(); if (!target) return; // Called after the managed side was collected, so nothing to do here // Release the current weak handle and replace it with a strong handle. - uint32_t strong_gchandle = MonoGCHandle::new_strong_handle(target); - gchandle->release(); - gchandle->set_handle(strong_gchandle, MonoGCHandle::STRONG_HANDLE); + MonoGCHandleData strong_gchandle = MonoGCHandleData::new_strong_handle(target); + gchandle.release(); + gchandle = strong_gchandle; } } @@ -1523,27 +1525,27 @@ bool CSharpLanguage::refcount_decremented_instance_binding(Object *p_object) { CRASH_COND(!data); CSharpScriptBinding &script_binding = ((Map::Element *)data)->get(); - Ref &gchandle = script_binding.gchandle; + MonoGCHandleData &gchandle = script_binding.gchandle; int refcount = ref_owner->reference_get_count(); if (!script_binding.inited) return refcount == 0; - if (refcount == 1 && gchandle.is_valid() && !gchandle->is_weak()) { // The managed side also holds a reference, hence 1 instead of 0 + if (refcount == 1 && !gchandle.is_released() && !gchandle.is_weak()) { // The managed side also holds a reference, hence 1 instead of 0 GD_MONO_SCOPE_THREAD_ATTACH; // If owner owner is no longer referenced by the unmanaged side, // the managed instance takes responsibility of deleting the owner when GCed. - MonoObject *target = gchandle->get_target(); + MonoObject *target = gchandle.get_target(); if (!target) return refcount == 0; // Called after the managed side was collected, so nothing to do here // Release the current strong handle and replace it with a weak handle. - uint32_t weak_gchandle = MonoGCHandle::new_weak_handle(target); - gchandle->release(); - gchandle->set_handle(weak_gchandle, MonoGCHandle::WEAK_HANDLE); + MonoGCHandleData weak_gchandle = MonoGCHandleData::new_weak_handle(target); + gchandle.release(); + gchandle = weak_gchandle; return false; } @@ -1551,7 +1553,7 @@ bool CSharpLanguage::refcount_decremented_instance_binding(Object *p_object) { return refcount == 0; } -CSharpInstance *CSharpInstance::create_for_managed_type(Object *p_owner, CSharpScript *p_script, const Ref &p_gchandle) { +CSharpInstance *CSharpInstance::create_for_managed_type(Object *p_owner, CSharpScript *p_script, const MonoGCHandleData &p_gchandle) { CSharpInstance *instance = memnew(CSharpInstance(Ref(p_script))); @@ -1571,8 +1573,8 @@ CSharpInstance *CSharpInstance::create_for_managed_type(Object *p_owner, CSharpS MonoObject *CSharpInstance::get_mono_object() const { - ERR_FAIL_COND_V(gchandle.is_null(), NULL); - return gchandle->get_target(); + ERR_FAIL_COND_V(gchandle.is_released(), NULL); + return gchandle.get_target(); } Object *CSharpInstance::get_owner() { @@ -1945,10 +1947,6 @@ bool CSharpInstance::_unreference_owner_unsafe() { } MonoObject *CSharpInstance::_internal_new_managed() { -#ifdef DEBUG_ENABLED - CRASH_COND(!gchandle.is_valid()); -#endif - // Search the constructor first, to fail with an error if it's not found before allocating anything else. GDMonoMethod *ctor = script->script_class->get_method(CACHED_STRING_NAME(dotctor), 0); ERR_FAIL_NULL_V_MSG(ctor, NULL, @@ -1975,7 +1973,7 @@ MonoObject *CSharpInstance::_internal_new_managed() { } // Tie managed to unmanaged - gchandle = MonoGCHandle::create_strong(mono_object); + gchandle = MonoGCHandleData::new_strong_handle(mono_object); if (base_ref) _reference_owner_unsafe(); // Here, after assigning the gchandle (for the refcount_incremented callback) @@ -1994,7 +1992,7 @@ void CSharpInstance::mono_object_disposed(MonoObject *p_obj) { #ifdef DEBUG_ENABLED CRASH_COND(base_ref); - CRASH_COND(gchandle.is_null()); + CRASH_COND(gchandle.is_released()); #endif CSharpLanguage::get_singleton()->release_script_gchandle(p_obj, gchandle); } @@ -2003,7 +2001,7 @@ void CSharpInstance::mono_object_disposed_baseref(MonoObject *p_obj, bool p_is_f #ifdef DEBUG_ENABLED CRASH_COND(!base_ref); - CRASH_COND(gchandle.is_null()); + CRASH_COND(gchandle.is_released()); #endif r_remove_script_instance = false; @@ -2069,7 +2067,7 @@ void CSharpInstance::refcount_incremented() { Reference *ref_owner = Object::cast_to(owner); - if (ref_owner->reference_get_count() > 1 && gchandle->is_weak()) { // The managed side also holds a reference, hence 1 instead of 0 + if (ref_owner->reference_get_count() > 1 && gchandle.is_weak()) { // The managed side also holds a reference, hence 1 instead of 0 GD_MONO_SCOPE_THREAD_ATTACH; // The reference count was increased after the managed side was the only one referencing our owner. @@ -2077,9 +2075,9 @@ void CSharpInstance::refcount_incremented() { // so the owner must hold the managed side alive again to avoid it from being GCed. // Release the current weak handle and replace it with a strong handle. - uint32_t strong_gchandle = MonoGCHandle::new_strong_handle(gchandle->get_target()); - gchandle->release(); - gchandle->set_handle(strong_gchandle, MonoGCHandle::STRONG_HANDLE); + MonoGCHandleData strong_gchandle = MonoGCHandleData::new_strong_handle(gchandle.get_target()); + gchandle.release(); + gchandle = strong_gchandle; } } @@ -2094,16 +2092,16 @@ bool CSharpInstance::refcount_decremented() { int refcount = ref_owner->reference_get_count(); - if (refcount == 1 && !gchandle->is_weak()) { // The managed side also holds a reference, hence 1 instead of 0 + if (refcount == 1 && !gchandle.is_weak()) { // The managed side also holds a reference, hence 1 instead of 0 GD_MONO_SCOPE_THREAD_ATTACH; // If owner owner is no longer referenced by the unmanaged side, // the managed instance takes responsibility of deleting the owner when GCed. // Release the current strong handle and replace it with a weak handle. - uint32_t weak_gchandle = MonoGCHandle::new_weak_handle(gchandle->get_target()); - gchandle->release(); - gchandle->set_handle(weak_gchandle, MonoGCHandle::WEAK_HANDLE); + MonoGCHandleData weak_gchandle = MonoGCHandleData::new_weak_handle(gchandle.get_target()); + gchandle.release(); + gchandle = weak_gchandle; return false; } @@ -2269,7 +2267,7 @@ CSharpInstance::~CSharpInstance() { destructing_script_instance = true; - if (gchandle.is_valid()) { + if (!gchandle.is_released()) { if (!predelete_notified && !ref_dying) { // This destructor is not called from the owners destructor. // This could be being called from the owner's set_script_instance method, @@ -2277,7 +2275,7 @@ CSharpInstance::~CSharpInstance() { // we must call Dispose here, because Dispose calls owner->set_script_instance(NULL) // and that would mess up with the new script instance if called later. - MonoObject *mono_object = gchandle->get_target(); + MonoObject *mono_object = gchandle.get_target(); if (mono_object) { MonoException *exc = NULL; @@ -2289,7 +2287,7 @@ CSharpInstance::~CSharpInstance() { } } - gchandle->release(); // Make sure the gchandle is released + gchandle.release(); // Make sure the gchandle is released } // If not being called from the owner's destructor, and we still hold a reference to the owner @@ -2449,7 +2447,7 @@ bool CSharpScript::_update_exports() { return false; } - uint32_t tmp_pinned_gchandle = MonoGCHandle::new_strong_handle_pinned(tmp_object); // pin it (not sure if needed) + uint32_t tmp_pinned_gchandle = GDMonoUtils::new_strong_gchandle_pinned(tmp_object); // pin it (not sure if needed) GDMonoMethod *ctor = script_class->get_method(CACHED_STRING_NAME(dotctor), 0); @@ -2464,7 +2462,7 @@ bool CSharpScript::_update_exports() { if (ctor_exc) { // TODO: Should we free 'tmp_native' if the exception was thrown after its creation? - MonoGCHandle::free_handle(tmp_pinned_gchandle); + GDMonoUtils::free_gchandle(tmp_pinned_gchandle); tmp_object = NULL; ERR_PRINT("Exception thrown from constructor of temporary MonoObject:"); @@ -2543,7 +2541,7 @@ bool CSharpScript::_update_exports() { GDMonoUtils::debug_print_unhandled_exception(exc); } - MonoGCHandle::free_handle(tmp_pinned_gchandle); + GDMonoUtils::free_gchandle(tmp_pinned_gchandle); tmp_object = NULL; if (tmp_native && !base_ref) { @@ -3109,8 +3107,8 @@ CSharpInstance *CSharpScript::_create_instance(const Variant **p_args, int p_arg CRASH_COND(data == NULL); CSharpScriptBinding &script_binding = ((Map::Element *)data)->get(); - if (script_binding.inited && script_binding.gchandle.is_valid()) { - MonoObject *mono_object = script_binding.gchandle->get_target(); + if (script_binding.inited && !script_binding.gchandle.is_released()) { + MonoObject *mono_object = script_binding.gchandle.get_target(); if (mono_object) { MonoException *exc = NULL; GDMonoUtils::dispose(mono_object, &exc); @@ -3120,6 +3118,7 @@ CSharpInstance *CSharpScript::_create_instance(const Variant **p_args, int p_arg } } + script_binding.gchandle.release(); // Just in case script_binding.inited = false; } } @@ -3148,7 +3147,7 @@ CSharpInstance *CSharpScript::_create_instance(const Variant **p_args, int p_arg } // Tie managed to unmanaged - instance->gchandle = MonoGCHandle::create_strong(mono_object); + instance->gchandle = MonoGCHandleData::new_strong_handle(mono_object); if (instance->base_ref) instance->_reference_owner_unsafe(); // Here, after assigning the gchandle (for the refcount_incremented callback) diff --git a/modules/mono/csharp_script.h b/modules/mono/csharp_script.h index 77f9ddb71a..53b4aa6c20 100644 --- a/modules/mono/csharp_script.h +++ b/modules/mono/csharp_script.h @@ -240,7 +240,7 @@ class CSharpInstance : public ScriptInstance { bool destructing_script_instance = false; Ref script; - Ref gchandle; + MonoGCHandleData gchandle; Vector event_signal_callables; @@ -258,7 +258,7 @@ class CSharpInstance : public ScriptInstance { // Do not use unless you know what you are doing friend void GDMonoInternals::tie_managed_to_unmanaged(MonoObject *, Object *); - static CSharpInstance *create_for_managed_type(Object *p_owner, CSharpScript *p_script, const Ref &p_gchandle); + static CSharpInstance *create_for_managed_type(Object *p_owner, CSharpScript *p_script, const MonoGCHandleData &p_gchandle); void _call_multilevel(MonoObject *p_mono_object, const StringName &p_method, const Variant **p_args, int p_argcount); @@ -326,8 +326,14 @@ struct CSharpScriptBinding { bool inited; StringName type_name; GDMonoClass *wrapper_class; - Ref gchandle; + MonoGCHandleData gchandle; Object *owner; + + CSharpScriptBinding() : + inited(false), + wrapper_class(NULL), + owner(NULL) { + } }; class ManagedCallableMiddleman : public Object { @@ -414,8 +420,8 @@ public: _FORCE_INLINE_ EditorPlugin *get_godotsharp_editor() const { return godotsharp_editor; } #endif - static void release_script_gchandle(Ref &p_gchandle); - static void release_script_gchandle(MonoObject *p_expected_obj, Ref &p_gchandle); + static void release_script_gchandle(MonoGCHandleData &p_gchandle); + static void release_script_gchandle(MonoObject *p_expected_obj, MonoGCHandleData &p_gchandle); bool debug_break(const String &p_error, bool p_allow_continue = true); bool debug_break_parse(const String &p_file, int p_line, const String &p_error); diff --git a/modules/mono/glue/base_object_glue.cpp b/modules/mono/glue/base_object_glue.cpp index f7a2e7f1e7..120668d1ef 100644 --- a/modules/mono/glue/base_object_glue.cpp +++ b/modules/mono/glue/base_object_glue.cpp @@ -70,8 +70,8 @@ void godot_icall_Object_Disposed(MonoObject *p_obj, Object *p_ptr) { if (data) { CSharpScriptBinding &script_binding = ((Map::Element *)data)->get(); if (script_binding.inited) { - Ref &gchandle = script_binding.gchandle; - if (gchandle.is_valid()) { + MonoGCHandleData &gchandle = script_binding.gchandle; + if (!gchandle.is_released()) { CSharpLanguage::release_script_gchandle(p_obj, gchandle); } } @@ -117,8 +117,8 @@ void godot_icall_Reference_Disposed(MonoObject *p_obj, Object *p_ptr, MonoBoolea if (data) { CSharpScriptBinding &script_binding = ((Map::Element *)data)->get(); if (script_binding.inited) { - Ref &gchandle = script_binding.gchandle; - if (gchandle.is_valid()) { + MonoGCHandleData &gchandle = script_binding.gchandle; + if (!gchandle.is_released()) { CSharpLanguage::release_script_gchandle(p_obj, gchandle); } } diff --git a/modules/mono/managed_callable.cpp b/modules/mono/managed_callable.cpp index 75420612fc..a9cf64d1cc 100644 --- a/modules/mono/managed_callable.cpp +++ b/modules/mono/managed_callable.cpp @@ -44,8 +44,8 @@ bool ManagedCallable::compare_equal(const CallableCustom *p_a, const CallableCus const ManagedCallable *a = static_cast(p_a); const ManagedCallable *b = static_cast(p_b); - MonoDelegate *delegate_a = (MonoDelegate *)a->delegate_handle->get_target(); - MonoDelegate *delegate_b = (MonoDelegate *)b->delegate_handle->get_target(); + MonoDelegate *delegate_a = (MonoDelegate *)a->delegate_handle.get_target(); + MonoDelegate *delegate_b = (MonoDelegate *)b->delegate_handle.get_target(); if (!delegate_a || !delegate_b) { if (!delegate_a && !delegate_b) @@ -66,7 +66,7 @@ bool ManagedCallable::compare_less(const CallableCustom *p_a, const CallableCust uint32_t ManagedCallable::hash() const { // hmm uint32_t hash = delegate_invoke->get_name().hash(); - return hash_djb2_one_64(delegate_handle.ptr()->handle, hash); + return hash_djb2_one_64(delegate_handle.handle, hash); } String ManagedCallable::get_as_text() const { @@ -92,12 +92,12 @@ void ManagedCallable::call(const Variant **p_arguments, int p_argcount, Variant #ifdef GD_MONO_HOT_RELOAD // Lost during hot-reload ERR_FAIL_NULL(delegate_invoke); - ERR_FAIL_COND(delegate_handle.is_null()); + ERR_FAIL_COND(delegate_handle.is_released()); #endif ERR_FAIL_COND(delegate_invoke->get_parameters_count() < p_argcount); - MonoObject *delegate = delegate_handle->get_target(); + MonoObject *delegate = delegate_handle.get_target(); MonoException *exc = NULL; MonoObject *ret = delegate_invoke->invoke(delegate, p_arguments, &exc); @@ -111,7 +111,7 @@ void ManagedCallable::call(const Variant **p_arguments, int p_argcount, Variant } void ManagedCallable::set_delegate(MonoDelegate *p_delegate) { - delegate_handle = MonoGCHandle::create_strong((MonoObject *)p_delegate); + delegate_handle = MonoGCHandleData::new_strong_handle((MonoObject *)p_delegate); MonoMethod *delegate_invoke_raw = mono_get_delegate_invoke(mono_object_get_class((MonoObject *)p_delegate)); const StringName &delegate_invoke_name = CSharpLanguage::get_singleton()->get_string_names().delegate_invoke_method_name; delegate_invoke = memnew(GDMonoMethod(delegate_invoke_name, delegate_invoke_raw)); // TODO: Use pooling for this GDMonoMethod instances @@ -132,12 +132,14 @@ ManagedCallable::ManagedCallable(MonoDelegate *p_delegate) { #endif } -#ifdef GD_MONO_HOT_RELOAD ManagedCallable::~ManagedCallable() { +#ifdef GD_MONO_HOT_RELOAD { MutexLock lock(instances_mutex); instances.remove(&self_instance); instances_pending_reload.erase(this); } -} #endif + + delegate_handle.release(); +} diff --git a/modules/mono/managed_callable.h b/modules/mono/managed_callable.h index 34963a82ea..4f71e14a2f 100644 --- a/modules/mono/managed_callable.h +++ b/modules/mono/managed_callable.h @@ -42,7 +42,7 @@ class ManagedCallable : public CallableCustom { friend class CSharpLanguage; - Ref delegate_handle; + MonoGCHandleData delegate_handle; GDMonoMethod *delegate_invoke; #ifdef GD_MONO_HOT_RELOAD @@ -60,7 +60,7 @@ public: ObjectID get_object() const override; void call(const Variant **p_arguments, int p_argcount, Variant &r_return_value, Callable::CallError &r_call_error) const override; - _FORCE_INLINE_ MonoDelegate *get_delegate() { return (MonoDelegate *)delegate_handle->get_target(); } + _FORCE_INLINE_ MonoDelegate *get_delegate() { return (MonoDelegate *)delegate_handle.get_target(); } void set_delegate(MonoDelegate *p_delegate); @@ -71,9 +71,7 @@ public: static constexpr CompareEqualFunc compare_less_func_ptr = &ManagedCallable::compare_less; ManagedCallable(MonoDelegate *p_delegate); -#ifdef GD_MONO_HOT_RELOAD ~ManagedCallable(); -#endif }; #endif // MANAGED_CALLABLE_H diff --git a/modules/mono/mono_gc_handle.cpp b/modules/mono/mono_gc_handle.cpp index feeea848ee..4b6d7269e9 100644 --- a/modules/mono/mono_gc_handle.cpp +++ b/modules/mono/mono_gc_handle.cpp @@ -32,56 +32,35 @@ #include "mono_gd/gd_mono.h" -uint32_t MonoGCHandle::new_strong_handle(MonoObject *p_object) { - - return mono_gchandle_new(p_object, /* pinned: */ false); -} - -uint32_t MonoGCHandle::new_strong_handle_pinned(MonoObject *p_object) { - - return mono_gchandle_new(p_object, /* pinned: */ true); -} - -uint32_t MonoGCHandle::new_weak_handle(MonoObject *p_object) { - - return mono_gchandle_new_weakref(p_object, /* track_resurrection: */ false); -} - -void MonoGCHandle::free_handle(uint32_t p_gchandle) { - - mono_gchandle_free(p_gchandle); -} - -Ref MonoGCHandle::create_strong(MonoObject *p_object) { - - return memnew(MonoGCHandle(new_strong_handle(p_object), STRONG_HANDLE)); -} - -Ref MonoGCHandle::create_weak(MonoObject *p_object) { - - return memnew(MonoGCHandle(new_weak_handle(p_object), WEAK_HANDLE)); -} - -void MonoGCHandle::release() { - +void MonoGCHandleData::release() { #ifdef DEBUG_ENABLED - CRASH_COND(!released && GDMono::get_singleton() == NULL); + CRASH_COND(handle && GDMono::get_singleton() == NULL); #endif - if (!released && GDMono::get_singleton()->is_runtime_initialized()) { - free_handle(handle); - released = true; + if (handle && GDMono::get_singleton()->is_runtime_initialized()) { + GDMonoUtils::free_gchandle(handle); + handle = 0; } } -MonoGCHandle::MonoGCHandle(uint32_t p_handle, HandleType p_handle_type) { - - released = false; - weak = p_handle_type == WEAK_HANDLE; - handle = p_handle; +MonoGCHandleData MonoGCHandleData::new_strong_handle(MonoObject *p_object) { + return MonoGCHandleData(GDMonoUtils::new_strong_gchandle(p_object), gdmono::GCHandleType::STRONG_HANDLE); } -MonoGCHandle::~MonoGCHandle() { - - release(); +MonoGCHandleData MonoGCHandleData::new_strong_handle_pinned(MonoObject *p_object) { + return MonoGCHandleData(GDMonoUtils::new_strong_gchandle_pinned(p_object), gdmono::GCHandleType::STRONG_HANDLE); +} + +MonoGCHandleData MonoGCHandleData::new_weak_handle(MonoObject *p_object) { + return MonoGCHandleData(GDMonoUtils::new_weak_gchandle(p_object), gdmono::GCHandleType::WEAK_HANDLE); +} + +Ref MonoGCHandleRef::create_strong(MonoObject *p_object) { + + return memnew(MonoGCHandleRef(MonoGCHandleData::new_strong_handle(p_object))); +} + +Ref MonoGCHandleRef::create_weak(MonoObject *p_object) { + + return memnew(MonoGCHandleRef(MonoGCHandleData::new_weak_handle(p_object))); } diff --git a/modules/mono/mono_gc_handle.h b/modules/mono/mono_gc_handle.h index 8f664dadd0..705b2265ba 100644 --- a/modules/mono/mono_gc_handle.h +++ b/modules/mono/mono_gc_handle.h @@ -35,44 +35,79 @@ #include "core/reference.h" -class MonoGCHandle : public Reference { +namespace gdmono { - GDCLASS(MonoGCHandle, Reference); +enum class GCHandleType : char { + NIL, + STRONG_HANDLE, + WEAK_HANDLE +}; - bool released; - bool weak; +} - friend class ManagedCallable; +// Manual release of the GC handle must be done when using this struct +struct MonoGCHandleData { uint32_t handle; + gdmono::GCHandleType type; -public: - enum HandleType { - STRONG_HANDLE, - WEAK_HANDLE - }; + _FORCE_INLINE_ bool is_released() const { return !handle; } + _FORCE_INLINE_ bool is_weak() const { return type == gdmono::GCHandleType::WEAK_HANDLE; } - static uint32_t new_strong_handle(MonoObject *p_object); - static uint32_t new_strong_handle_pinned(MonoObject *p_object); - static uint32_t new_weak_handle(MonoObject *p_object); - static void free_handle(uint32_t p_gchandle); + _FORCE_INLINE_ MonoObject *get_target() const { return handle ? mono_gchandle_get_target(handle) : NULL; } - static Ref create_strong(MonoObject *p_object); - static Ref create_weak(MonoObject *p_object); - - _FORCE_INLINE_ bool is_released() { return released; } - _FORCE_INLINE_ bool is_weak() { return weak; } - - _FORCE_INLINE_ MonoObject *get_target() const { return released ? NULL : mono_gchandle_get_target(handle); } - - _FORCE_INLINE_ void set_handle(uint32_t p_handle, HandleType p_handle_type) { - released = false; - weak = p_handle_type == WEAK_HANDLE; - handle = p_handle; - } void release(); - MonoGCHandle(uint32_t p_handle, HandleType p_handle_type); - ~MonoGCHandle(); + MonoGCHandleData &operator=(const MonoGCHandleData &p_other) { +#ifdef DEBUG_ENABLED + CRASH_COND(!is_released()); +#endif + handle = p_other.handle; + type = p_other.type; + return *this; + } + + MonoGCHandleData(const MonoGCHandleData &) = default; + + MonoGCHandleData() : + handle(0), + type(gdmono::GCHandleType::NIL) { + } + + MonoGCHandleData(uint32_t p_handle, gdmono::GCHandleType p_type) : + handle(p_handle), + type(p_type) { + } + + static MonoGCHandleData new_strong_handle(MonoObject *p_object); + static MonoGCHandleData new_strong_handle_pinned(MonoObject *p_object); + static MonoGCHandleData new_weak_handle(MonoObject *p_object); +}; + +class MonoGCHandleRef : public Reference { + + GDCLASS(MonoGCHandleRef, Reference); + + MonoGCHandleData data; + +public: + static Ref create_strong(MonoObject *p_object); + static Ref create_weak(MonoObject *p_object); + + _FORCE_INLINE_ bool is_released() const { return data.is_released(); } + _FORCE_INLINE_ bool is_weak() const { return data.is_weak(); } + + _FORCE_INLINE_ MonoObject *get_target() const { return data.get_target(); } + + void release() { data.release(); } + + _FORCE_INLINE_ void set_handle(uint32_t p_handle, gdmono::GCHandleType p_handle_type) { + data = MonoGCHandleData(p_handle, p_handle_type); + } + + MonoGCHandleRef(const MonoGCHandleData &p_gc_handle_data) : + data(p_gc_handle_data) { + } + ~MonoGCHandleRef() { release(); } }; #endif // CSHARP_GC_HANDLE_H diff --git a/modules/mono/mono_gd/gd_mono_cache.cpp b/modules/mono/mono_gd/gd_mono_cache.cpp index 7a3234b69b..418de87b5d 100644 --- a/modules/mono/mono_gd/gd_mono_cache.cpp +++ b/modules/mono/mono_gd/gd_mono_cache.cpp @@ -186,7 +186,7 @@ void CachedData::clear_godot_api_cache() { // End of MarshalUtils methods - task_scheduler_handle = Ref(); + task_scheduler_handle = Ref(); } #define GODOT_API_CLASS(m_class) (GDMono::get_singleton()->get_core_api_assembly()->get_class(BINDINGS_NAMESPACE, #m_class)) @@ -316,7 +316,7 @@ void update_godot_api_cache() { // TODO Move to CSharpLanguage::init() and do handle disposal MonoObject *task_scheduler = mono_object_new(mono_domain_get(), GODOT_API_CLASS(GodotTaskScheduler)->get_mono_ptr()); GDMonoUtils::runtime_object_init(task_scheduler, GODOT_API_CLASS(GodotTaskScheduler)); - cached_data.task_scheduler_handle = MonoGCHandle::create_strong(task_scheduler); + cached_data.task_scheduler_handle = MonoGCHandleRef::create_strong(task_scheduler); cached_data.godot_api_cache_updated = true; } diff --git a/modules/mono/mono_gd/gd_mono_cache.h b/modules/mono/mono_gd/gd_mono_cache.h index 74a8d445c6..a0e1879c5a 100644 --- a/modules/mono/mono_gd/gd_mono_cache.h +++ b/modules/mono/mono_gd/gd_mono_cache.h @@ -156,7 +156,7 @@ struct CachedData { // End of MarshalUtils methods - Ref task_scheduler_handle; + Ref task_scheduler_handle; bool corlib_cache_updated; bool godot_api_cache_updated; diff --git a/modules/mono/mono_gd/gd_mono_internals.cpp b/modules/mono/mono_gd/gd_mono_internals.cpp index f343a1d646..53e642f317 100644 --- a/modules/mono/mono_gd/gd_mono_internals.cpp +++ b/modules/mono/mono_gd/gd_mono_internals.cpp @@ -75,7 +75,7 @@ void tie_managed_to_unmanaged(MonoObject *managed, Object *unmanaged) { script_binding.inited = true; script_binding.type_name = NATIVE_GDMONOCLASS_NAME(klass); script_binding.wrapper_class = klass; - script_binding.gchandle = ref ? MonoGCHandle::create_weak(managed) : MonoGCHandle::create_strong(managed); + script_binding.gchandle = ref ? MonoGCHandleData::new_weak_handle(managed) : MonoGCHandleData::new_strong_handle(managed); script_binding.owner = unmanaged; if (ref) { @@ -101,7 +101,7 @@ void tie_managed_to_unmanaged(MonoObject *managed, Object *unmanaged) { return; } - Ref gchandle = ref ? MonoGCHandle::create_weak(managed) : MonoGCHandle::create_strong(managed); + MonoGCHandleData gchandle = ref ? MonoGCHandleData::new_weak_handle(managed) : MonoGCHandleData::new_strong_handle(managed); Ref script = CSharpScript::create_for_managed_type(klass, native); diff --git a/modules/mono/mono_gd/gd_mono_utils.cpp b/modules/mono/mono_gd/gd_mono_utils.cpp index fa04183ea3..cdb26ae61b 100644 --- a/modules/mono/mono_gd/gd_mono_utils.cpp +++ b/modules/mono/mono_gd/gd_mono_utils.cpp @@ -86,10 +86,9 @@ MonoObject *unmanaged_get_managed(Object *unmanaged) { } } - Ref &gchandle = script_binding.gchandle; - ERR_FAIL_COND_V(gchandle.is_null(), NULL); + MonoGCHandleData &gchandle = script_binding.gchandle; - MonoObject *target = gchandle->get_target(); + MonoObject *target = gchandle.get_target(); if (target) return target; @@ -106,7 +105,7 @@ MonoObject *unmanaged_get_managed(Object *unmanaged) { 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); + gchandle = MonoGCHandleData::new_strong_handle(mono_object); // Tie managed to unmanaged Reference *ref = Object::cast_to(unmanaged); @@ -156,6 +155,22 @@ bool is_thread_attached() { return mono_domain_get() != NULL; } +uint32_t new_strong_gchandle(MonoObject *p_object) { + return mono_gchandle_new(p_object, /* pinned: */ false); +} + +uint32_t new_strong_gchandle_pinned(MonoObject *p_object) { + return mono_gchandle_new(p_object, /* pinned: */ true); +} + +uint32_t new_weak_gchandle(MonoObject *p_object) { + return mono_gchandle_new_weakref(p_object, /* track_resurrection: */ false); +} + +void free_gchandle(uint32_t p_gchandle) { + mono_gchandle_free(p_gchandle); +} + void runtime_object_init(MonoObject *p_this_obj, GDMonoClass *p_class, MonoException **r_exc) { GDMonoMethod *ctor = p_class->get_method(".ctor", 0); ERR_FAIL_NULL(ctor); diff --git a/modules/mono/mono_gd/gd_mono_utils.h b/modules/mono/mono_gd/gd_mono_utils.h index 7b7afc44e0..fd02907d87 100644 --- a/modules/mono/mono_gd/gd_mono_utils.h +++ b/modules/mono/mono_gd/gd_mono_utils.h @@ -92,6 +92,11 @@ _FORCE_INLINE_ bool is_main_thread() { return mono_domain_get() != NULL && mono_thread_get_main() == mono_thread_current(); } +uint32_t new_strong_gchandle(MonoObject *p_object); +uint32_t new_strong_gchandle_pinned(MonoObject *p_object); +uint32_t new_weak_gchandle(MonoObject *p_object); +void free_gchandle(uint32_t p_gchandle); + void runtime_object_init(MonoObject *p_this_obj, GDMonoClass *p_class, MonoException **r_exc = NULL); bool mono_delegate_equal(MonoDelegate *p_a, MonoDelegate *p_b); diff --git a/modules/mono/signal_awaiter_utils.cpp b/modules/mono/signal_awaiter_utils.cpp index 28719f7bc6..25e5a41215 100644 --- a/modules/mono/signal_awaiter_utils.cpp +++ b/modules/mono/signal_awaiter_utils.cpp @@ -114,8 +114,15 @@ void SignalAwaiterCallable::call(const Variant **p_arguments, int p_argcount, Va mono_array_setref(signal_args, i, boxed); } + MonoObject *awaiter = awaiter_handle.get_target(); + + if (!awaiter) { + r_call_error.error = Callable::CallError::CALL_ERROR_INSTANCE_IS_NULL; + return; + } + MonoException *exc = NULL; - CACHED_METHOD_THUNK(SignalAwaiter, SignalCallback).invoke(awaiter_handle->get_target(), signal_args, &exc); + CACHED_METHOD_THUNK(SignalAwaiter, SignalCallback).invoke(awaiter, signal_args, &exc); if (exc) { GDMonoUtils::set_pending_exception(exc); @@ -127,10 +134,14 @@ void SignalAwaiterCallable::call(const Variant **p_arguments, int p_argcount, Va SignalAwaiterCallable::SignalAwaiterCallable(Object *p_target, MonoObject *p_awaiter, const StringName &p_signal) : target_id(p_target->get_instance_id()), - awaiter_handle(MonoGCHandle::create_strong(p_awaiter)), + awaiter_handle(MonoGCHandleData::new_strong_handle(p_awaiter)), signal(p_signal) { } +SignalAwaiterCallable::~SignalAwaiterCallable() { + awaiter_handle.release(); +} + bool EventSignalCallable::compare_equal(const CallableCustom *p_a, const CallableCustom *p_b) { const EventSignalCallable *a = static_cast(p_a); const EventSignalCallable *b = static_cast(p_b); @@ -159,7 +170,6 @@ String EventSignalCallable::get_as_text() const { String class_name = owner->get_class(); Ref