From 63dfbb6adcef66d4d9de74ab03612de037f6b428 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Thu, 5 Nov 2026 14:09:27 -0600 Subject: [PATCH 1/2] Keep libraries mapped while unmanaged instances may exist createUnmanagedInstance() returns a raw pointer that class_loader cannot track, but destroying the ClassLoader (or MultiLibraryClassLoader, as pluginlib does) still unloaded the library. Using or deleting the instance afterwards then called into unmapped code and crashed. Pin the library in memory when an unmanaged instance is created (RTLD_NODELETE on POSIX, GET_MODULE_HANDLE_EX_FLAG_PIN on Windows). Loader bookkeeping is unchanged: unloading still detaches the library from its loaders and moves its meta-objects to the graveyard, which revives them if the library is loaded again. Fixes #218 Signed-off-by: Michael Carroll --- include/class_loader/class_loader.hpp | 7 +++++ include/class_loader/class_loader_core.hpp | 12 ++++++++ src/class_loader_core.cpp | 32 ++++++++++++++++++++++ test/utest.cpp | 25 +++++++++++++++++ 4 files changed, 76 insertions(+) diff --git a/include/class_loader/class_loader.hpp b/include/class_loader/class_loader.hpp index 27da8cf..850c7e1 100644 --- a/include/class_loader/class_loader.hpp +++ b/include/class_loader/class_loader.hpp @@ -171,6 +171,8 @@ class ClassLoader * * Creating an unmanaged instance disables dynamically unloading libraries when * managed pointers go out of scope for all class loaders in this process. + * The library the instance came from stays in memory until the process exits, so the instance + * remains usable after this ClassLoader is destroyed or the library is unloaded. * * @param derived_class_name * The name of the class we want to create (@see getAvailableClasses()). @@ -357,6 +359,11 @@ class ClassLoader Base * obj = class_loader::impl::createInstance(derived_class_name, this, std::forward(args)...); + if (!managed) { + // Unmanaged instances may outlive this loader or its library being unloaded. + class_loader::impl::pinLibrary(getLibraryPath()); + } + if (managed) { std::lock_guard lock(plugin_ref_count_mutex_); ++plugin_ref_count_; diff --git a/include/class_loader/class_loader_core.hpp b/include/class_loader/class_loader_core.hpp index 418418b..7605d00 100644 --- a/include/class_loader/class_loader_core.hpp +++ b/include/class_loader/class_loader_core.hpp @@ -469,6 +469,18 @@ void loadLibrary(const std::string & library_path, ClassLoader * loader); CLASS_LOADER_PUBLIC void unloadLibrary(const std::string & library_path, ClassLoader * loader); +/** + * @brief Keeps a loaded library mapped in memory until the process exits. + * + * Used for libraries that unmanaged instances were created from: nothing tells class_loader when + * those are deleted, so the library's code must stay mapped even after it is unloaded. + * Bookkeeping is unaffected; unloadLibrary() still detaches it from its ClassLoaders. + * + * @param library_path - The name of the (already loaded) library to pin + */ +CLASS_LOADER_PUBLIC +void pinLibrary(const std::string & library_path); + } // namespace impl } // namespace class_loader diff --git a/src/class_loader_core.cpp b/src/class_loader_core.cpp index 6c08c37..d59afd2 100644 --- a/src/class_loader_core.cpp +++ b/src/class_loader_core.cpp @@ -37,6 +37,12 @@ #include #include +#ifdef _WIN32 +#include +#else +#include +#endif + namespace class_loader { namespace impl @@ -470,6 +476,32 @@ void loadLibrary(const std::string & library_path, ClassLoader * loader) open_libraries.push_back(LibraryPair(library_path, library_handle)); } +void pinLibrary(const std::string & library_path) +{ + std::lock_guard llv_lock(getLoadedLibraryVectorMutex()); + LibraryVector::iterator itr = findLoadedLibrary(library_path); + if (itr == getLoadedLibraryVector().end()) { + return; + } + const std::string & path = itr->second->get_library_path(); +#ifdef _WIN32 + HMODULE handle = nullptr; + const bool pinned = GetModuleHandleExA(GET_MODULE_HANDLE_EX_FLAG_PIN, path.c_str(), &handle); +#else + // RTLD_NOLOAD only looks up the already-loaded library; RTLD_NODELETE keeps it mapped. + void * handle = dlopen(path.c_str(), RTLD_LAZY | RTLD_NOLOAD | RTLD_NODELETE); + const bool pinned = handle != nullptr; + if (pinned) { + dlclose(handle); // Drops the reference this call added; the library stays pinned. + } +#endif + if (!pinned) { + CONSOLE_BRIDGE_logWarn( + "class_loader.impl: Could not pin library %s; unloading it while unmanaged instances " + "exist will crash.", path.c_str()); + } +} + void unloadLibrary(const std::string & library_path, ClassLoader * loader) { std::lock_guard loader_lock(getLoaderMutex()); diff --git a/test/utest.cpp b/test/utest.cpp index e9c3da1..002bc39 100644 --- a/test/utest.cpp +++ b/test/utest.cpp @@ -302,6 +302,31 @@ TYPED_TEST(MultiClassLoaderTest, nonExistentPlugin) { // Tests that don't depend on how instances are managed. +// ros/class_loader#218: an unmanaged instance must stay usable after its loader is destroyed. +TEST(ClassLoaderUnmanagedTest, instanceOutlivesLoader) { + auto loader = std::make_unique(LIBRARY_1, false); + std::unique_ptr unmanaged(loader->createUnmanagedInstance("Cat")); + loader.reset(); + EXPECT_FALSE(class_loader::impl::isLibraryLoadedByAnybody(LIBRARY_1)); + EXPECT_EQ("Meow", unmanaged->saySomething()); + unmanaged.reset(); + + // The library can still be loaded again afterwards. + class_loader::ClassLoader loader2(LIBRARY_1, false); + EXPECT_EQ("Meow", loader2.createUniqueInstance("Cat")->saySomething()); +} + +// Same, through MultiLibraryClassLoader, as used by pluginlib. +TEST(ClassLoaderUnmanagedTest, instanceOutlivesMultiLibraryClassLoader) { + for (bool lazy : {false, true}) { + auto loader = std::make_unique(lazy); + loader->loadLibrary(LIBRARY_2); + std::unique_ptr unmanaged(loader->createUnmanagedInstance("Robot")); + loader.reset(); + EXPECT_EQ("Beep boop", unmanaged->saySomething()); + } +} + TEST(ClassLoaderBasicTest, basicLoadFailures) { class_loader::ClassLoader loader1(LIBRARY_1, false); class_loader::ClassLoader loader2("", false); From 52997122c87ce992fa76fff3df3d8c4052fdf43d Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Mon, 9 Nov 2026 15:46:20 -0600 Subject: [PATCH 2/2] Move the unmanaged instance tests to unmanaged_test Creating an unmanaged instance now pins its library for the rest of the process, so these tests left TestPlugins1 and TestPlugins2 pinned for every later test in utest. Run them in unmanaged_test, which exists for such process-wide effects. Signed-off-by: Michael Carroll --- test/CMakeLists.txt | 1 + test/unmanaged_test.cpp | 27 +++++++++++++++++++++++++++ test/utest.cpp | 25 ------------------------- 3 files changed, 28 insertions(+), 25 deletions(-) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 8c72c68..ca10ba8 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -66,6 +66,7 @@ if(TARGET ${PROJECT_NAME}_unmanaged_test) target_link_libraries(${PROJECT_NAME}_unmanaged_test ${PROJECT_NAME} project_options) add_dependencies(${PROJECT_NAME}_unmanaged_test ${PROJECT_NAME}_TestPlugins1 + ${PROJECT_NAME}_TestPlugins2 ${PROJECT_NAME}_TestPlugins3) endif() diff --git a/test/unmanaged_test.cpp b/test/unmanaged_test.cpp index 3eeb96b..1125fe0 100644 --- a/test/unmanaged_test.cpp +++ b/test/unmanaged_test.cpp @@ -31,6 +31,7 @@ #include #include "class_loader/class_loader.hpp" +#include "class_loader/multi_library_class_loader.hpp" #include "gtest/gtest.h" @@ -40,6 +41,7 @@ // run in their own executable. const std::string LIBRARY_1 = class_loader::systemLibraryFormat("class_loader_TestPlugins1"); // NOLINT +const std::string LIBRARY_2 = class_loader::systemLibraryFormat("class_loader_TestPlugins2"); // NOLINT const std::string LIBRARY_3 = class_loader::systemLibraryFormat("class_loader_TestPlugins3"); // NOLINT TEST(ClassLoaderUnmanagedTest, basicLoadUnmanaged) { @@ -60,6 +62,31 @@ TEST(ClassLoaderUnmanagedTest, constructorLoadUnmanaged) { delete unmanaged_instance; } +// ros/class_loader#218: an unmanaged instance must stay usable after its loader is destroyed. +TEST(ClassLoaderUnmanagedTest, instanceOutlivesLoader) { + auto loader = std::make_unique(LIBRARY_1, false); + std::unique_ptr unmanaged(loader->createUnmanagedInstance("Cat")); + loader.reset(); + EXPECT_FALSE(class_loader::impl::isLibraryLoadedByAnybody(LIBRARY_1)); + EXPECT_EQ("Meow", unmanaged->saySomething()); + unmanaged.reset(); + + // The library can still be loaded again afterwards. + class_loader::ClassLoader loader2(LIBRARY_1, false); + EXPECT_EQ("Meow", loader2.createUniqueInstance("Cat")->saySomething()); +} + +// Same, through MultiLibraryClassLoader, as used by pluginlib. +TEST(ClassLoaderUnmanagedTest, instanceOutlivesMultiLibraryClassLoader) { + for (bool lazy : {false, true}) { + auto loader = std::make_unique(lazy); + loader->loadLibrary(LIBRARY_2); + std::unique_ptr unmanaged(loader->createUnmanagedInstance("Robot")); + loader.reset(); + EXPECT_EQ("Beep boop", unmanaged->saySomething()); + } +} + int main(int argc, char ** argv) { testing::InitGoogleTest(&argc, argv); diff --git a/test/utest.cpp b/test/utest.cpp index 002bc39..e9c3da1 100644 --- a/test/utest.cpp +++ b/test/utest.cpp @@ -302,31 +302,6 @@ TYPED_TEST(MultiClassLoaderTest, nonExistentPlugin) { // Tests that don't depend on how instances are managed. -// ros/class_loader#218: an unmanaged instance must stay usable after its loader is destroyed. -TEST(ClassLoaderUnmanagedTest, instanceOutlivesLoader) { - auto loader = std::make_unique(LIBRARY_1, false); - std::unique_ptr unmanaged(loader->createUnmanagedInstance("Cat")); - loader.reset(); - EXPECT_FALSE(class_loader::impl::isLibraryLoadedByAnybody(LIBRARY_1)); - EXPECT_EQ("Meow", unmanaged->saySomething()); - unmanaged.reset(); - - // The library can still be loaded again afterwards. - class_loader::ClassLoader loader2(LIBRARY_1, false); - EXPECT_EQ("Meow", loader2.createUniqueInstance("Cat")->saySomething()); -} - -// Same, through MultiLibraryClassLoader, as used by pluginlib. -TEST(ClassLoaderUnmanagedTest, instanceOutlivesMultiLibraryClassLoader) { - for (bool lazy : {false, true}) { - auto loader = std::make_unique(lazy); - loader->loadLibrary(LIBRARY_2); - std::unique_ptr unmanaged(loader->createUnmanagedInstance("Robot")); - loader.reset(); - EXPECT_EQ("Beep boop", unmanaged->saySomething()); - } -} - TEST(ClassLoaderBasicTest, basicLoadFailures) { class_loader::ClassLoader loader1(LIBRARY_1, false); class_loader::ClassLoader loader2("", false);