From 0a66234f390393fda608ccc8e6145b615d8d1344 Mon Sep 17 00:00:00 2001
From: Michael Carroll <mjcarroll.oss@gmail.com>
Date: Mon, 2 Nov 2026 14:46:17 -0600
Subject: [PATCH] Don't use a destroyed ClassLoader when deleting its instances

Deleters for managed instances captured the ClassLoader's `this`, so
releasing an instance after its ClassLoader was destroyed was a
heap-use-after-free. Have the deleters share a small context with the
loader instead; the loader clears it on destruction, after which a
deleter just deletes the object (the library is not unloaded while
instances exist).

Fixes the lifetime half of #199 without changing the API.

Signed-off-by: Michael Carroll <mjcarroll.oss@gmail.com>
---
 include/class_loader/class_loader.hpp | 25 +++++++++++++++++++++++--
 src/class_loader.cpp                  |  6 +++++-
 test/utest.cpp                        | 13 +++++++++++++
 3 files changed, 41 insertions(+), 3 deletions(-)

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<Base>(
       createRawInstance<Base>(derived_class_name, true, std::forward<Args>(args)...),
-      [this](Base * p) {onPluginDeletion<Base>(p);}
+      makeDeleter<Base>()
     );
   }
 
@@ -159,7 +159,7 @@ class ClassLoader
     Base * raw = createRawInstance<Base>(derived_class_name, true, std::forward<Args>(args)...);
     return std::unique_ptr<Base, DeleterType<Base>>(
       raw,
-      [this](Base * p) {onPluginDeletion<Base>(p);}
+      makeDeleter<Base>()
     );
   }
 
@@ -266,6 +266,20 @@ class ClassLoader
   int unloadLibrary();
 
 private:
+  /// Deleter for managed instances that stays valid if the instance outlives this ClassLoader.
+  template<class Base>
+  DeleterType<Base> makeDeleter()
+  {
+    return [context = deleter_context_](Base * p) {
+             std::lock_guard<std::recursive_mutex> lock(context->mutex);
+             if (context->loader) {
+               context->loader->onPluginDeletion<Base>(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<bool> has_unmanaged_instance_been_created_;
+
+  struct DeleterContext
+  {
+    std::recursive_mutex mutex;
+    ClassLoader * loader = nullptr;  // Cleared when the ClassLoader is destroyed.
+  };
+  std::shared_ptr<DeleterContext> deleter_context_;
 };
 
 }  // namespace class_loader
diff --git a/src/class_loader.cpp b/src/class_loader.cpp
index 76785bc..2f99418 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<DeleterContext>())
 {
+  deleter_context_->loader = this;
   CONSOLE_BRIDGE_logDebug(
     "class_loader.ClassLoader: "
     "Constructing new ClassLoader (%p) bound to library %s.",
@@ -72,7 +74,9 @@ ClassLoader::~ClassLoader()
     "%s",
     "class_loader.ClassLoader: Destroying class loader, "
     "unloading associated library...\n");
+  std::lock_guard<std::recursive_mutex> lock(deleter_context_->mutex);
   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 f4e13ee..a21636f 100644
--- a/test/utest.cpp
+++ b/test/utest.cpp
@@ -486,6 +486,19 @@ TEST(ClassLoaderGraveyardTest, loadUnloadLoadFromGraveyard) {
 
 #endif  // ifndef _WIN32
 
+// Leaves LIBRARY_2 loaded, so keep this suite last.
+TEST(ClassLoaderLifetimeTest, instancesOutliveLoader) {
+  auto loader = std::make_unique<class_loader::ClassLoader>(LIBRARY_2, false);
+  std::shared_ptr<Base> shared = loader->createInstance<Base>("Robot");
+  class_loader::ClassLoader::UniquePtr<Base> unique = loader->createUniqueInstance<Base>("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)
 {