From 77727d823e12d9f63396be4f439cb83b5a99f9db Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Mon, 9 Nov 2026 14:38:34 -0600 Subject: [PATCH] Never destroy class_loader's global state A ClassLoader or MultiLibraryClassLoader with static storage duration, such as a static pluginlib::ClassLoader, is usually created before class_loader's function-local static globals, so it is destroyed after them at exit. Its destructor then unloads its libraries using the already-destroyed library vector, factory maps and mutexes. Allocate the globals on first use and never destroy them. This also makes the MetaObjectGraveyardVector destructor workaround unnecessary. Fixes #33 Signed-off-by: Michael Carroll --- include/class_loader/class_loader_core.hpp | 15 +--------- src/class_loader_core.cpp | 32 ++++++++++++---------- test/utest.cpp | 22 +++++++++++++++ 3 files changed, 41 insertions(+), 28 deletions(-) diff --git a/include/class_loader/class_loader_core.hpp b/include/class_loader/class_loader_core.hpp index 418418b..d94c9d6 100644 --- a/include/class_loader/class_loader_core.hpp +++ b/include/class_loader/class_loader_core.hpp @@ -84,20 +84,7 @@ using BaseToFactoryMapMap = std::map; using LibraryPair = std::pair>; using LibraryVector = std::vector; using MetaObjectVector = std::vector; -class MetaObjectGraveyardVector : public MetaObjectVector -{ -public: - ~MetaObjectGraveyardVector() - { - // Make sure not to access the pointer value in the static variable of `getMetaObjectGraveyard() - // when destroying `meta_object` in the unique_ptr deleter. Because the static variable in - // `getMetaObjectGraveyard()` can be destructed before the static variable - // `g_register_plugin_ ## UniqueID` in some circumstances. - // NOTE of the vector dtor in the STL: if the elements themselves are pointers, the pointed-to - // memory is not touched in any way. Managing the pointer is the user's responsibility. - clear(); - } -}; +using MetaObjectGraveyardVector = MetaObjectVector; CLASS_LOADER_PUBLIC void printDebugInfoToScreen(); diff --git a/src/class_loader_core.cpp b/src/class_loader_core.cpp index 6c08c37..735eadc 100644 --- a/src/class_loader_core.cpp +++ b/src/class_loader_core.cpp @@ -43,17 +43,21 @@ namespace impl { // Global data +// +// These are allocated on first use and never destroyed. A ClassLoader or MultiLibraryClassLoader +// with static storage duration (e.g. a static pluginlib::ClassLoader) may be destroyed after +// them at exit, and still needs them to unload its libraries (ros/class_loader#33). std::recursive_mutex & getLoadedLibraryVectorMutex() { - static std::recursive_mutex m; - return m; + static auto * m = new std::recursive_mutex(); + return *m; } std::recursive_mutex & getPluginBaseToFactoryMapMapMutex() { - static std::recursive_mutex m; - return m; + static auto * m = new std::recursive_mutex(); + return *m; } namespace @@ -61,15 +65,15 @@ namespace // Serializes loadLibrary()/unloadLibrary() so a dlopen() never overlaps a dlclose(). std::recursive_mutex & getLoaderMutex() { - static std::recursive_mutex m; - return m; + static auto * m = new std::recursive_mutex(); + return *m; } } // namespace BaseToFactoryMapMap & getGlobalPluginBaseToFactoryMapMap() { - static BaseToFactoryMapMap instance; - return instance; + static auto * instance = new BaseToFactoryMapMap(); + return *instance; } FactoryMap & getFactoryMapForBaseClass(const std::string & typeid_base_class_name) @@ -81,20 +85,20 @@ FactoryMap & getFactoryMapForBaseClass(const std::string & typeid_base_class_nam MetaObjectGraveyardVector & getMetaObjectGraveyard() { - static MetaObjectGraveyardVector instance; - return instance; + static auto * instance = new MetaObjectGraveyardVector(); + return *instance; } LibraryVector & getLoadedLibraryVector() { - static LibraryVector instance; - return instance; + static auto * instance = new LibraryVector(); + return *instance; } std::string & getCurrentlyLoadingLibraryNameReference() { - static std::string library_name; - return library_name; + static auto * library_name = new std::string(); + return *library_name; } std::string getCurrentlyLoadingLibraryName() diff --git a/test/utest.cpp b/test/utest.cpp index 60da908..1a07974 100644 --- a/test/utest.cpp +++ b/test/utest.cpp @@ -28,6 +28,7 @@ */ #include +#include #ifndef _WIN32 #include @@ -320,6 +321,27 @@ TYPED_TEST(MultiClassLoaderTest, instanceOutlivesLoader) { // Tests that don't depend on how instances are managed. +// ros/class_loader#33: loaders with static storage duration, like a static pluginlib::ClassLoader, +// are destroyed at exit after class_loader's own globals were created, and must still be able to +// unload their libraries then. +TEST(ClassLoaderStaticTest, staticLoadersDestroyedAtExit) { + // Re-run the binary for the child, so the loaders below are created before class_loader's + // globals, as they would be in a program that has a static loader. + GTEST_FLAG_SET(death_test_style, "threadsafe"); + EXPECT_EXIT( + { + static class_loader::MultiLibraryClassLoader multi_loader(false); + static class_loader::ClassLoader loader(LIBRARY_2, false); + multi_loader.loadLibrary(LIBRARY_1); + if (multi_loader.createInstance("Dog")->saySomething() != "Bark" || + loader.createInstance("Robot")->saySomething() != "Beep boop") + { + std::exit(1); + } + std::exit(0); + }, ::testing::ExitedWithCode(0), ""); +} + TEST(ClassLoaderBasicTest, basicLoadFailures) { class_loader::ClassLoader loader1(LIBRARY_1, false); class_loader::ClassLoader loader2("", false);