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.
This commit is contained in:
Ignacio Etcheverry 2019-02-03 06:35:22 +01:00
parent 4e4e889c75
commit 3233083f63
7 changed files with 257 additions and 128 deletions

View File

@ -1924,6 +1924,11 @@ void *Object::get_script_instance_binding(int p_script_language_index) {
return _script_instance_bindings[p_script_language_index]; return _script_instance_bindings[p_script_language_index];
} }
bool Object::has_script_instance_binding(int p_script_language_index) {
return _script_instance_bindings[p_script_language_index] != NULL;
}
Object::Object() { Object::Object() {
_class_ptr = NULL; _class_ptr = NULL;

View File

@ -729,6 +729,7 @@ public:
//used by script languages to store binding data //used by script languages to store binding data
void *get_script_instance_binding(int p_script_language_index); void *get_script_instance_binding(int p_script_language_index);
bool has_script_instance_binding(int p_script_language_index);
void clear_internal_resource_paths(); void clear_internal_resource_paths();

View File

@ -139,14 +139,24 @@ void CSharpLanguage::finish() {
} }
#endif #endif
// Release gchandle bindings before finalizing mono runtime // Make sure all script binding gchandles are released before finalizing GDMono
script_bindings.clear(); for (Map<Object *, CSharpScriptBinding>::Element *E = script_bindings.front(); E; E = E->next()) {
CSharpScriptBinding &script_binding = E->value();
if (script_binding.gchandle.is_valid()) {
script_binding.gchandle->release();
script_binding.inited = false;
}
}
if (gdmono) { if (gdmono) {
memdelete(gdmono); memdelete(gdmono);
gdmono = NULL; gdmono = NULL;
} }
// Clear here, after finalizing all domains to make sure there is nothing else referencing the elements.
script_bindings.clear();
finalizing = false; finalizing = false;
} }
@ -1054,12 +1064,14 @@ CSharpLanguage::~CSharpLanguage() {
singleton = NULL; singleton = NULL;
} }
void *CSharpLanguage::alloc_instance_binding_data(Object *p_object) { bool CSharpLanguage::setup_csharp_script_binding(CSharpScriptBinding &r_script_binding, Object *p_object) {
#ifdef DEBUG_ENABLED #ifdef DEBUG_ENABLED
// I don't trust you // I don't trust you
if (p_object->get_script_instance()) if (p_object->get_script_instance()) {
CRASH_COND(NULL != CAST_CSHARP_INSTANCE(p_object->get_script_instance())); CSharpInstance *csharp_instance = CAST_CSHARP_INSTANCE(p_object->get_script_instance());
CRASH_COND(csharp_instance != NULL && !csharp_instance->is_destructing_script_instance());
}
#endif #endif
StringName type_name = p_object->get_class_name(); StringName type_name = p_object->get_class_name();
@ -1068,29 +1080,21 @@ void *CSharpLanguage::alloc_instance_binding_data(Object *p_object) {
const ClassDB::ClassInfo *classinfo = ClassDB::classes.getptr(type_name); const ClassDB::ClassInfo *classinfo = ClassDB::classes.getptr(type_name);
while (classinfo && !classinfo->exposed) while (classinfo && !classinfo->exposed)
classinfo = classinfo->inherits_ptr; classinfo = classinfo->inherits_ptr;
ERR_FAIL_NULL_V(classinfo, NULL); ERR_FAIL_NULL_V(classinfo, false);
type_name = classinfo->name; type_name = classinfo->name;
GDMonoClass *type_class = GDMonoUtils::type_get_proxy_class(type_name); GDMonoClass *type_class = GDMonoUtils::type_get_proxy_class(type_name);
ERR_FAIL_NULL_V(type_class, NULL); ERR_FAIL_NULL_V(type_class, false);
MonoObject *mono_object = GDMonoUtils::create_managed_for_godot_object(type_class, type_name, p_object); MonoObject *mono_object = GDMonoUtils::create_managed_for_godot_object(type_class, type_name, p_object);
ERR_FAIL_NULL_V(mono_object, NULL); ERR_FAIL_NULL_V(mono_object, false);
CSharpScriptBinding script_binding; r_script_binding.inited = true;
r_script_binding.type_name = type_name;
script_binding.type_name = type_name; r_script_binding.wrapper_class = type_class; // cache
script_binding.wrapper_class = type_class; // cache r_script_binding.gchandle = MonoGCHandle::create_strong(mono_object);
script_binding.gchandle = MonoGCHandle::create_strong(mono_object);
void *data;
{
SCOPED_MUTEX_LOCK(language_bind_mutex);
data = (void *)script_bindings.insert(p_object, script_binding);
}
// Tie managed to unmanaged // Tie managed to unmanaged
Reference *ref = Object::cast_to<Reference>(p_object); Reference *ref = Object::cast_to<Reference>(p_object);
@ -1104,6 +1108,23 @@ void *CSharpLanguage::alloc_instance_binding_data(Object *p_object) {
ref->reference(); ref->reference();
} }
return true;
}
void *CSharpLanguage::alloc_instance_binding_data(Object *p_object) {
CSharpScriptBinding script_binding;
if (!setup_csharp_script_binding(script_binding, p_object))
return NULL;
void *data;
{
SCOPED_MUTEX_LOCK(language_bind_mutex);
data = (void *)script_bindings.insert(p_object, script_binding);
}
return data; return data;
} }
@ -1125,10 +1146,15 @@ void CSharpLanguage::free_instance_binding_data(void *p_data) {
Map<Object *, CSharpScriptBinding>::Element *data = (Map<Object *, CSharpScriptBinding>::Element *)p_data; Map<Object *, CSharpScriptBinding>::Element *data = (Map<Object *, CSharpScriptBinding>::Element *)p_data;
// Set the native instance field to IntPtr.Zero, if not yet garbage collected CSharpScriptBinding &script_binding = data->value();
MonoObject *mono_object = data->value().gchandle->get_target();
if (mono_object) { if (script_binding.inited) {
CACHED_FIELD(GodotObject, ptr)->set_value_raw(mono_object, NULL); // 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();
if (mono_object) {
CACHED_FIELD(GodotObject, ptr)->set_value_raw(mono_object, NULL);
}
} }
script_bindings.erase(data); script_bindings.erase(data);
@ -1144,9 +1170,10 @@ void CSharpLanguage::refcount_incremented_instance_binding(Object *p_object) {
#endif #endif
void *data = p_object->get_script_instance_binding(get_language_index()); void *data = p_object->get_script_instance_binding(get_language_index());
if (!data) CRASH_COND(!data);
return;
Ref<MonoGCHandle> &gchandle = ((Map<Object *, CSharpScriptBinding>::Element *)data)->get().gchandle; CSharpScriptBinding &script_binding = ((Map<Object *, CSharpScriptBinding>::Element *)data)->get();
Ref<MonoGCHandle> &gchandle = script_binding.gchandle;
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
// The reference count was increased after the managed side was the only one referencing our owner. // The reference count was increased after the managed side was the only one referencing our owner.
@ -1175,9 +1202,10 @@ bool CSharpLanguage::refcount_decremented_instance_binding(Object *p_object) {
int refcount = ref_owner->reference_get_count(); int refcount = ref_owner->reference_get_count();
void *data = p_object->get_script_instance_binding(get_language_index()); void *data = p_object->get_script_instance_binding(get_language_index());
if (!data) CRASH_COND(!data);
return refcount == 0;
Ref<MonoGCHandle> &gchandle = ((Map<Object *, CSharpScriptBinding>::Element *)data)->get().gchandle; CSharpScriptBinding &script_binding = ((Map<Object *, CSharpScriptBinding>::Element *)data)->get();
Ref<MonoGCHandle> &gchandle = script_binding.gchandle;
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_valid() && !gchandle->is_weak()) { // The managed side also holds a reference, hence 1 instead of 0
// If owner owner is no longer referenced by the unmanaged side, // If owner owner is no longer referenced by the unmanaged side,
@ -1223,6 +1251,10 @@ MonoObject *CSharpInstance::get_mono_object() const {
return gchandle->get_target(); return gchandle->get_target();
} }
Object *CSharpInstance::get_owner() {
return owner;
}
bool CSharpInstance::set(const StringName &p_name, const Variant &p_value) { bool CSharpInstance::set(const StringName &p_name, const Variant &p_value) {
ERR_FAIL_COND_V(!script.is_valid(), false); ERR_FAIL_COND_V(!script.is_valid(), false);
@ -1483,14 +1515,8 @@ bool CSharpInstance::_unreference_owner_unsafe() {
// Unsafe refcount decrement. The managed instance also counts as a reference. // Unsafe refcount decrement. The managed instance also counts as a reference.
// See: _reference_owner_unsafe() // See: _reference_owner_unsafe()
bool die = static_cast<Reference *>(owner)->unreference(); // Destroying the owner here means self destructing, so we defer the owner destruction to the caller.
return static_cast<Reference *>(owner)->unreference();
if (die) {
memdelete(owner);
owner = NULL;
}
return die;
} }
MonoObject *CSharpInstance::_internal_new_managed() { MonoObject *CSharpInstance::_internal_new_managed() {
@ -1503,27 +1529,33 @@ MonoObject *CSharpInstance::_internal_new_managed() {
ERR_FAIL_NULL_V(owner, NULL); ERR_FAIL_NULL_V(owner, NULL);
ERR_FAIL_COND_V(script.is_null(), NULL); ERR_FAIL_COND_V(script.is_null(), NULL);
if (base_ref)
_reference_owner_unsafe();
MonoObject *mono_object = mono_object_new(SCRIPTS_DOMAIN, script->script_class->get_mono_ptr()); MonoObject *mono_object = mono_object_new(SCRIPTS_DOMAIN, script->script_class->get_mono_ptr());
if (!mono_object) { if (!mono_object) {
// Important to clear this before destroying the script instance here
script = Ref<CSharpScript>(); script = Ref<CSharpScript>();
owner->set_script_instance(NULL); owner = NULL;
bool die = _unreference_owner_unsafe();
// Not ok for the owner to die here. If there is a situation where this can happen, it will be considered a bug.
CRASH_COND(die == true);
ERR_EXPLAIN("Failed to allocate memory for the object"); ERR_EXPLAIN("Failed to allocate memory for the object");
ERR_FAIL_V(NULL); ERR_FAIL_V(NULL);
} }
// Tie managed to unmanaged
gchandle = MonoGCHandle::create_strong(mono_object);
if (base_ref)
_reference_owner_unsafe(); // Here, after assigning the gchandle (for the refcount_incremented callback)
CACHED_FIELD(GodotObject, ptr)->set_value_raw(mono_object, owner); CACHED_FIELD(GodotObject, ptr)->set_value_raw(mono_object, owner);
// Construct // Construct
GDMonoMethod *ctor = script->script_class->get_method(CACHED_STRING_NAME(dotctor), 0); GDMonoMethod *ctor = script->script_class->get_method(CACHED_STRING_NAME(dotctor), 0);
ctor->invoke_raw(mono_object, NULL); ctor->invoke_raw(mono_object, NULL);
// Tie managed to unmanaged
gchandle = MonoGCHandle::create_strong(mono_object);
return mono_object; return mono_object;
} }
@ -1536,25 +1568,36 @@ void CSharpInstance::mono_object_disposed(MonoObject *p_obj) {
CSharpLanguage::get_singleton()->release_script_gchandle(p_obj, gchandle); CSharpLanguage::get_singleton()->release_script_gchandle(p_obj, gchandle);
} }
void CSharpInstance::mono_object_disposed_baseref(MonoObject *p_obj, bool p_is_finalizer, bool &r_owner_deleted) { void CSharpInstance::mono_object_disposed_baseref(MonoObject *p_obj, bool p_is_finalizer, bool &r_delete_owner, bool &r_remove_script_instance) {
#ifdef DEBUG_ENABLED #ifdef DEBUG_ENABLED
CRASH_COND(!base_ref); CRASH_COND(!base_ref);
CRASH_COND(gchandle.is_null()); CRASH_COND(gchandle.is_null());
#endif #endif
r_remove_script_instance = false;
if (_unreference_owner_unsafe()) { if (_unreference_owner_unsafe()) {
r_owner_deleted = true; // Safe to self destruct here with memdelete(owner), but it's deferred to the caller to prevent future mistakes.
r_delete_owner = true;
} else { } else {
r_owner_deleted = false; r_delete_owner = false;
CSharpLanguage::get_singleton()->release_script_gchandle(p_obj, gchandle); CSharpLanguage::get_singleton()->release_script_gchandle(p_obj, gchandle);
if (p_is_finalizer && !GDMono::get_singleton()->is_finalizing_scripts_domain()) {
// If the native instance is still alive, then it was if (!p_is_finalizer) {
// referenced from another thread before the finalizer could // If the native instance is still alive and Dispose() was called
// unreference it and delete it, so we want to keep it. // (instead of the finalizer), then we remove the script instance.
// GC.ReRegisterForFinalize(this) is not safe because the objects r_remove_script_instance = true;
// referenced by this could have already been collected. } else if (!GDMono::get_singleton()->is_finalizing_scripts_domain()) {
// Instead we will create a new managed instance here. // If the native instance is still alive and this is called from the finalizer,
_internal_new_managed(); // then it was referenced from another thread before the finalizer could
// unreference and delete it, so we want to keep it.
// GC.ReRegisterForFinalize(this) is not safe because the objects referenced by 'this'
// could have already been collected. Instead we will create a new managed instance here.
MonoObject *new_managed = _internal_new_managed();
if (!new_managed) {
r_remove_script_instance = true;
}
} }
} }
} }
@ -1750,6 +1793,8 @@ CSharpInstance::CSharpInstance() :
CSharpInstance::~CSharpInstance() { CSharpInstance::~CSharpInstance() {
destructing_script_instance = true;
if (gchandle.is_valid()) { if (gchandle.is_valid()) {
if (!predelete_notified && !ref_dying) { if (!predelete_notified && !ref_dying) {
// This destructor is not called from the owners destructor. // This destructor is not called from the owners destructor.
@ -1762,9 +1807,7 @@ CSharpInstance::~CSharpInstance() {
if (mono_object) { if (mono_object) {
MonoException *exc = NULL; MonoException *exc = NULL;
destructing_script_instance = true;
GDMonoUtils::dispose(mono_object, &exc); GDMonoUtils::dispose(mono_object, &exc);
destructing_script_instance = false;
if (exc) { if (exc) {
GDMonoUtils::set_pending_exception(exc); GDMonoUtils::set_pending_exception(exc);
@ -1772,11 +1815,23 @@ CSharpInstance::~CSharpInstance() {
} }
} }
gchandle->release(); // Make sure it's released gchandle->release(); // Make sure the gchandle is released
} }
if (base_ref && !ref_dying && owner) { // it may be called from the owner's destructor // If not being called from the owner's destructor, and we still hold a reference to the owner
_unreference_owner_unsafe(); if (base_ref && !ref_dying && owner && unsafe_referenced) {
// The owner's script or script instance is being replaced (or removed)
// Transfer ownership to an "instance binding"
void *data = owner->get_script_instance_binding(CSharpLanguage::get_singleton()->get_language_index());
CRASH_COND(data == NULL);
CSharpScriptBinding &script_binding = ((Map<Object *, CSharpScriptBinding>::Element *)data)->get();
CRASH_COND(!script_binding.inited);
bool die = _unreference_owner_unsafe();
CRASH_COND(die == true); // The "instance binding" should be holding a reference
} }
if (script.is_valid() && owner) { if (script.is_valid() && owner) {
@ -2327,6 +2382,32 @@ StringName CSharpScript::get_instance_base_type() const {
CSharpInstance *CSharpScript::_create_instance(const Variant **p_args, int p_argcount, Object *p_owner, bool p_isref, Variant::CallError &r_error) { CSharpInstance *CSharpScript::_create_instance(const Variant **p_args, int p_argcount, Object *p_owner, bool p_isref, Variant::CallError &r_error) {
/* STEP 1, CREATE */ /* STEP 1, CREATE */
Ref<Reference> ref;
if (p_isref) {
// Hold it alive. Important if we have to dispose a script instance binding before creating the CSharpInstance.
ref = Ref<Reference>(static_cast<Reference *>(p_owner));
}
// If the object had a script instance binding, dispose it before adding the CSharpInstance
if (p_owner->has_script_instance_binding(CSharpLanguage::get_singleton()->get_language_index())) {
void *data = p_owner->get_script_instance_binding(CSharpLanguage::get_singleton()->get_language_index());
CRASH_COND(data == NULL);
CSharpScriptBinding &script_binding = ((Map<Object *, CSharpScriptBinding>::Element *)data)->get();
if (script_binding.inited && script_binding.gchandle.is_valid()) {
MonoObject *mono_object = script_binding.gchandle->get_target();
if (mono_object) {
MonoException *exc = NULL;
GDMonoUtils::dispose(mono_object, &exc);
if (exc) {
GDMonoUtils::set_pending_exception(exc);
}
}
script_binding.inited = false;
}
}
CSharpInstance *instance = memnew(CSharpInstance); CSharpInstance *instance = memnew(CSharpInstance);
instance->base_ref = p_isref; instance->base_ref = p_isref;
@ -2334,16 +2415,20 @@ CSharpInstance *CSharpScript::_create_instance(const Variant **p_args, int p_arg
instance->owner = p_owner; instance->owner = p_owner;
instance->owner->set_script_instance(instance); instance->owner->set_script_instance(instance);
if (instance->base_ref)
instance->_reference_owner_unsafe();
/* STEP 2, INITIALIZE AND CONSTRUCT */ /* STEP 2, INITIALIZE AND CONSTRUCT */
MonoObject *mono_object = mono_object_new(SCRIPTS_DOMAIN, script_class->get_mono_ptr()); MonoObject *mono_object = mono_object_new(SCRIPTS_DOMAIN, script_class->get_mono_ptr());
if (!mono_object) { if (!mono_object) {
// Important to clear this before destroying the script instance here
instance->script = Ref<CSharpScript>(); instance->script = Ref<CSharpScript>();
instance->owner->set_script_instance(NULL); instance->owner = NULL;
bool die = instance->_unreference_owner_unsafe();
// Not ok for the owner to die here. If there is a situation where this can happen, it will be considered a bug.
CRASH_COND(die == true);
p_owner->set_script_instance(NULL);
r_error.error = Variant::CallError::CALL_ERROR_INSTANCE_IS_NULL; r_error.error = Variant::CallError::CALL_ERROR_INSTANCE_IS_NULL;
ERR_EXPLAIN("Failed to allocate memory for the object"); ERR_EXPLAIN("Failed to allocate memory for the object");
ERR_FAIL_V(NULL); ERR_FAIL_V(NULL);
@ -2352,6 +2437,9 @@ CSharpInstance *CSharpScript::_create_instance(const Variant **p_args, int p_arg
// Tie managed to unmanaged // Tie managed to unmanaged
instance->gchandle = MonoGCHandle::create_strong(mono_object); instance->gchandle = MonoGCHandle::create_strong(mono_object);
if (instance->base_ref)
instance->_reference_owner_unsafe(); // Here, after assigning the gchandle (for the refcount_incremented callback)
{ {
SCOPED_MUTEX_LOCK(CSharpLanguage::get_singleton()->script_instances_mutex); SCOPED_MUTEX_LOCK(CSharpLanguage::get_singleton()->script_instances_mutex);
instances.insert(instance->owner); instances.insert(instance->owner);

View File

@ -206,8 +206,15 @@ class CSharpInstance : public ScriptInstance {
Ref<MonoGCHandle> gchandle; Ref<MonoGCHandle> gchandle;
bool _reference_owner_unsafe(); bool _reference_owner_unsafe();
/*
* If true is returned, the caller must memdelete the script instance's owner.
*/
bool _unreference_owner_unsafe(); bool _unreference_owner_unsafe();
/*
* If NULL is returned, the caller must destroy the script instance by removing it from its owner.
*/
MonoObject *_internal_new_managed(); MonoObject *_internal_new_managed();
// Do not use unless you know what you are doing // Do not use unless you know what you are doing
@ -223,6 +230,8 @@ public:
_FORCE_INLINE_ bool is_destructing_script_instance() { return destructing_script_instance; } _FORCE_INLINE_ bool is_destructing_script_instance() { return destructing_script_instance; }
virtual Object *get_owner();
virtual bool set(const StringName &p_name, const Variant &p_value); virtual bool set(const StringName &p_name, const Variant &p_value);
virtual bool get(const StringName &p_name, Variant &r_ret) const; virtual bool get(const StringName &p_name, Variant &r_ret) const;
virtual void get_property_list(List<PropertyInfo> *p_properties) const; virtual void get_property_list(List<PropertyInfo> *p_properties) const;
@ -235,7 +244,12 @@ public:
virtual void call_multilevel_reversed(const StringName &p_method, const Variant **p_args, int p_argcount); virtual void call_multilevel_reversed(const StringName &p_method, const Variant **p_args, int p_argcount);
void mono_object_disposed(MonoObject *p_obj); void mono_object_disposed(MonoObject *p_obj);
void mono_object_disposed_baseref(MonoObject *p_obj, bool p_is_finalizer, bool &r_owner_deleted);
/*
* If 'r_delete_owner' is set to true, the caller must memdelete the script instance's owner. Otherwise, if
* 'r_remove_script_instance' is set to true, the caller must destroy the script instance by removing it from its owner.
*/
void mono_object_disposed_baseref(MonoObject *p_obj, bool p_is_finalizer, bool &r_delete_owner, bool &r_remove_script_instance);
virtual void refcount_incremented(); virtual void refcount_incremented();
virtual bool refcount_decremented(); virtual bool refcount_decremented();
@ -255,6 +269,7 @@ public:
}; };
struct CSharpScriptBinding { struct CSharpScriptBinding {
bool inited;
StringName type_name; StringName type_name;
GDMonoClass *wrapper_class; GDMonoClass *wrapper_class;
Ref<MonoGCHandle> gchandle; Ref<MonoGCHandle> gchandle;
@ -391,6 +406,8 @@ public:
virtual void refcount_incremented_instance_binding(Object *p_object); virtual void refcount_incremented_instance_binding(Object *p_object);
virtual bool refcount_decremented_instance_binding(Object *p_object); virtual bool refcount_decremented_instance_binding(Object *p_object);
bool setup_csharp_script_binding(CSharpScriptBinding &r_script_binding, Object *p_object);
#ifdef DEBUG_ENABLED #ifdef DEBUG_ENABLED
Vector<StackInfo> stack_trace_get_info(MonoObject *p_stack_trace); Vector<StackInfo> stack_trace_get_info(MonoObject *p_stack_trace);
#endif #endif

View File

@ -65,9 +65,12 @@ void godot_icall_Object_Disposed(MonoObject *p_obj, Object *p_ptr) {
void *data = p_ptr->get_script_instance_binding(CSharpLanguage::get_singleton()->get_language_index()); void *data = p_ptr->get_script_instance_binding(CSharpLanguage::get_singleton()->get_language_index());
if (data) { if (data) {
Ref<MonoGCHandle> &gchandle = ((Map<Object *, CSharpScriptBinding>::Element *)data)->get().gchandle; CSharpScriptBinding &script_binding = ((Map<Object *, CSharpScriptBinding>::Element *)data)->get();
if (gchandle.is_valid()) { if (script_binding.inited) {
CSharpLanguage::release_script_gchandle(p_obj, gchandle); Ref<MonoGCHandle> &gchandle = script_binding.gchandle;
if (gchandle.is_valid()) {
CSharpLanguage::release_script_gchandle(p_obj, gchandle);
}
} }
} }
} }
@ -85,11 +88,14 @@ void godot_icall_Reference_Disposed(MonoObject *p_obj, Object *p_ptr, bool p_is_
CSharpInstance *cs_instance = CAST_CSHARP_INSTANCE(ref->get_script_instance()); CSharpInstance *cs_instance = CAST_CSHARP_INSTANCE(ref->get_script_instance());
if (cs_instance) { if (cs_instance) {
if (!cs_instance->is_destructing_script_instance()) { if (!cs_instance->is_destructing_script_instance()) {
bool r_owner_deleted; bool delete_owner;
cs_instance->mono_object_disposed_baseref(p_obj, p_is_finalizer, r_owner_deleted); bool remove_script_instance;
if (!r_owner_deleted && !p_is_finalizer) {
// If the native instance is still alive and Dispose() was called cs_instance->mono_object_disposed_baseref(p_obj, p_is_finalizer, delete_owner, remove_script_instance);
// (instead of the finalizer), then we remove the script instance.
if (delete_owner) {
memdelete(ref);
} else if (remove_script_instance) {
ref->set_script_instance(NULL); ref->set_script_instance(NULL);
} }
} }
@ -105,9 +111,12 @@ void godot_icall_Reference_Disposed(MonoObject *p_obj, Object *p_ptr, bool p_is_
void *data = ref->get_script_instance_binding(CSharpLanguage::get_singleton()->get_language_index()); void *data = ref->get_script_instance_binding(CSharpLanguage::get_singleton()->get_language_index());
if (data) { if (data) {
Ref<MonoGCHandle> &gchandle = ((Map<Object *, CSharpScriptBinding>::Element *)data)->get().gchandle; CSharpScriptBinding &script_binding = ((Map<Object *, CSharpScriptBinding>::Element *)data)->get();
if (gchandle.is_valid()) { if (script_binding.inited) {
CSharpLanguage::release_script_gchandle(p_obj, gchandle); Ref<MonoGCHandle> &gchandle = script_binding.gchandle;
if (gchandle.is_valid()) {
CSharpLanguage::release_script_gchandle(p_obj, gchandle);
}
} }
} }
} }
@ -138,7 +147,7 @@ MonoObject *godot_icall_Object_weakref(Object *p_obj) {
wref->set_obj(p_obj); wref->set_obj(p_obj);
} }
return GDMonoUtils::create_managed_for_godot_object(CACHED_CLASS(WeakRef), Reference::get_class_static(), Object::cast_to<Object>(wref.ptr())); return GDMonoUtils::unmanaged_get_managed(wref.ptr());
} }
Error godot_icall_SignalAwaiter_connect(Object *p_source, MonoString *p_signal, Object *p_target, MonoObject *p_awaiter) { Error godot_icall_SignalAwaiter_connect(Object *p_source, MonoString *p_signal, Object *p_target, MonoObject *p_awaiter) {

View File

@ -65,7 +65,7 @@ Ref<MonoGCHandle> MonoGCHandle::create_weak(MonoObject *p_object) {
void MonoGCHandle::release() { void MonoGCHandle::release() {
#ifdef DEBUG_ENABLED #ifdef DEBUG_ENABLED
CRASH_COND(GDMono::get_singleton() == NULL); CRASH_COND(!released && GDMono::get_singleton() == NULL);
#endif #endif
if (!released && GDMono::get_singleton()->is_runtime_initialized()) { if (!released && GDMono::get_singleton()->is_runtime_initialized()) {

View File

@ -265,61 +265,70 @@ void clear_cache() {
} }
MonoObject *unmanaged_get_managed(Object *unmanaged) { 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) { if (!unmanaged)
return cs_instance->get_mono_object(); return NULL;
}
}
// If the owner does not have a CSharpInstance... if (unmanaged->get_script_instance()) {
CSharpInstance *cs_instance = CAST_CSHARP_INSTANCE(unmanaged->get_script_instance());
void *data = unmanaged->get_script_instance_binding(CSharpLanguage::get_singleton()->get_language_index()); if (cs_instance) {
return cs_instance->get_mono_object();
if (data) {
CSharpScriptBinding &script_binding = ((Map<Object *, CSharpScriptBinding>::Element *)data)->value();
Ref<MonoGCHandle> &gchandle = script_binding.gchandle;
ERR_FAIL_COND_V(gchandle.is_null(), NULL);
MonoObject *target = gchandle->get_target();
if (target)
return target;
CSharpLanguage::get_singleton()->release_script_gchandle(gchandle);
// Create a new one
#ifdef DEBUG_ENABLED
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);
// Tie managed to unmanaged
Reference *ref = Object::cast_to<Reference>(unmanaged);
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)
ref->reference();
}
return mono_object;
} }
} }
return NULL; // 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;
CSharpScriptBinding &script_binding = ((Map<Object *, CSharpScriptBinding>::Element *)data)->value();
if (!script_binding.inited) {
// Already had a binding that needs to be setup
CSharpLanguage::get_singleton()->setup_csharp_script_binding(script_binding, unmanaged);
if (!script_binding.inited)
return NULL;
}
Ref<MonoGCHandle> &gchandle = script_binding.gchandle;
ERR_FAIL_COND_V(gchandle.is_null(), NULL);
MonoObject *target = gchandle->get_target();
if (target)
return target;
CSharpLanguage::get_singleton()->release_script_gchandle(gchandle);
// Create a new one
#ifdef DEBUG_ENABLED
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);
// Tie managed to unmanaged
Reference *ref = Object::cast_to<Reference>(unmanaged);
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)
ref->reference();
}
return mono_object;
} }
void set_main_thread(MonoThread *p_thread) { void set_main_thread(MonoThread *p_thread) {