GDNative terminate too early if the same shared library been loaded multiple times
Opened this issue · 3 comments
Godot version:
3.0 ~ master
OS/device including version:
OS and device independent
Issue description:
When a shared library is loaded multiple times, and load_once = true
, godot will crash at first terminate call.
Steps to reproduce:
- create a gdnative library
- create a
.gdnlib
- create a
.gdns
, and attach to a node - create a
.gd
and attach to a node. in_init
, load, initialize and terminate gdnative library - run the project
godot will be crashed by the terminate call in .gd
.
Minimal reproduction project:
simple.zip
The fix
this issue is caused by these lines
// modules/gdnative/gdnative.cpp
bool GDNative::initialize() {
...
if (library->should_load_once()) {
if (GDNativeLibrary::loaded_libraries->has(lib_path)) {
// already loaded. Don't load again.
// copy some of the stuff instead
this->native_handle = (*GDNativeLibrary::loaded_libraries)[lib_path][0]->native_handle;
initialized = true;
return true;
}
}
...
}
Add one line to fix this problem
this->native_handle = (*GDNativeLibrary::loaded_libraries)[lib_path][0]->native_handle;
initialized = true;
GDNativeLibrary::loaded_libraries[lib_path].push_back(Ref<GDNative>(this));
return true;
@Calinou will the fix be merged into master? Without the one line fix, the code block bellow will run terminate no matter the same shared library been loaded how many times (gdnatives->size() > 1
never true)
// modules/gdnative/gdnative.cpp
bool GDNative::terminate() {
...
if (library->should_load_once()) {
Vector<Ref<GDNative> > *gdnatives = &GDNativeLibrary::loaded_libraries[library->get_current_library_path()];
if (gdnatives->size() > 1) {
// there are other GDNative's still using this library, so we actually don't terminate
gdnatives->erase(Ref<GDNative>(this));
initialized = false;
return true;
} else if (gdnatives->size() == 1) {
// we're the last one, terminate!
gdnatives->clear();
// whew this looks scary, but all it does is remove the entry completely
GDNativeLibrary::loaded_libraries.erase(GDNativeLibrary::loaded_libraries.find(library->get_current_library_path()));
}
}
...
The recommended workflow would be for you to make a Pull Request with the suggested fix. Then we can review it with GDNative contributors to confirm that it's good, or ask for changes.
I see