Mono/C#: Add error checks to detect possible Reference leaks

This commit is contained in:
Ignacio Etcheverry 2020-01-13 21:00:07 +01:00
parent ea737db43c
commit a6a5ef0fd6
5 changed files with 51 additions and 4 deletions

View file

@ -159,6 +159,19 @@ void CSharpLanguage::finish() {
// Clear here, after finalizing all domains to make sure there is nothing else referencing the elements. // Clear here, after finalizing all domains to make sure there is nothing else referencing the elements.
script_bindings.clear(); 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; finalizing = false;
} }
@ -615,6 +628,23 @@ Vector<ScriptLanguage::StackInfo> CSharpLanguage::stack_trace_get_info(MonoObjec
} }
#endif #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() { void CSharpLanguage::frame() {
if (gdmono && gdmono->is_runtime_initialized() && gdmono->get_core_api_assembly() != NULL) { 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) // See: godot_icall_Reference_Dtor(MonoObject *p_obj, Object *p_ptr)
ref->reference(); ref->reference();
CSharpLanguage::get_singleton()->post_unsafe_reference(ref);
} }
return true; return true;
@ -1736,9 +1767,12 @@ bool CSharpInstance::_reference_owner_unsafe() {
// See: _unreference_owner_unsafe() // See: _unreference_owner_unsafe()
// May not me referenced yet, so we must use init_ref() instead of reference() // May not me referenced yet, so we must use init_ref() instead of reference()
bool success = Object::cast_to<Reference>(owner)->init_ref(); if (static_cast<Reference *>(owner)->init_ref()) {
unsafe_referenced = success; CSharpLanguage::get_singleton()->post_unsafe_reference(owner);
return success; unsafe_referenced = true;
}
return unsafe_referenced;
} }
bool CSharpInstance::_unreference_owner_unsafe() { bool CSharpInstance::_unreference_owner_unsafe() {
@ -1759,6 +1793,7 @@ bool CSharpInstance::_unreference_owner_unsafe() {
// See: _reference_owner_unsafe() // See: _reference_owner_unsafe()
// Destroying the owner here means self destructing, so we defer the owner destruction to the caller. // 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(); return static_cast<Reference *>(owner)->unreference();
} }

View file

@ -307,6 +307,11 @@ class CSharpLanguage : public ScriptLanguage {
Map<Object *, CSharpScriptBinding> script_bindings; Map<Object *, CSharpScriptBinding> script_bindings;
#ifdef DEBUG_ENABLED
// List of unsafely referenced objects
List<ObjectID> unsafely_referenced_objects;
#endif
struct StringNameCache { struct StringNameCache {
StringName _signal_callback; StringName _signal_callback;
@ -458,6 +463,9 @@ public:
Vector<StackInfo> stack_trace_get_info(MonoObject *p_stack_trace); Vector<StackInfo> stack_trace_get_info(MonoObject *p_stack_trace);
#endif #endif
void post_unsafe_reference(Object *p_obj);
void pre_unsafe_unreference(Object *p_obj);
CSharpLanguage(); CSharpLanguage();
~CSharpLanguage(); ~CSharpLanguage();
}; };

View file

@ -108,6 +108,7 @@ void godot_icall_Reference_Disposed(MonoObject *p_obj, Object *p_ptr, MonoBoolea
// Unsafe refcount decrement. The managed instance also counts as a reference. // Unsafe refcount decrement. The managed instance also counts as a reference.
// See: CSharpLanguage::alloc_instance_binding_data(Object *p_object) // See: CSharpLanguage::alloc_instance_binding_data(Object *p_object)
CSharpLanguage::get_singleton()->pre_unsafe_unreference(ref);
if (ref->unreference()) { if (ref->unreference()) {
memdelete(ref); memdelete(ref);
} else { } else {

View file

@ -83,7 +83,9 @@ void tie_managed_to_unmanaged(MonoObject *managed, Object *unmanaged) {
// See: godot_icall_Reference_Dtor(MonoObject *p_obj, Object *p_ptr) // See: godot_icall_Reference_Dtor(MonoObject *p_obj, Object *p_ptr)
// May not me referenced yet, so we must use init_ref() instead of reference() // May not me referenced yet, so we must use init_ref() instead of reference()
ref->init_ref(); if (ref->init_ref()) {
CSharpLanguage::get_singleton()->post_unsafe_reference(ref);
}
} }
// The object was just created, no script instance binding should have been attached // The object was just created, no script instance binding should have been attached

View file

@ -115,6 +115,7 @@ MonoObject *unmanaged_get_managed(Object *unmanaged) {
// but the managed instance is alive, the refcount will be 1 instead of 0. // 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) // See: godot_icall_Reference_Dtor(MonoObject *p_obj, Object *p_ptr)
ref->reference(); ref->reference();
CSharpLanguage::get_singleton()->post_unsafe_reference(ref);
} }
return mono_object; return mono_object;