Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix memory leak when ClassDB::bind_method_custom() fails #102131

Merged
merged 1 commit into from
Jan 30, 2025

Conversation

dsnopek
Copy link
Contributor

@dsnopek dsnopek commented Jan 28, 2025

Fixes #101870

In other cases (for example, ClassDB::_bind_vararg_method() and ClassDB::bind_methodfi()), the method bind is deleted just before returning if it can't be added for some reason.

A couple examples:

godot/core/object/class_db.cpp

Lines 1899 to 1902 in 71d80b2

if (!type) {
memdelete(bind);
ERR_FAIL_NULL_V(type, nullptr);
}

godot/core/object/class_db.cpp

Lines 1949 to 1953 in 71d80b2

if (!p_compatibility && type->method_map.has(mdname)) {
memdelete(p_bind);
// overloading not supported
ERR_FAIL_V_MSG(nullptr, vformat("Method already bound '%s::%s'.", instance_type, mdname));
}

So, this PR is aiming to do the same thing but in ClassDB::_bind_method_custom().

(Because I needed to stash p_method->get_name() in a variable so I could use it after the method bind is deleted, I also did a tiny refactor to stash that in a local variable for all cases, since it was referred to quite a lot.)

@RandomShaper
Copy link
Member

Name stashing looks like needed here, too.

@dsnopek dsnopek force-pushed the classdb-bind-method-custom-leak branch from ea9363a to e904c0c Compare January 29, 2025 13:48
@dsnopek
Copy link
Contributor Author

dsnopek commented Jan 29, 2025

Sorry, it looks like I didn't push at the end? Should be right now.

@Repiteo Repiteo merged commit 699237d into godotengine:master Jan 30, 2025
19 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Jan 30, 2025

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible memory leak in gdextension.cpp when method names are duplicated
3 participants