From 9786b5160140f7b2462db6a18bf4a7a524fbb6d9 Mon Sep 17 00:00:00 2001 From: karroffel Date: Sat, 9 Mar 2019 18:01:08 +0100 Subject: [PATCH] [GDNative] fix crash at shutdown when using singleton libraries and NativeScript When a singleton library was exposing NativeScript functionality, the NativeScriptLanguage would attempt to terminate the library at shutdown. Since the GDNative module itself handles singleton libraries, it closes all singleton libraries at shutdown as well. This double free could cause a crash, since the library referenced would no longer be alive. --- modules/gdnative/gdnative.cpp | 1 + .../gdnative/nativescript/nativescript.cpp | 27 +++++++++++++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/modules/gdnative/gdnative.cpp b/modules/gdnative/gdnative.cpp index 34325db9fdc..e8278825bc8 100644 --- a/modules/gdnative/gdnative.cpp +++ b/modules/gdnative/gdnative.cpp @@ -414,6 +414,7 @@ bool GDNative::terminate() { if (error || !library_terminate) { OS::get_singleton()->close_dynamic_library(native_handle); native_handle = NULL; + initialized = false; return true; } diff --git a/modules/gdnative/nativescript/nativescript.cpp b/modules/gdnative/nativescript/nativescript.cpp index c2b772df858..2da9d6bfdc2 100644 --- a/modules/gdnative/nativescript/nativescript.cpp +++ b/modules/gdnative/nativescript/nativescript.cpp @@ -1036,8 +1036,16 @@ NativeScriptLanguage::~NativeScriptLanguage() { for (Map >::Element *L = NSL->library_gdnatives.front(); L; L = L->next()) { - if (L->get().is_valid()) - L->get()->terminate(); + Ref lib = L->get(); + // only shut down valid libs, duh! + if (lib.is_valid()) { + + // If it's a singleton-library then the gdnative module + // manages the destruction at engine shutdown, not NativeScript. + if (!lib->get_library()->is_singleton()) { + lib->terminate(); + } + } } NSL->library_classes.clear(); @@ -1641,10 +1649,19 @@ void NativeReloadNode::_notification(int p_what) { continue; } + // Don't unload what should not be reloaded! if (!gdn->get_library()->is_reloadable()) { continue; } + // singleton libraries might have alive pointers living inside the + // editor. Also reloading a singleton library would mean that + // the singleton entry will not be called again, as this only + // happens at engine startup. + if (gdn->get_library()->is_singleton()) { + continue; + } + gdn->terminate(); } @@ -1672,6 +1689,12 @@ void NativeReloadNode::_notification(int p_what) { continue; } + // since singleton libraries are not unloaded there is no point + // in loading them again. + if (!gdn->get_library()->is_singleton()) { + continue; + } + if (!gdn->initialize()) { libs_to_remove.insert(L->key()); continue;