From 60da3c95e14a9370950308deb3b87980e696cdc2 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Mon, 2 Nov 2026 15:02:01 -0600 Subject: [PATCH] Have MultiLibraryClassLoader instances keep their ClassLoader alive MultiLibraryClassLoader kept every ClassLoader it created in a process-wide static vector so loaders survived while their instances were alive. Loaders whose instances outlived the MultiLibraryClassLoader stayed there until process exit, and without on-demand unloading their libraries were never unloaded. Own the loaders per MultiLibraryClassLoader instead, and have each instance hold a shared_ptr to its ClassLoader. A loader now lives exactly as long as its MultiLibraryClassLoader or its last instance, and the static vector and its destruction-order workaround go away. Signed-off-by: Michael Carroll --- .../multi_library_class_loader.hpp | 41 +++++++++-- src/multi_library_class_loader.cpp | 69 +++++-------------- test/utest.cpp | 18 +++++ 3 files changed, 72 insertions(+), 56 deletions(-) diff --git a/include/class_loader/multi_library_class_loader.hpp b/include/class_loader/multi_library_class_loader.hpp index a8ea70e..ec4b51a 100644 --- a/include/class_loader/multi_library_class_loader.hpp +++ b/include/class_loader/multi_library_class_loader.hpp @@ -111,7 +111,8 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader "was explicitly loaded through MultiLibraryClassLoader::loadLibrary()"); } - return loader->createInstance(class_name, std::forward(args)...); + return keepLoaderAlive( + loader, loader->createInstance(class_name, std::forward(args)...)); } /** @@ -137,7 +138,8 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader "MultiLibraryClassLoader bound to library " + library_path + " Ensure you called MultiLibraryClassLoader::loadLibrary()"); } - return loader->createInstance(class_name, std::forward(args)...); + return keepLoaderAlive( + loader, loader->createInstance(class_name, std::forward(args)...)); } /// Creates an instance of an object of given class name with ancestor class Base @@ -167,7 +169,8 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader "Make sure that the library exists and was explicitly loaded through " "MultiLibraryClassLoader::loadLibrary()"); } - return loader->createUniqueInstance(class_name, std::forward(args)...); + return keepLoaderAlive( + loader, loader->createUniqueInstance(class_name, std::forward(args)...)); } /// Creates an instance of an object of given class name with ancestor class Base @@ -195,7 +198,8 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader "MultiLibraryClassLoader bound to library " + library_path + " Ensure you called MultiLibraryClassLoader::loadLibrary()"); } - return loader->createUniqueInstance(class_name, std::forward(args)...); + return keepLoaderAlive( + loader, loader->createUniqueInstance(class_name, std::forward(args)...)); } /** @@ -333,6 +337,35 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader int unloadLibrary(const std::string & library_path); private: + std::shared_ptr getSharedClassLoader(const ClassLoader * loader) const; + + /// Makes the instance keep its ClassLoader alive, so it may outlive this MultiLibraryClassLoader. + template + std::shared_ptr keepLoaderAlive(const ClassLoader * loader, std::shared_ptr obj) const + { + struct Holder // Members are destroyed in reverse order: the instance before its loader. + { + std::shared_ptr loader; + std::shared_ptr obj; + }; + auto holder = std::make_shared(Holder{getSharedClassLoader(loader), std::move(obj)}); + return std::shared_ptr(holder, holder->obj.get()); + } + + /// Makes the instance keep its ClassLoader alive, so it may outlive this MultiLibraryClassLoader. + template + ClassLoader::UniquePtr keepLoaderAlive( + const ClassLoader * loader, ClassLoader::UniquePtr obj) const + { + auto deleter = obj.get_deleter(); + return ClassLoader::UniquePtr( + obj.release(), + [deleter = std::move(deleter), keep_alive = getSharedClassLoader(loader)](Base * p) mutable { + deleter(p); + keep_alive.reset(); // unique_ptr::reset() keeps the deleter alive, so release here. + }); + } + /** * @brief Indicates if on-demand (lazy) load/unload is enabled so libraries are loaded/unloaded * diff --git a/src/multi_library_class_loader.cpp b/src/multi_library_class_loader.cpp index 36f4aa0..5316596 100644 --- a/src/multi_library_class_loader.cpp +++ b/src/multi_library_class_loader.cpp @@ -31,55 +31,21 @@ #include #include +#include #include -#include #include #include namespace class_loader { -using ClassLoaderPtrVector = std::vector>; - -class ClassLoaderDependency -{ -protected: - ClassLoaderDependency() - { - // make the static variable in `ClassLoader` destroyed after `class_loader_ptrs_`, - // which is a member of ClassLoaderPtrVectorImpl defined as a static instance in the - // `getClassLoaderPtrVectorImpl` - class_loader::impl::getLoadedLibraryVectorMutex(); - class_loader::impl::getPluginBaseToFactoryMapMapMutex(); - class_loader::impl::getGlobalPluginBaseToFactoryMapMap(); - class_loader::impl::getMetaObjectGraveyard(); - class_loader::impl::getLoadedLibraryVector(); - class_loader::impl::getCurrentlyLoadingLibraryName(); - class_loader::impl::getCurrentlyActiveClassLoader(); - class_loader::impl::hasANonPurePluginLibraryBeenOpened(); - } -}; - -class ClassLoaderPtrVectorImpl : public ClassLoaderDependency -{ -public: - ClassLoaderPtrVector class_loader_ptrs_; - std::mutex loader_mutex_; -}; - class MultiLibraryClassLoaderImpl { public: bool enable_ondemand_loadunload_; - LibraryToClassLoaderMap active_class_loaders_; + std::map> active_class_loaders_; }; -ClassLoaderPtrVectorImpl & getClassLoaderPtrVectorImpl() -{ - static ClassLoaderPtrVectorImpl instance; - return instance; -} - MultiLibraryClassLoader::MultiLibraryClassLoader(bool enable_ondemand_loadunload) : impl_(std::make_unique()) { @@ -104,7 +70,7 @@ std::vector MultiLibraryClassLoader::getRegisteredLibraries() const ClassLoader * MultiLibraryClassLoader::getClassLoaderForLibrary(const std::string & library_path) { - return impl_->active_class_loaders_[library_path]; + return impl_->active_class_loaders_[library_path].get(); } const ClassLoader * @@ -114,14 +80,14 @@ MultiLibraryClassLoader::getClassLoaderForLibrary(const std::string & library_pa if (it == impl_->active_class_loaders_.end()) { return nullptr; } - return it->second; + return it->second.get(); } ClassLoaderVector MultiLibraryClassLoader::getAllAvailableClassLoaders() const { ClassLoaderVector loaders; for (auto & it : impl_->active_class_loaders_) { - loaders.push_back(it.second); + loaders.push_back(it.second.get()); } return loaders; } @@ -135,13 +101,20 @@ bool MultiLibraryClassLoader::isLibraryAvailable(const std::string & library_nam void MultiLibraryClassLoader::loadLibrary(const std::string & library_path) { if (!isLibraryAvailable(library_path)) { - std::lock_guard lock(getClassLoaderPtrVectorImpl().loader_mutex_); - getClassLoaderPtrVectorImpl().class_loader_ptrs_.emplace_back( - std::make_shared(library_path, isOnDemandLoadUnloadEnabled()) - ); impl_->active_class_loaders_[library_path] = - getClassLoaderPtrVectorImpl().class_loader_ptrs_.back().get(); + std::make_shared(library_path, isOnDemandLoadUnloadEnabled()); + } +} + +std::shared_ptr +MultiLibraryClassLoader::getSharedClassLoader(const ClassLoader * loader) const +{ + for (const auto & it : impl_->active_class_loaders_) { + if (it.second.get() == loader) { + return it.second; + } } + return nullptr; } void MultiLibraryClassLoader::shutdownAllClassLoaders() @@ -159,14 +132,6 @@ int MultiLibraryClassLoader::unloadLibrary(const std::string & library_path) remaining_unloads = loader->unloadLibrary(); if (remaining_unloads == 0) { impl_->active_class_loaders_[library_path] = nullptr; - std::lock_guard lock(getClassLoaderPtrVectorImpl().loader_mutex_); - auto & class_loader_ptrs = getClassLoaderPtrVectorImpl().class_loader_ptrs_; - if (auto iter = std::ranges::find( - class_loader_ptrs, loader, [](const auto & p) {return p.get();}); - iter != class_loader_ptrs.end()) - { - class_loader_ptrs.erase(iter); - } } } return remaining_unloads; diff --git a/test/utest.cpp b/test/utest.cpp index e9c3da1..60da908 100644 --- a/test/utest.cpp +++ b/test/utest.cpp @@ -300,6 +300,24 @@ TYPED_TEST(MultiClassLoaderTest, nonExistentPlugin) { class_loader::ClassLoaderException); } +// An instance keeps its library loaded after the MultiLibraryClassLoader that created it is +// destroyed, and the library is unloaded once the last instance goes away. +TYPED_TEST(MultiClassLoaderTest, instanceOutlivesLoader) { + for (bool lazy : {false, true}) { + SCOPED_TRACE(lazy ? "lazy" : "non-lazy"); + typename TypeParam::template Ptr dog; + { + class_loader::MultiLibraryClassLoader loader(lazy); + loader.loadLibrary(LIBRARY_1); + dog = TypeParam::template create(loader, "Dog"); + } + EXPECT_TRUE(class_loader::impl::isLibraryLoadedByAnybody(LIBRARY_1)); + EXPECT_EQ("Bark", dog->saySomething()); + dog.reset(); + EXPECT_FALSE(class_loader::impl::isLibraryLoadedByAnybody(LIBRARY_1)); + } +} + // Tests that don't depend on how instances are managed. TEST(ClassLoaderBasicTest, basicLoadFailures) {