diff --git a/include/class_loader/class_loader.hpp b/include/class_loader/class_loader.hpp index 5fbb246..27da8cf 100644 --- a/include/class_loader/class_loader.hpp +++ b/include/class_loader/class_loader.hpp @@ -132,7 +132,7 @@ class ClassLoader { return std::shared_ptr( createRawInstance(derived_class_name, true, std::forward(args)...), - [this](Base * p) {onPluginDeletion(p);} + makeDeleter() ); } @@ -159,7 +159,7 @@ class ClassLoader Base * raw = createRawInstance(derived_class_name, true, std::forward(args)...); return std::unique_ptr>( raw, - [this](Base * p) {onPluginDeletion(p);} + makeDeleter() ); } @@ -266,6 +266,20 @@ class ClassLoader int unloadLibrary(); private: + /// Deleter for managed instances that stays valid if the instance outlives this ClassLoader. + template + DeleterType makeDeleter() + { + return [context = deleter_context_](Base * p) { + std::lock_guard lock(context->mutex); + if (context->loader) { + context->loader->onPluginDeletion(p); + } else { + delete p; // The library is still loaded: it is not unloaded while instances exist. + } + }; + } + /** * @brief Callback method when a plugin created by this class loader is destroyed * @@ -381,6 +395,13 @@ class ClassLoader int plugin_ref_count_; std::recursive_mutex plugin_ref_count_mutex_; static std::atomic has_unmanaged_instance_been_created_; + + struct DeleterContext + { + std::recursive_mutex mutex; + ClassLoader * loader = nullptr; // Cleared when the ClassLoader is destroyed. + }; + std::shared_ptr deleter_context_; }; } // namespace class_loader diff --git a/src/class_loader.cpp b/src/class_loader.cpp index f90acd0..66d98a8 100644 --- a/src/class_loader.cpp +++ b/src/class_loader.cpp @@ -55,8 +55,10 @@ ClassLoader::ClassLoader(const std::string & library_path, bool ondemand_load_un : ondemand_load_unload_(ondemand_load_unload), library_path_(library_path), load_ref_count_(0), - plugin_ref_count_(0) + plugin_ref_count_(0), + deleter_context_(std::make_shared()) { + deleter_context_->loader = this; CONSOLE_BRIDGE_logDebug( "class_loader.ClassLoader: " "Constructing new ClassLoader (%p) bound to library %s.", @@ -72,9 +74,10 @@ ClassLoader::~ClassLoader() "%s", "class_loader.ClassLoader: Destroying class loader, " "unloading associated library...\n"); + std::lock_guard lock(deleter_context_->mutex); { // The deferred on-demand unload in onPluginDeletion() can't happen once this loader is gone. - std::lock_guard lock(plugin_ref_count_mutex_); + std::lock_guard ref_count_lock(plugin_ref_count_mutex_); if (plugin_ref_count_ > 0 && isOnDemandLoadUnloadEnabled()) { CONSOLE_BRIDGE_logWarn( "%s", @@ -83,6 +86,7 @@ ClassLoader::~ClassLoader() } } unloadLibrary(); // TODO(mikaelarguedas): while(unloadLibrary() > 0){} ?? + deleter_context_->loader = nullptr; } const std::string & ClassLoader::getLibraryPath() const diff --git a/test/utest.cpp b/test/utest.cpp index 85aabf1..8151a08 100644 --- a/test/utest.cpp +++ b/test/utest.cpp @@ -521,6 +521,19 @@ TEST(ClassLoaderGraveyardTest, loadUnloadLoadFromGraveyard) { #endif // ifndef _WIN32 +// Leaves LIBRARY_2 loaded, so keep this suite last. +TEST(ClassLoaderLifetimeTest, instancesOutliveLoader) { + auto loader = std::make_unique(LIBRARY_2, false); + std::shared_ptr shared = loader->createInstance("Robot"); + class_loader::ClassLoader::UniquePtr unique = loader->createUniqueInstance("Robot"); + loader.reset(); + EXPECT_EQ("Beep boop", shared->saySomething()); + EXPECT_EQ("Beep boop", unique->saySomething()); + // Must not use the destroyed loader (heap-use-after-free under AddressSanitizer). + shared.reset(); + unique.reset(); +} + // Run all the tests that were declared with TEST() int main(int argc, char ** argv) {