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

NativeScriptLanguage::unregister_binding_functions fails. #28923

Closed
fsecilia opened this issue May 15, 2019 · 0 comments · Fixed by #28924
Closed

NativeScriptLanguage::unregister_binding_functions fails. #28923

fsecilia opened this issue May 15, 2019 · 0 comments · Fixed by #28924

Comments

@fsecilia
Copy link
Contributor

Godot version:
3.1-stable, but probably since 0b2afa2.

OS/device including version:
Arch Linux

Issue description:
NativeScriptLanguage::unregister_binding_functions() fails when binding_data.size() gets out of sync with binding_functions.size().

This happens when a gdnative library creates instance bindings for an instance of a global type, then more gdnative libraries are loaded but do not refer to that instance. Each loaded library increases the size of binding_functions, but an instance's binding_data is only updated when looking up that instance's instance bindings.

Steps to reproduce:
Consider this case. A gdnative library is loaded. It registers instance binding functions. binding_functions.size() is now 1. That library then looks up instance bindings for ResourceLoader. This creates instance bindings on ResourceLoader for that library's language. ResourceLoader's binding_data.size() is now 1.

Later, another library is loaded. This library also registers instance binding functions, so binding_functions.size() is now 2. Nothing refers to ResourceLoader's instance bindings, so its binding_data.size() is still 1.

When NativeScriptLanguage::unregister_binding_functions is called, binding_functions.size() is still 2, but ResourceLoader's binding_data.size() is still 1, causing binding_data[p_idx] to fail.

Minimal reproduction project:
This isn't trivial to reproduce, but I have a patch ready.

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

Successfully merging a pull request may close this issue.

3 participants