Skip to content

Commit

Permalink
C#: Clear instance bindings callbacks on finalizing the language
Browse files Browse the repository at this point in the history
When finalizing the C# language every C# instance is disposed and the instance bindings callbacks are no longer valid. Clearing the instance bindings ensures these callbacks are not called, and since we dispose of every C# instance there should be no leaks.
  • Loading branch information
raulsntos committed Jun 14, 2024
1 parent 5833f59 commit 13d26fc
Showing 1 changed file with 11 additions and 2 deletions.
13 changes: 11 additions & 2 deletions modules/mono/csharp_script.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,14 +148,18 @@ void CSharpLanguage::finalize() {

finalizing = true;

// Make sure all script binding gchandles are released before finalizing GDMono
// Make sure all script binding gchandles are released before finalizing GDMono.
for (KeyValue<Object *, CSharpScriptBinding> &E : script_bindings) {
CSharpScriptBinding &script_binding = E.value;

if (!script_binding.gchandle.is_released()) {
script_binding.gchandle.release();
script_binding.inited = false;
}

// Make sure we clear all the instance binding callbacks so they don't get called
// after finalizing the C# language.
script_binding.owner->free_instance_binding(this);
}

if (gdmono) {
Expand Down Expand Up @@ -1227,6 +1231,11 @@ void CSharpLanguage::_instance_binding_free_callback(void *, void *, void *p_bin
}

GDExtensionBool CSharpLanguage::_instance_binding_reference_callback(void *p_token, void *p_binding, GDExtensionBool p_reference) {
// Instance bindings callbacks can only be called if the C# language is available.
// Failing this assert usually means that we didn't clear the instance binding in some Object
// and the C# language has already been finalized.
DEV_ASSERT(CSharpLanguage::get_singleton() != nullptr);

CRASH_COND(!p_binding);

CSharpScriptBinding &script_binding = ((RBMap<Object *, CSharpScriptBinding>::Element *)p_binding)->get();
Expand Down Expand Up @@ -1662,7 +1671,7 @@ bool CSharpInstance::_reference_owner_unsafe() {
// but the managed instance is alive, the refcount will be 1 instead of 0.
// See: _unreference_owner_unsafe()

// May not me referenced yet, so we must use init_ref() instead of reference()
// May not be referenced yet, so we must use init_ref() instead of reference()
if (static_cast<RefCounted *>(owner)->init_ref()) {
CSharpLanguage::get_singleton()->post_unsafe_reference(owner);
unsafe_referenced = true;
Expand Down

0 comments on commit 13d26fc

Please sign in to comment.