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/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);