Have MultiLibraryClassLoader instances keep their ClassLoader alive

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<Base>(class_name, std::forward<Args>(args)...);
+    return keepLoaderAlive<Base>(
+      loader, loader->createInstance<Base>(class_name, std::forward<Args>(args)...));
   }
 
   /**
@@ -137,7 +138,8 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader
               "MultiLibraryClassLoader bound to library " + library_path +
               " Ensure you called MultiLibraryClassLoader::loadLibrary()");
     }
-    return loader->createInstance<Base>(class_name, std::forward<Args>(args)...);
+    return keepLoaderAlive<Base>(
+      loader, loader->createInstance<Base>(class_name, std::forward<Args>(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<Base>(class_name, std::forward<Args>(args)...);
+    return keepLoaderAlive<Base>(
+      loader, loader->createUniqueInstance<Base>(class_name, std::forward<Args>(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<Base>(class_name, std::forward<Args>(args)...);
+    return keepLoaderAlive<Base>(
+      loader, loader->createUniqueInstance<Base>(class_name, std::forward<Args>(args)...));
   }
 
   /**
@@ -333,6 +337,35 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader
   int unloadLibrary(const std::string & library_path);
 
 private:
+  std::shared_ptr<ClassLoader> getSharedClassLoader(const ClassLoader * loader) const;
+
+  /// Makes the instance keep its ClassLoader alive, so it may outlive this MultiLibraryClassLoader.
+  template<class Base>
+  std::shared_ptr<Base> keepLoaderAlive(const ClassLoader * loader, std::shared_ptr<Base> obj) const
+  {
+    struct Holder  // Members are destroyed in reverse order: the instance before its loader.
+    {
+      std::shared_ptr<ClassLoader> loader;
+      std::shared_ptr<Base> obj;
+    };
+    auto holder = std::make_shared<Holder>(Holder{getSharedClassLoader(loader), std::move(obj)});
+    return std::shared_ptr<Base>(holder, holder->obj.get());
+  }
+
+  /// Makes the instance keep its ClassLoader alive, so it may outlive this MultiLibraryClassLoader.
+  template<class Base>
+  ClassLoader::UniquePtr<Base> keepLoaderAlive(
+    const ClassLoader * loader, ClassLoader::UniquePtr<Base> obj) const
+  {
+    auto deleter = obj.get_deleter();
+    return ClassLoader::UniquePtr<Base>(
+      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
    *

src/multi_library_class_loader.cpp

@@ -31,55 +31,21 @@
 
 #include <algorithm>
 #include <cstddef>
+#include <map>
 #include <memory>
-#include <mutex>
 #include <string>
 #include <vector>
 
 namespace class_loader
 {
 
-using ClassLoaderPtrVector = std::vector<std::shared_ptr<class_loader::ClassLoader>>;
-
-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<LibraryPath, std::shared_ptr<ClassLoader>> active_class_loaders_;
 };
 
-ClassLoaderPtrVectorImpl & getClassLoaderPtrVectorImpl()
-{
-  static ClassLoaderPtrVectorImpl instance;
-  return instance;
-}
-
 MultiLibraryClassLoader::MultiLibraryClassLoader(bool enable_ondemand_loadunload)
 : impl_(std::make_unique<MultiLibraryClassLoaderImpl>())
 {
@@ -104,7 +70,7 @@ std::vector<std::string> 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<std::mutex> lock(getClassLoaderPtrVectorImpl().loader_mutex_);
-    getClassLoaderPtrVectorImpl().class_loader_ptrs_.emplace_back(
-      std::make_shared<class_loader::ClassLoader>(library_path, isOnDemandLoadUnloadEnabled())
-    );
     impl_->active_class_loaders_[library_path] =
-      getClassLoaderPtrVectorImpl().class_loader_ptrs_.back().get();
+      std::make_shared<class_loader::ClassLoader>(library_path, isOnDemandLoadUnloadEnabled());
+  }
+}
+
+std::shared_ptr<ClassLoader>
+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<std::mutex> 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;

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<Base> dog;
+    {
+      class_loader::MultiLibraryClassLoader loader(lazy);
+      loader.loadLibrary(LIBRARY_1);
+      dog = TypeParam::template create<Base>(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) {