diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml new file mode 100644 index 00000000..9836444b --- /dev/null +++ b/.github/workflows/ci.yaml @@ -0,0 +1,30 @@ +# This config uses industrial_ci (https://github.com/ros-industrial/industrial_ci.git). +# For troubleshooting, see readme (https://github.com/ros-industrial/industrial_ci/blob/master/README.rst) + +name: CI + +on: + workflow_dispatch: + pull_request: + push: + branches: + - ros2 + +jobs: + ci: + strategy: + fail-fast: false + matrix: + env: + - TARGET_CMAKE_ARGS: -DENABLE_SANITIZER_ADDRESS=ON -DCMAKE_BUILD_TYPE=Debug + NAME: Address Sanitizer + - TARGET_CMAKE_ARGS: -DENABLE_SANITIZER_THREAD=ON -DCMAKE_BUILD_TYPE=Debug + NAME: Thread Sanitizer + env: + ROS_DISTRO: rolling + runs-on: ubuntu-latest + name: ${{ matrix.env.NAME }} + steps: + - uses: actions/checkout@v2 + - uses: 'ros-industrial/industrial_ci@master' + env: ${{matrix.env}} diff --git a/CMakeLists.txt b/CMakeLists.txt index 3f9c7735..23b2338b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -4,7 +4,7 @@ project(class_loader) # Default to C++14 if(NOT CMAKE_CXX_STANDARD) - set(CMAKE_CXX_STANDARD 14) + set(CMAKE_CXX_STANDARD 17) endif() if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") add_compile_options(-Wall -Wextra -Wpedantic) diff --git a/cmake/sanitizers.cmake b/cmake/sanitizers.cmake new file mode 100644 index 00000000..b264fbee --- /dev/null +++ b/cmake/sanitizers.cmake @@ -0,0 +1,68 @@ +function(enable_sanitizers project_name) + + if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR CMAKE_CXX_COMPILER_ID MATCHES ".*Clang") + option(ENABLE_COVERAGE "Enable coverage reporting for gcc/clang" OFF) + + if(ENABLE_COVERAGE) + target_compile_options(${project_name} INTERFACE --coverage -O0 -g -fno-omit-frame-pointer) + target_link_libraries(${project_name} INTERFACE --coverage) + endif() + + set(SANITIZERS "") + + option(ENABLE_SANITIZER_ADDRESS "Enable address sanitizer" OFF) + if(ENABLE_SANITIZER_ADDRESS) + list(APPEND SANITIZERS "address") + endif() + + option(ENABLE_SANITIZER_LEAK "Enable leak sanitizer" OFF) + if(ENABLE_SANITIZER_LEAK) + list(APPEND SANITIZERS "leak") + endif() + + option(ENABLE_SANITIZER_UNDEFINED_BEHAVIOR "Enable undefined behavior sanitizer" OFF) + if(ENABLE_SANITIZER_UNDEFINED_BEHAVIOR) + list(APPEND SANITIZERS "undefined") + endif() + + option(ENABLE_SANITIZER_THREAD "Enable thread sanitizer" OFF) + if(ENABLE_SANITIZER_THREAD) + if("address" IN_LIST SANITIZERS OR "leak" IN_LIST SANITIZERS) + message(WARNING "Thread sanitizer does not work with Address and Leak sanitizer enabled") + else() + list(APPEND SANITIZERS "thread") + endif() + endif() + + option(ENABLE_SANITIZER_MEMORY "Enable memory sanitizer" OFF) + if(ENABLE_SANITIZER_MEMORY AND CMAKE_CXX_COMPILER_ID MATCHES ".*Clang") + message(WARNING "Memory sanitizer requires all the code (including libc++) \ + to be MSan-instrumented otherwise it reports false positives") + if("address" IN_LIST SANITIZERS + OR "thread" IN_LIST SANITIZERS + OR "leak" IN_LIST SANITIZERS) + message(WARNING "Memory sanitizer does not work with Address, Thread and Leak sanitizer enabled") + else() + list(APPEND SANITIZERS "memory") + endif() + endif() + + list( + JOIN + SANITIZERS + "," + LIST_OF_SANITIZERS) + + endif() + + if(LIST_OF_SANITIZERS) + if(NOT + "${LIST_OF_SANITIZERS}" + STREQUAL + "") + target_compile_options(${project_name} INTERFACE -fsanitize=${LIST_OF_SANITIZERS}) + target_link_options(${project_name} INTERFACE -fsanitize=${LIST_OF_SANITIZERS}) + endif() + endif() + +endfunction() diff --git a/include/class_loader/class_loader.hpp b/include/class_loader/class_loader.hpp index bda67e56..a383c9bc 100644 --- a/include/class_loader/class_loader.hpp +++ b/include/class_loader/class_loader.hpp @@ -76,7 +76,7 @@ std::string systemLibraryFormat(const std::string & library_name); * definitions from which objects can be created/destroyed during runtime (i.e. class_loader). * Libraries loaded by a ClassLoader are only accessible within scope of that ClassLoader object. */ -class ClassLoader +class ClassLoader : public std::enable_shared_from_this { public: template @@ -86,14 +86,17 @@ class ClassLoader using UniquePtr = std::unique_ptr>; /** - * @brief Constructor for ClassLoader + * @brief This is required (as apposed to the old constructor) to enforce that the lifetime of the ClassLoader + * can be preserved by objects that this class creates. * * @param library_path - The path of the runtime library to load * @param ondemand_load_unload - Indicates if on-demand (lazy) unloading/loading of libraries * occurs as plugins are created/destroyed. */ CLASS_LOADER_PUBLIC - explicit ClassLoader(const std::string & library_path, bool ondemand_load_unload = false); + [[nodiscard]] static std::shared_ptr Make( + const std::string & library_path, + bool ondemand_load_unload = false); /** * @brief Destructor for ClassLoader. All libraries opened by this ClassLoader are unloaded automatically. @@ -109,7 +112,7 @@ class ClassLoader template std::vector getAvailableClasses() const { - return class_loader::impl::getAvailableClasses(this); + return class_loader::impl::getAvailableClasses(shared_from_this()); } /** @@ -126,7 +129,9 @@ class ClassLoader { return std::shared_ptr( createRawInstance(derived_class_name, true), - std::bind(&ClassLoader::onPluginDeletion, this, std::placeholders::_1) + [class_loader = shared_from_this()](Base * obj) { + return class_loader->onPluginDeletion(obj); + } ); } @@ -149,7 +154,9 @@ class ClassLoader Base * raw = createRawInstance(derived_class_name, true); return std::unique_ptr>( raw, - std::bind(&ClassLoader::onPluginDeletion, this, std::placeholders::_1) + [class_loader = shared_from_this()](Base * obj) { + return class_loader->onPluginDeletion(obj); + } ); } @@ -253,6 +260,16 @@ class ClassLoader int unloadLibrary(); private: + /** + * @brief Private Constructor for ClassLoader + * + * @param library_path - The path of the runtime library to load + * @param ondemand_load_unload - Indicates if on-demand (lazy) unloading/loading of libraries + * occurs as plugins are created/destroyed. + */ + CLASS_LOADER_PUBLIC + explicit ClassLoader(const std::string & library_path, bool ondemand_load_unload = false); + /** * @brief Callback method when a plugin created by this class loader is destroyed * @@ -324,7 +341,7 @@ class ClassLoader loadLibrary(); } - Base * obj = class_loader::impl::createInstance(derived_class_name, this); + Base * obj = class_loader::impl::createInstance(derived_class_name, shared_from_this()); assert(obj != NULL); // Unreachable assertion if createInstance() throws on failure. if (managed) { diff --git a/include/class_loader/class_loader_core.hpp b/include/class_loader/class_loader_core.hpp index bfb17fe3..39e64a4b 100644 --- a/include/class_loader/class_loader_core.hpp +++ b/include/class_loader/class_loader_core.hpp @@ -76,11 +76,14 @@ namespace impl typedef std::string LibraryPath; typedef std::string ClassName; typedef std::string BaseClassName; -typedef std::map FactoryMap; +typedef std::map> FactoryMap; typedef std::map BaseToFactoryMapMap; typedef std::pair> LibraryPair; typedef std::vector LibraryVector; -typedef std::vector MetaObjectVector; +typedef std::vector> MetaObjectVector; + +CLASS_LOADER_PUBLIC +MetaObjectVector & getMetaObjectGraveyard(); CLASS_LOADER_PUBLIC void printDebugInfoToScreen(); @@ -139,8 +142,15 @@ ClassLoader * getCurrentlyActiveClassLoader(); * @param loader - pointer to the currently active ClassLoader. */ CLASS_LOADER_PUBLIC -void setCurrentlyActiveClassLoader(ClassLoader * loader); +void setCurrentlyActiveClassLoader(std::shared_ptr loader); +/** + * @brief Inserts meta object into the graveyard to preserve the lifetime. + * + * @param meta_obj - pointer to the meta object. + */ +CLASS_LOADER_PUBLIC +void insertMetaObjectIntoGraveyard(std::shared_ptr meta_obj); /** * @brief This function extracts a reference to the FactoryMap for appropriate base class out of @@ -240,8 +250,8 @@ void registerPlugin(const std::string & class_name, const std::string & base_cla } // Create factory - impl::AbstractMetaObject * new_factory = - new impl::MetaObject(class_name, base_class_name); + auto new_factory = + std::make_shared>(class_name, base_class_name); new_factory->addOwningClassLoader(getCurrentlyActiveClassLoader()); new_factory->setAssociatedLibraryPath(getCurrentlyLoadingLibraryName()); @@ -260,13 +270,17 @@ void registerPlugin(const std::string & class_name, const std::string & base_cla "and use either class_loader::ClassLoader/MultiLibraryClassLoader to open.", class_name.c_str()); } + + // We insert every factory into the graveyard to preserve the lifetime of all meta objects + // until the process exits. + insertMetaObjectIntoGraveyard(new_factory); factoryMap[class_name] = new_factory; getPluginBaseToFactoryMapMapMutex().unlock(); CONSOLE_BRIDGE_logDebug( "class_loader.impl: " "Registration of %s complete (Metaobject Address = %p)", - class_name.c_str(), reinterpret_cast(new_factory)); + class_name.c_str(), reinterpret_cast(new_factory.get())); } /** @@ -278,14 +292,14 @@ void registerPlugin(const std::string & class_name, const std::string & base_cla * @return A pointer to newly created plugin, note caller is responsible for object destruction */ template -Base * createInstance(const std::string & derived_class_name, ClassLoader * loader) +Base * createInstance(const std::string & derived_class_name, std::shared_ptr loader) { AbstractMetaObject * factory = nullptr; getPluginBaseToFactoryMapMapMutex().lock(); FactoryMap & factoryMap = getFactoryMapForBaseClass(); if (factoryMap.find(derived_class_name) != factoryMap.end()) { - factory = dynamic_cast *>(factoryMap[derived_class_name]); + factory = dynamic_cast *>(factoryMap[derived_class_name].get()); } else { CONSOLE_BRIDGE_logError( "class_loader.impl: No metaobject exists for class type %s.", derived_class_name.c_str()); @@ -293,7 +307,7 @@ Base * createInstance(const std::string & derived_class_name, ClassLoader * load getPluginBaseToFactoryMapMapMutex().unlock(); Base * obj = nullptr; - if (factory != nullptr && factory->isOwnedBy(loader)) { + if (factory != nullptr && factory->isOwnedBy(loader.get())) { obj = factory->create(); } @@ -333,7 +347,7 @@ Base * createInstance(const std::string & derived_class_name, ClassLoader * load * @return A vector of strings where each string is a plugin we can create */ template -std::vector getAvailableClasses(const ClassLoader * loader) +std::vector getAvailableClasses(std::shared_ptr loader) { std::lock_guard lock(getPluginBaseToFactoryMapMapMutex()); @@ -342,8 +356,8 @@ std::vector getAvailableClasses(const ClassLoader * loader) std::vector classes_with_no_owner; for (auto & it : factory_map) { - AbstractMetaObjectBase * factory = it.second; - if (factory->isOwnedBy(loader)) { + auto factory = it.second; + if (factory->isOwnedBy(loader.get())) { classes.push_back(it.first); } else if (factory->isOwnedBy(nullptr)) { classes_with_no_owner.push_back(it.first); @@ -364,7 +378,8 @@ std::vector getAvailableClasses(const ClassLoader * loader) * within a ClassLoader's visible scope */ CLASS_LOADER_PUBLIC -std::vector getAllLibrariesUsedByClassLoader(const ClassLoader * loader); +std::vector getAllLibrariesUsedByClassLoader( + std::shared_ptr loader); /** * @brief Indicates if passed library loaded within scope of a ClassLoader. @@ -375,7 +390,7 @@ std::vector getAllLibrariesUsedByClassLoader(const ClassLoader * lo * @return true if the library is loaded within loader's scope, else false */ CLASS_LOADER_PUBLIC -bool isLibraryLoaded(const std::string & library_path, const ClassLoader * loader); +bool isLibraryLoaded(const std::string & library_path, std::shared_ptr loader); /** * @brief Indicates if passed library has been loaded by ANY ClassLoader diff --git a/include/class_loader/meta_object.hpp b/include/class_loader/meta_object.hpp index 24801936..622a622b 100644 --- a/include/class_loader/meta_object.hpp +++ b/include/class_loader/meta_object.hpp @@ -64,12 +64,11 @@ class CLASS_LOADER_PUBLIC AbstractMetaObjectBase const std::string & class_name, const std::string & base_class_name, const std::string & typeid_base_class_name = "UNSET"); + /** - * @brief Destructor for the class. THIS MUST NOT BE VIRTUAL AND OVERRIDDEN BY - * TEMPLATE SUBCLASSES, OTHERWISE THEY WILL PULL IN A REDUNDANT METAOBJECT - * DESTRUCTOR OUTSIDE OF libclass_loader WITHIN THE PLUGIN LIBRARY! T + * @brief Default virtual destructor */ - ~AbstractMetaObjectBase(); + virtual ~AbstractMetaObjectBase() = default; /** * @brief Gets the literal name of the class. @@ -144,12 +143,13 @@ class CLASS_LOADER_PUBLIC AbstractMetaObjectBase ClassLoader * getAssociatedClassLoader(size_t index) const; protected: - /** - * This is needed to make base class polymorphic (i.e. have a vtable) - */ - virtual void dummyMethod() {} + typedef std::vector ClassLoaderVector; - AbstractMetaObjectBaseImpl * impl_; + ClassLoaderVector associated_class_loaders_; + std::string associated_library_path_; + std::string base_class_name_; + std::string class_name_; + std::string typeid_base_class_name_; }; /** diff --git a/include/class_loader/multi_library_class_loader.hpp b/include/class_loader/multi_library_class_loader.hpp index f04afc8f..522c456b 100644 --- a/include/class_loader/multi_library_class_loader.hpp +++ b/include/class_loader/multi_library_class_loader.hpp @@ -56,8 +56,8 @@ namespace class_loader { typedef std::string LibraryPath; -typedef std::map LibraryToClassLoaderMap; -typedef std::vector ClassLoaderVector; +typedef std::map> LibraryToClassLoaderMap; +typedef std::vector> ClassLoaderVector; class MultiLibraryClassLoaderImpl; @@ -96,7 +96,7 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader "class_loader::MultiLibraryClassLoader: " "Attempting to create instance of class type %s.", class_name.c_str()); - ClassLoader * loader = getClassLoaderForClass(class_name); + std::shared_ptr loader = getClassLoaderForClass(class_name); if (nullptr == loader) { throw class_loader::CreateClassException( "MultiLibraryClassLoader: Could not create object of class type " + @@ -121,7 +121,7 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader std::shared_ptr createInstance( const std::string & class_name, const std::string & library_path) { - ClassLoader * loader = getClassLoaderForLibrary(library_path); + std::shared_ptr loader = getClassLoaderForLibrary(library_path); if (nullptr == loader) { throw class_loader::NoClassLoaderExistsException( "Could not create instance as there is no ClassLoader in " @@ -146,7 +146,7 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader CONSOLE_BRIDGE_logDebug( "class_loader::MultiLibraryClassLoader: Attempting to create instance of class type %s.", class_name.c_str()); - ClassLoader * loader = getClassLoaderForClass(class_name); + std::shared_ptr loader = getClassLoaderForClass(class_name); if (nullptr == loader) { throw class_loader::CreateClassException( "MultiLibraryClassLoader: Could not create object of class type " + class_name + @@ -170,7 +170,7 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader ClassLoader::UniquePtr createUniqueInstance(const std::string & class_name, const std::string & library_path) { - ClassLoader * loader = getClassLoaderForLibrary(library_path); + std::shared_ptr loader = getClassLoaderForLibrary(library_path); if (nullptr == loader) { throw class_loader::NoClassLoaderExistsException( "Could not create instance as there is no ClassLoader in " @@ -193,7 +193,7 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader template Base * createUnmanagedInstance(const std::string & class_name) { - ClassLoader * loader = getClassLoaderForClass(class_name); + std::shared_ptr loader = getClassLoaderForClass(class_name); if (nullptr == loader) { throw class_loader::CreateClassException( "MultiLibraryClassLoader: Could not create class of type " + class_name); @@ -213,7 +213,7 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader template Base * createUnmanagedInstance(const std::string & class_name, const std::string & library_path) { - ClassLoader * loader = getClassLoaderForLibrary(library_path); + std::shared_ptr loader = getClassLoaderForLibrary(library_path); if (nullptr == loader) { throw class_loader::NoClassLoaderExistsException( "Could not create instance as there is no ClassLoader in MultiLibraryClassLoader " @@ -273,7 +273,7 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader template std::vector getAvailableClassesForLibrary(const std::string & library_path) { - ClassLoader * loader = getClassLoaderForLibrary(library_path); + std::shared_ptr loader = getClassLoaderForLibrary(library_path); if (nullptr == loader) { throw class_loader::NoClassLoaderExistsException( "There is no ClassLoader in MultiLibraryClassLoader bound to library " + @@ -321,7 +321,7 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader * @param library_path - the library from which we want to create the plugin * @return A pointer to the ClassLoader*, == nullptr if not found */ - ClassLoader * getClassLoaderForLibrary(const std::string & library_path); + std::shared_ptr getClassLoaderForLibrary(const std::string & library_path); /// Gets a handle to the class loader corresponding to a specific class. /** @@ -329,7 +329,7 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader * @return A pointer to the ClassLoader, or NULL if not found. */ template - ClassLoader * getClassLoaderForClass(const std::string & class_name) + std::shared_ptr getClassLoaderForClass(const std::string & class_name) { ClassLoaderVector loaders = getAllAvailableClassLoaders(); for (ClassLoaderVector::iterator i = loaders.begin(); i != loaders.end(); ++i) { @@ -355,7 +355,7 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader */ void shutdownAllClassLoaders(); - MultiLibraryClassLoaderImpl * impl_; + std::unique_ptr impl_; }; diff --git a/src/class_loader.cpp b/src/class_loader.cpp index a9df86b9..854967a4 100644 --- a/src/class_loader.cpp +++ b/src/class_loader.cpp @@ -29,6 +29,7 @@ #include "class_loader/class_loader.hpp" +#include #include namespace class_loader @@ -36,6 +37,14 @@ namespace class_loader bool ClassLoader::has_unmananged_instance_been_created_ = false; +std::shared_ptr ClassLoader::Make( + const std::string & library_path, + bool ondemand_load_unload) +{ + // Not using make_shared because the constructor for ClassLoader is private + return std::shared_ptr(new ClassLoader{library_path, ondemand_load_unload}); +} + bool ClassLoader::hasUnmanagedInstanceBeenCreated() { return ClassLoader::has_unmananged_instance_been_created_; @@ -82,7 +91,7 @@ const std::string & ClassLoader::getLibraryPath() const bool ClassLoader::isLibraryLoaded() const { - return class_loader::impl::isLibraryLoaded(getLibraryPath(), this); + return class_loader::impl::isLibraryLoaded(getLibraryPath(), shared_from_this()); } bool ClassLoader::isLibraryLoadedByAnyClassloader() const diff --git a/src/class_loader_core.cpp b/src/class_loader_core.cpp index 56cc7704..088e5c6b 100644 --- a/src/class_loader_core.cpp +++ b/src/class_loader_core.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #include namespace class_loader @@ -165,11 +166,13 @@ MetaObjectVector allMetaObjects() } MetaObjectVector -filterAllMetaObjectsOwnedBy(const MetaObjectVector & to_filter, const ClassLoader * owner) +filterAllMetaObjectsOwnedBy( + const MetaObjectVector & to_filter, + std::shared_ptr owner) { MetaObjectVector filtered_objs; for (auto & f : to_filter) { - if (f->isOwnedBy(owner)) { + if (f->isOwnedBy(owner.get())) { filtered_objs.push_back(f); } } @@ -190,7 +193,7 @@ filterAllMetaObjectsAssociatedWithLibrary( } MetaObjectVector -allMetaObjectsForClassLoader(const ClassLoader * owner) +allMetaObjectsForClassLoader(std::shared_ptr owner) { return filterAllMetaObjectsOwnedBy(allMetaObjects(), owner); } @@ -202,46 +205,36 @@ allMetaObjectsForLibrary(const std::string & library_path) } MetaObjectVector -allMetaObjectsForLibraryOwnedBy(const std::string & library_path, const ClassLoader * owner) +allMetaObjectsForLibraryOwnedBy( + const std::string & library_path, + std::shared_ptr owner) { return filterAllMetaObjectsOwnedBy(allMetaObjectsForLibrary(library_path), owner); } -void insertMetaObjectIntoGraveyard(AbstractMetaObjectBase * meta_obj) +void insertMetaObjectIntoGraveyard(std::shared_ptr meta_obj) { CONSOLE_BRIDGE_logDebug( "class_loader.impl: " "Inserting MetaObject (class = %s, base_class = %s, ptr = %p) into graveyard", meta_obj->className().c_str(), meta_obj->baseClassName().c_str(), - reinterpret_cast(meta_obj)); - getMetaObjectGraveyard().push_back(meta_obj); + reinterpret_cast(meta_obj.get())); + getMetaObjectGraveyard().push_back(std::move(meta_obj)); } void destroyMetaObjectsForLibrary( const std::string & library_path, FactoryMap & factories, const ClassLoader * loader) { - FactoryMap::iterator factory_itr = factories.begin(); - while (factory_itr != factories.end()) { - AbstractMetaObjectBase * meta_obj = factory_itr->second; + CONSOLE_BRIDGE_logDebug( + "class_loader.impl: destroyMetaObjectsForLibrary.\n"); + for (auto factory_itr = + factories.cbegin(); factory_itr != factories.cend() /* not hoisted */; /* no increment */) + { + auto & meta_obj = factory_itr->second; if (meta_obj->getAssociatedLibraryPath() == library_path && meta_obj->isOwnedBy(loader)) { meta_obj->removeOwningClassLoader(loader); if (!meta_obj->isOwnedByAnybody()) { - FactoryMap::iterator factory_itr_copy = factory_itr; - factory_itr++; - // TODO(mikaelarguedas) fix this when branching out for melodic - // Note: map::erase does not return iterator like vector::erase does. - // Hence the ugliness of this code and a need for copy. Should be fixed in next C++ revision - factories.erase(factory_itr_copy); - - // Insert into graveyard - // We remove the metaobject from its factory map, but we don't destroy it...instead it - // saved to a "graveyard" to the side. - // This is due to our static global variable initialization problem that causes factories - // to not be registered when a library is closed and then reopened. - // This is because it's truly not closed due to the use of global symbol binding i.e. - // calling dlopen with RTLD_GLOBAL instead of RTLD_LOCAL. - // We require using the former as the which is required to support RTTI - insertMetaObjectIntoGraveyard(meta_obj); + factory_itr = factories.erase(factory_itr); } else { ++factory_itr; } @@ -302,7 +295,7 @@ bool isLibraryLoadedByAnybody(const std::string & library_path) } } -bool isLibraryLoaded(const std::string & library_path, const ClassLoader * loader) +bool isLibraryLoaded(const std::string & library_path, std::shared_ptr loader) { bool is_lib_loaded_by_anyone = isLibraryLoadedByAnybody(library_path); size_t num_meta_objs_for_lib = allMetaObjectsForLibrary(library_path).size(); @@ -315,7 +308,7 @@ bool isLibraryLoaded(const std::string & library_path, const ClassLoader * loade return is_lib_loaded_by_anyone && are_meta_objs_bound_to_loader; } -std::vector getAllLibrariesUsedByClassLoader(const ClassLoader * loader) +std::vector getAllLibrariesUsedByClassLoader(std::shared_ptr loader) { MetaObjectVector all_loader_meta_objs = allMetaObjectsForClassLoader(loader); std::vector all_libs; @@ -340,7 +333,7 @@ void addClassLoaderOwnerForAllExistingMetaObjectsForLibrary( "class_loader.impl: " "Tagging existing MetaObject %p (base = %s, derived = %s) with " "class loader %p (library path = %s).", - reinterpret_cast(meta_obj), meta_obj->baseClassName().c_str(), + reinterpret_cast(meta_obj.get()), meta_obj->baseClassName().c_str(), meta_obj->className().c_str(), reinterpret_cast(loader), nullptr == loader ? "NULL" : loader->getLibraryPath().c_str()); @@ -360,7 +353,7 @@ void revivePreviouslyCreateMetaobjectsFromGraveyard( "class_loader.impl: " "Resurrected factory metaobject from graveyard, class = %s, base_class = %s ptr = %p..." "bound to ClassLoader %p (library path = %s)", - obj->className().c_str(), obj->baseClassName().c_str(), reinterpret_cast(obj), + obj->className().c_str(), obj->baseClassName().c_str(), reinterpret_cast(obj.get()), reinterpret_cast(loader), nullptr == loader ? "NULL" : loader->getLibraryPath().c_str()); @@ -383,13 +376,13 @@ void purgeGraveyardOfMetaobjects( MetaObjectVector::iterator itr = graveyard.begin(); while (itr != graveyard.end()) { - AbstractMetaObjectBase * obj = *itr; + auto obj = *itr; if (obj->getAssociatedLibraryPath() == library_path) { CONSOLE_BRIDGE_logDebug( "class_loader.impl: " "Purging factory metaobject from graveyard, class = %s, base_class = %s ptr = %p.." ".bound to ClassLoader %p (library path = %s)", - obj->className().c_str(), obj->baseClassName().c_str(), reinterpret_cast(obj), + obj->className().c_str(), obj->baseClassName().c_str(), reinterpret_cast(obj.get()), reinterpret_cast(loader), nullptr == loader ? "NULL" : loader->getLibraryPath().c_str()); @@ -409,16 +402,9 @@ void purgeGraveyardOfMetaobjects( "class_loader.impl: " "Also destroying metaobject %p (class = %s, base_class = %s, library_path = %s) " "in addition to purging it from graveyard.", - reinterpret_cast(obj), obj->className().c_str(), obj->baseClassName().c_str(), + reinterpret_cast(obj.get()), + obj->className().c_str(), obj->baseClassName().c_str(), obj->getAssociatedLibraryPath().c_str()); -#ifndef _WIN32 -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wdelete-non-virtual-dtor" -#endif - delete (obj); // Note: This is the only place where metaobjects can be destroyed -#ifndef _WIN32 -#pragma GCC diagnostic pop -#endif } } } else { @@ -579,11 +565,11 @@ void printDebugInfoToScreen() printf("--------------------------------------------------------------------------------\n"); MetaObjectVector meta_objs = allMetaObjects(); for (size_t c = 0; c < meta_objs.size(); c++) { - AbstractMetaObjectBase * obj = meta_objs.at(c); + auto obj = meta_objs.at(c); printf( "Metaobject %zu (ptr = %p):\n TypeId = %s\n Associated Library = %s\n", c, - reinterpret_cast(obj), + reinterpret_cast(obj.get()), (typeid(*obj).name()), obj->getAssociatedLibraryPath().c_str()); diff --git a/src/meta_object.cpp b/src/meta_object.cpp index 002b55e2..32be1b0f 100644 --- a/src/meta_object.cpp +++ b/src/meta_object.cpp @@ -38,70 +38,48 @@ namespace class_loader namespace impl { -typedef std::vector ClassLoaderVector; - -class AbstractMetaObjectBaseImpl -{ -public: - ClassLoaderVector associated_class_loaders_; - std::string associated_library_path_; - std::string base_class_name_; - std::string class_name_; - std::string typeid_base_class_name_; -}; - AbstractMetaObjectBase::AbstractMetaObjectBase( const std::string & class_name, const std::string & base_class_name, const std::string & typeid_base_class_name) -: impl_(new AbstractMetaObjectBaseImpl()) +: associated_library_path_{"Unknown"}, + base_class_name_{base_class_name}, + class_name_{class_name}, + typeid_base_class_name_{typeid_base_class_name} { - impl_->associated_library_path_ = "Unknown"; - impl_->base_class_name_ = base_class_name; - impl_->class_name_ = class_name; - impl_->typeid_base_class_name_ = typeid_base_class_name; CONSOLE_BRIDGE_logDebug( "class_loader.impl.AbstractMetaObjectBase: Creating MetaObject %p " "(base = %s, derived = %s, library path = %s)", this, baseClassName().c_str(), className().c_str(), getAssociatedLibraryPath().c_str()); } -AbstractMetaObjectBase::~AbstractMetaObjectBase() -{ - CONSOLE_BRIDGE_logDebug( - "class_loader.impl.AbstractMetaObjectBase: " - "Destroying MetaObject %p (base = %s, derived = %s, library path = %s)", - this, baseClassName().c_str(), className().c_str(), getAssociatedLibraryPath().c_str()); - delete impl_; -} - const std::string & AbstractMetaObjectBase::className() const { - return impl_->class_name_; + return class_name_; } const std::string & AbstractMetaObjectBase::baseClassName() const { - return impl_->base_class_name_; + return base_class_name_; } const std::string & AbstractMetaObjectBase::typeidBaseClassName() const { - return impl_->typeid_base_class_name_; + return typeid_base_class_name_; } const std::string & AbstractMetaObjectBase::getAssociatedLibraryPath() const { - return impl_->associated_library_path_; + return associated_library_path_; } void AbstractMetaObjectBase::setAssociatedLibraryPath(const std::string & library_path) { - impl_->associated_library_path_ = library_path; + associated_library_path_ = library_path; } void AbstractMetaObjectBase::addOwningClassLoader(ClassLoader * loader) { - ClassLoaderVector & v = impl_->associated_class_loaders_; + ClassLoaderVector & v = associated_class_loaders_; if (std::find(v.begin(), v.end(), loader) == v.end()) { v.push_back(loader); } @@ -109,7 +87,7 @@ void AbstractMetaObjectBase::addOwningClassLoader(ClassLoader * loader) void AbstractMetaObjectBase::removeOwningClassLoader(const ClassLoader * loader) { - ClassLoaderVector & v = impl_->associated_class_loaders_; + ClassLoaderVector & v = associated_class_loaders_; ClassLoaderVector::iterator itr = std::find(v.begin(), v.end(), loader); if (itr != v.end()) { v.erase(itr); @@ -118,24 +96,24 @@ void AbstractMetaObjectBase::removeOwningClassLoader(const ClassLoader * loader) bool AbstractMetaObjectBase::isOwnedBy(const ClassLoader * loader) const { - const ClassLoaderVector & v = impl_->associated_class_loaders_; + const ClassLoaderVector & v = associated_class_loaders_; auto it = std::find(v.begin(), v.end(), loader); return it != v.end(); } bool AbstractMetaObjectBase::isOwnedByAnybody() const { - return impl_->associated_class_loaders_.size() > 0; + return associated_class_loaders_.size() > 0; } size_t AbstractMetaObjectBase::getAssociatedClassLoadersCount() const { - return impl_->associated_class_loaders_.size(); + return associated_class_loaders_.size(); } ClassLoader * AbstractMetaObjectBase::getAssociatedClassLoader(size_t index) const { - return impl_->associated_class_loaders_[index]; + return associated_class_loaders_[index]; } } // namespace impl diff --git a/src/multi_library_class_loader.cpp b/src/multi_library_class_loader.cpp index c22f4888..eb836547 100644 --- a/src/multi_library_class_loader.cpp +++ b/src/multi_library_class_loader.cpp @@ -30,6 +30,7 @@ #include "class_loader/multi_library_class_loader.hpp" #include +#include #include #include #include @@ -46,7 +47,7 @@ class MultiLibraryClassLoaderImpl }; MultiLibraryClassLoader::MultiLibraryClassLoader(bool enable_ondemand_loadunload) -: impl_(new MultiLibraryClassLoaderImpl()) +: impl_(std::make_unique()) { impl_->enable_ondemand_loadunload_ = enable_ondemand_loadunload; } @@ -54,7 +55,6 @@ MultiLibraryClassLoader::MultiLibraryClassLoader(bool enable_ondemand_loadunload MultiLibraryClassLoader::~MultiLibraryClassLoader() { shutdownAllClassLoaders(); - delete impl_; } std::vector MultiLibraryClassLoader::getRegisteredLibraries() const @@ -68,7 +68,8 @@ std::vector MultiLibraryClassLoader::getRegisteredLibraries() const return libraries; } -ClassLoader * MultiLibraryClassLoader::getClassLoaderForLibrary(const std::string & library_path) +std::shared_ptr MultiLibraryClassLoader::getClassLoaderForLibrary( + const std::string & library_path) { return impl_->active_class_loaders_[library_path]; } @@ -93,7 +94,7 @@ void MultiLibraryClassLoader::loadLibrary(const std::string & library_path) { if (!isLibraryAvailable(library_path)) { impl_->active_class_loaders_[library_path] = - new class_loader::ClassLoader(library_path, isOnDemandLoadUnloadEnabled()); + class_loader::ClassLoader::Make(library_path, isOnDemandLoadUnloadEnabled()); } } @@ -108,11 +109,10 @@ int MultiLibraryClassLoader::unloadLibrary(const std::string & library_path) { int remaining_unloads = 0; if (isLibraryAvailable(library_path)) { - ClassLoader * loader = getClassLoaderForLibrary(library_path); + std::shared_ptr loader = getClassLoaderForLibrary(library_path); remaining_unloads = loader->unloadLibrary(); if (remaining_unloads == 0) { - impl_->active_class_loaders_[library_path] = nullptr; - delete (loader); + impl_->active_class_loaders_.erase(library_path); } } return remaining_unloads; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 50f0b67a..640a1ff3 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -2,6 +2,11 @@ find_package(ament_cmake_gtest REQUIRED) include("../cmake/class_loader_hide_library_symbols.cmake") +# Link this 'library' to set the compile-time options requested +add_library(project_options INTERFACE) +include(../cmake/sanitizers.cmake) +enable_sanitizers(project_options) + add_library(${PROJECT_NAME}_TestPlugins1 EXCLUDE_FROM_ALL SHARED plugins1.cpp) if(ament_cmake_FOUND) target_include_directories(${PROJECT_NAME}_TestPlugins1 @@ -60,7 +65,7 @@ if(TARGET ${PROJECT_NAME}_utest) PUBLIC "../include" ${console_bridge_INCLUDE_DIRS}) target_link_libraries(${PROJECT_NAME}_utest) endif() - target_link_libraries(${PROJECT_NAME}_utest ${PROJECT_NAME}) + target_link_libraries(${PROJECT_NAME}_utest ${PROJECT_NAME} project_options) if(NOT WIN32) target_link_libraries(${PROJECT_NAME}_utest pthread) endif() @@ -83,7 +88,7 @@ if(TARGET ${PROJECT_NAME}_unique_ptr_test) PUBLIC "../include") target_link_libraries(${PROJECT_NAME}_unique_ptr_test) endif() - target_link_libraries(${PROJECT_NAME}_unique_ptr_test ${PROJECT_NAME}) + target_link_libraries(${PROJECT_NAME}_unique_ptr_test ${PROJECT_NAME} project_options) if(NOT WIN32) target_link_libraries(${PROJECT_NAME}_unique_ptr_test pthread) endif() diff --git a/test/fviz_case_study/fviz_main.cpp b/test/fviz_case_study/fviz_main.cpp index cb385178..20a82c69 100644 --- a/test/fviz_case_study/fviz_main.cpp +++ b/test/fviz_case_study/fviz_main.cpp @@ -27,9 +27,9 @@ int main(void) foo("starting fviz"); foo("loading plugin: " + name); try { - class_loader::ClassLoader loader(name); - loader.createInstance("Bar")->speak(); - loader.createInstance("Baz")->speak(); + auto loader = class_loader::ClassLoader::Make(name); + loader->createInstance("Bar")->speak(); + loader->createInstance("Baz")->speak(); } catch (const class_loader::ClassLoaderException & e) { fprintf(stderr, "ClassLoaderException: %s\n", e.what()); throw; diff --git a/test/fviz_case_study/fviz_test.cpp b/test/fviz_case_study/fviz_test.cpp index 7217e534..35bb7842 100644 --- a/test/fviz_case_study/fviz_test.cpp +++ b/test/fviz_case_study/fviz_test.cpp @@ -23,9 +23,9 @@ const std::string name = class_loader::systemLibraryFormat("class_loader_Test_Fv TEST(FvizTest, basic_test) { try { - class_loader::ClassLoader loader(name); - loader.createInstance("Bar")->speak(); - loader.createInstance("Baz")->speak(); + auto loader = class_loader::ClassLoader::Make(name); + loader->createInstance("Bar")->speak(); + loader->createInstance("Baz")->speak(); } catch (const class_loader::ClassLoaderException & e) { FAIL() << "ClassLoaderException: " << e.what() << "\n"; } diff --git a/test/unique_ptr_test.cpp b/test/unique_ptr_test.cpp index 9d907a12..83012b9b 100644 --- a/test/unique_ptr_test.cpp +++ b/test/unique_ptr_test.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -41,6 +42,8 @@ #include "class_loader/class_loader.hpp" #include "class_loader/multi_library_class_loader.hpp" +#include "console_bridge/console.h" + #include "base.hpp" const std::string LIBRARY_1 = class_loader::systemLibraryFormat("class_loader_TestPlugins1"); // NOLINT @@ -49,25 +52,20 @@ const std::string LIBRARY_2 = class_loader::systemLibraryFormat("class_loader_Te using class_loader::ClassLoader; TEST(ClassLoaderUniquePtrTest, basicLoad) { - try { - ClassLoader loader1(LIBRARY_1, false); - loader1.createUniqueInstance("Cat")->saySomething(); // See if lazy load works - ASSERT_NO_THROW(class_loader::impl::printDebugInfoToScreen()); - SUCCEED(); - } catch (class_loader::ClassLoaderException & e) { - FAIL() << "ClassLoaderException: " << e.what() << "\n"; - } + auto loader1 = ClassLoader::Make(LIBRARY_1, false); + loader1->createUniqueInstance("Cat")->saySomething(); // See if lazy load works + ASSERT_NO_THROW(class_loader::impl::printDebugInfoToScreen()); } TEST(ClassLoaderUniquePtrTest, basicLoadFailures) { - ClassLoader loader1(LIBRARY_1, false); - ClassLoader loader2("", false); - loader2.loadLibrary(); + auto loader1 = ClassLoader::Make(LIBRARY_1, false); + auto loader2 = ClassLoader::Make("", false); + loader2->loadLibrary(); EXPECT_THROW( - class_loader::impl::loadLibrary("LIBRARY_1", &loader1), + class_loader::impl::loadLibrary("LIBRARY_1", loader1.get()), class_loader::LibraryLoadException); EXPECT_THROW( - class_loader::impl::unloadLibrary("LIBRARY_1", &loader1), + class_loader::impl::unloadLibrary("LIBRARY_1", loader1.get()), class_loader::LibraryUnloadException); } @@ -79,8 +77,8 @@ TEST(ClassLoaderUniquePtrTest, MultiLibraryClassLoaderFailures) { TEST(ClassLoaderUniquePtrTest, LibrariesUsedByClassLoader) { try { - ClassLoader loader1(LIBRARY_1, false); - std::vector v = class_loader::impl::getAllLibrariesUsedByClassLoader(&loader1); + auto loader1 = ClassLoader::Make(LIBRARY_1, false); + std::vector v = class_loader::impl::getAllLibrariesUsedByClassLoader(loader1); ASSERT_EQ(v.size(), 1u); SUCCEED(); } catch (class_loader::ClassLoaderException & e) { @@ -91,14 +89,14 @@ TEST(ClassLoaderUniquePtrTest, LibrariesUsedByClassLoader) { TEST(ClassLoaderUniquePtrTest, correctLazyLoadUnload) { try { ASSERT_FALSE(class_loader::impl::isLibraryLoadedByAnybody(LIBRARY_1)); - ClassLoader loader1(LIBRARY_1, true); + auto loader1 = ClassLoader::Make(LIBRARY_1, true); ASSERT_FALSE(class_loader::impl::isLibraryLoadedByAnybody(LIBRARY_1)); - ASSERT_FALSE(loader1.isLibraryLoaded()); + ASSERT_FALSE(loader1->isLibraryLoaded()); { - ClassLoader::UniquePtr obj = loader1.createUniqueInstance("Cat"); + ClassLoader::UniquePtr obj = loader1->createUniqueInstance("Cat"); ASSERT_TRUE(class_loader::impl::isLibraryLoadedByAnybody(LIBRARY_1)); - ASSERT_TRUE(loader1.isLibraryLoaded()); + ASSERT_TRUE(loader1->isLibraryLoaded()); } // The library will unload automatically when the only plugin object left is destroyed @@ -112,10 +110,10 @@ TEST(ClassLoaderUniquePtrTest, correctLazyLoadUnload) { } TEST(ClassLoaderUniquePtrTest, nonExistentPlugin) { - ClassLoader loader1(LIBRARY_1, false); + auto loader1 = ClassLoader::Make(LIBRARY_1, false); try { - ClassLoader::UniquePtr obj = loader1.createUniqueInstance("Bear"); + ClassLoader::UniquePtr obj = loader1->createUniqueInstance("Bear"); if (nullptr == obj) { FAIL() << "Null object being returned instead of exception thrown."; } @@ -136,7 +134,7 @@ void wait(int seconds) std::this_thread::sleep_for(std::chrono::seconds(seconds)); } -void run(ClassLoader * loader) +void run(std::shared_ptr loader) { std::vector classes = loader->getAvailableClasses(); for (auto & class_ : classes) { @@ -145,8 +143,8 @@ void run(ClassLoader * loader) } TEST(ClassLoaderUniquePtrTest, threadSafety) { - ClassLoader loader1(LIBRARY_1); - ASSERT_TRUE(loader1.isLibraryLoaded()); + auto loader1 = ClassLoader::Make(LIBRARY_1); + ASSERT_TRUE(loader1->isLibraryLoaded()); // Note: Hard to test thread safety to make sure memory isn't corrupted. // The hope is this test is hard enough that once in a while it'll segfault @@ -155,15 +153,15 @@ TEST(ClassLoaderUniquePtrTest, threadSafety) { std::vector client_threads; for (size_t c = 0; c < STRESS_TEST_NUM_THREADS; c++) { - client_threads.emplace_back(std::bind(&run, &loader1)); + client_threads.emplace_back([loader1]() {run(loader1);}); } for (auto & client_thread : client_threads) { client_thread.join(); } - loader1.unloadLibrary(); - ASSERT_FALSE(loader1.isLibraryLoaded()); + loader1->unloadLibrary(); + ASSERT_FALSE(loader1->isLibraryLoaded()); } catch (const class_loader::ClassLoaderException &) { FAIL() << "Unexpected ClassLoaderException."; } catch (...) { @@ -173,33 +171,33 @@ TEST(ClassLoaderUniquePtrTest, threadSafety) { TEST(ClassLoaderUniquePtrTest, loadRefCountingLazy) { try { - ClassLoader loader1(LIBRARY_1, true); - ASSERT_FALSE(loader1.isLibraryLoaded()); + auto loader1 = ClassLoader::Make(LIBRARY_1, true); + ASSERT_FALSE(loader1->isLibraryLoaded()); { - ClassLoader::UniquePtr obj = loader1.createUniqueInstance("Dog"); - ASSERT_TRUE(loader1.isLibraryLoaded()); + ClassLoader::UniquePtr obj = loader1->createUniqueInstance("Dog"); + ASSERT_TRUE(loader1->isLibraryLoaded()); } - ASSERT_FALSE(loader1.isLibraryLoaded()); + ASSERT_FALSE(loader1->isLibraryLoaded()); - loader1.loadLibrary(); - ASSERT_TRUE(loader1.isLibraryLoaded()); + loader1->loadLibrary(); + ASSERT_TRUE(loader1->isLibraryLoaded()); - loader1.loadLibrary(); - ASSERT_TRUE(loader1.isLibraryLoaded()); + loader1->loadLibrary(); + ASSERT_TRUE(loader1->isLibraryLoaded()); - loader1.unloadLibrary(); - ASSERT_TRUE(loader1.isLibraryLoaded()); + loader1->unloadLibrary(); + ASSERT_TRUE(loader1->isLibraryLoaded()); - loader1.unloadLibrary(); - ASSERT_FALSE(loader1.isLibraryLoaded()); + loader1->unloadLibrary(); + ASSERT_FALSE(loader1->isLibraryLoaded()); - loader1.unloadLibrary(); - ASSERT_FALSE(loader1.isLibraryLoaded()); + loader1->unloadLibrary(); + ASSERT_FALSE(loader1->isLibraryLoaded()); - loader1.loadLibrary(); - ASSERT_TRUE(loader1.isLibraryLoaded()); + loader1->loadLibrary(); + ASSERT_TRUE(loader1->isLibraryLoaded()); return; } catch (const class_loader::ClassLoaderException &) { @@ -263,6 +261,7 @@ TEST(MultiClassLoaderUniquePtrTest, noWarningOnLazyLoad) { // Run all the tests that were declared with TEST() int main(int argc, char ** argv) { + console_bridge::setLogLevel(console_bridge::CONSOLE_BRIDGE_LOG_DEBUG); testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); } diff --git a/test/utest.cpp b/test/utest.cpp index 6a0870b5..607c2e83 100644 --- a/test/utest.cpp +++ b/test/utest.cpp @@ -57,9 +57,9 @@ const std::string GLOBAL_PLUGINS = // NOLINT TEST(ClassLoaderTest, basicLoad) { try { - class_loader::ClassLoader loader1(LIBRARY_1, false); + auto loader1 = class_loader::ClassLoader::Make(LIBRARY_1, false); ASSERT_NO_THROW(class_loader::impl::printDebugInfoToScreen()); - loader1.createInstance("Cat")->saySomething(); // See if lazy load works + loader1->createInstance("Cat")->saySomething(); // See if lazy load works } catch (class_loader::ClassLoaderException & e) { FAIL() << "ClassLoaderException: " << e.what() << "\n"; } @@ -70,8 +70,8 @@ TEST(ClassLoaderTest, basicLoad) { // Requires separate namespace so static variables are isolated TEST(ClassLoaderUnmanagedTest, basicLoadUnmanaged) { try { - class_loader::ClassLoader loader1(LIBRARY_1, false); - Base * unmanaged_instance = loader1.createUnmanagedInstance("Dog"); + auto loader1 = class_loader::ClassLoader::Make(LIBRARY_1, false); + Base * unmanaged_instance = loader1->createUnmanagedInstance("Dog"); ASSERT_NE(unmanaged_instance, nullptr); unmanaged_instance->saySomething(); delete unmanaged_instance; @@ -83,14 +83,14 @@ TEST(ClassLoaderUnmanagedTest, basicLoadUnmanaged) { } TEST(ClassLoaderUniquePtrTest, basicLoadFailures) { - class_loader::ClassLoader loader1(LIBRARY_1, false); - class_loader::ClassLoader loader2("", false); - loader2.loadLibrary(); + auto loader1 = class_loader::ClassLoader::Make(LIBRARY_1, false); + auto loader2 = class_loader::ClassLoader::Make("", false); + loader2->loadLibrary(); EXPECT_THROW( - class_loader::impl::loadLibrary("LIBRARY_1", &loader1), + class_loader::impl::loadLibrary("LIBRARY_1", loader1.get()), class_loader::LibraryLoadException); EXPECT_THROW( - class_loader::impl::unloadLibrary("LIBRARY_1", &loader1), + class_loader::impl::unloadLibrary("LIBRARY_1", loader1.get()), class_loader::LibraryUnloadException); } @@ -103,12 +103,12 @@ TEST(ClassLoaderUniquePtrTest, MultiLibraryClassLoaderFailures) { TEST(ClassLoaderTest, correctNonLazyLoadUnload) { try { ASSERT_FALSE(class_loader::impl::isLibraryLoadedByAnybody(LIBRARY_1)); - class_loader::ClassLoader loader1(LIBRARY_1, false); + auto loader1 = class_loader::ClassLoader::Make(LIBRARY_1, false); ASSERT_TRUE(class_loader::impl::isLibraryLoadedByAnybody(LIBRARY_1)); - ASSERT_TRUE(loader1.isLibraryLoaded()); - loader1.unloadLibrary(); + ASSERT_TRUE(loader1->isLibraryLoaded()); + loader1->unloadLibrary(); ASSERT_FALSE(class_loader::impl::isLibraryLoadedByAnybody(LIBRARY_1)); - ASSERT_FALSE(loader1.isLibraryLoaded()); + ASSERT_FALSE(loader1->isLibraryLoaded()); return; } catch (class_loader::ClassLoaderException & e) { FAIL() << "ClassLoaderException: " << e.what() << "\n"; @@ -120,14 +120,14 @@ TEST(ClassLoaderTest, correctNonLazyLoadUnload) { TEST(ClassLoaderTest, correctLazyLoadUnload) { try { ASSERT_FALSE(class_loader::impl::isLibraryLoadedByAnybody(LIBRARY_1)); - class_loader::ClassLoader loader1(LIBRARY_1, true); + auto loader1 = class_loader::ClassLoader::Make(LIBRARY_1, true); ASSERT_FALSE(class_loader::impl::isLibraryLoadedByAnybody(LIBRARY_1)); - ASSERT_FALSE(loader1.isLibraryLoaded()); + ASSERT_FALSE(loader1->isLibraryLoaded()); { - std::shared_ptr obj = loader1.createInstance("Cat"); + std::shared_ptr obj = loader1->createInstance("Cat"); ASSERT_TRUE(class_loader::impl::isLibraryLoadedByAnybody(LIBRARY_1)); - ASSERT_TRUE(loader1.isLibraryLoaded()); + ASSERT_TRUE(loader1->isLibraryLoaded()); } // The library will unload automatically when the only plugin object left is destroyed @@ -141,10 +141,10 @@ TEST(ClassLoaderTest, correctLazyLoadUnload) { } TEST(ClassLoaderTest, nonExistentPlugin) { - class_loader::ClassLoader loader1(LIBRARY_1, false); + auto loader1 = class_loader::ClassLoader::Make(LIBRARY_1, false); try { - std::shared_ptr obj = loader1.createInstance("Bear"); + std::shared_ptr obj = loader1->createInstance("Bear"); if (nullptr == obj) { FAIL() << "Null object being returned instead of exception thrown."; } @@ -162,7 +162,7 @@ TEST(ClassLoaderTest, nonExistentPlugin) { TEST(ClassLoaderTest, nonExistentLibrary) { try { - class_loader::ClassLoader loader1("libDoesNotExist.so", false); + auto loader1 = class_loader::ClassLoader::Make("libDoesNotExist.so", false); } catch (const class_loader::LibraryLoadException &) { SUCCEED(); return; @@ -179,10 +179,10 @@ class InvalidBase TEST(ClassLoaderTest, invalidBase) { try { - class_loader::ClassLoader loader1(LIBRARY_1, false); - if (loader1.isClassAvailable("Cat")) { + auto loader1 = class_loader::ClassLoader::Make(LIBRARY_1, false); + if (loader1->isClassAvailable("Cat")) { FAIL() << "Cat should not be available for InvalidBase"; - } else if (loader1.isClassAvailable("Cat")) { + } else if (loader1->isClassAvailable("Cat")) { SUCCEED(); return; } else { @@ -200,7 +200,7 @@ void wait(int seconds) std::this_thread::sleep_for(std::chrono::seconds(seconds)); } -void run(class_loader::ClassLoader * loader) +void run(std::shared_ptr loader) { std::vector classes = loader->getAvailableClasses(); for (auto & class_ : classes) { @@ -209,8 +209,8 @@ void run(class_loader::ClassLoader * loader) } TEST(ClassLoaderTest, threadSafety) { - class_loader::ClassLoader loader1(LIBRARY_1); - ASSERT_TRUE(loader1.isLibraryLoaded()); + auto loader1 = class_loader::ClassLoader::Make(LIBRARY_1); + ASSERT_TRUE(loader1->isLibraryLoaded()); // Note: Hard to test thread safety to make sure memory isn't corrupted. // The hope is this test is hard enough that once in a while it'll segfault @@ -219,7 +219,7 @@ TEST(ClassLoaderTest, threadSafety) { std::vector client_threads; for (size_t c = 0; c < STRESS_TEST_NUM_THREADS; ++c) { - client_threads.push_back(new std::thread(std::bind(&run, &loader1))); + client_threads.push_back(new std::thread([loader1]() {run(loader1);})); } for (auto & client_thread : client_threads) { @@ -230,8 +230,8 @@ TEST(ClassLoaderTest, threadSafety) { delete (client_thread); } - loader1.unloadLibrary(); - ASSERT_FALSE(loader1.isLibraryLoaded()); + loader1->unloadLibrary(); + ASSERT_FALSE(loader1->isLibraryLoaded()); } catch (const class_loader::ClassLoaderException &) { FAIL() << "Unexpected ClassLoaderException."; } catch (...) { @@ -241,27 +241,27 @@ TEST(ClassLoaderTest, threadSafety) { TEST(ClassLoaderTest, loadRefCountingNonLazy) { try { - class_loader::ClassLoader loader1(LIBRARY_1, false); - ASSERT_TRUE(loader1.isLibraryLoaded()); + auto loader1 = class_loader::ClassLoader::Make(LIBRARY_1, false); + ASSERT_TRUE(loader1->isLibraryLoaded()); - loader1.loadLibrary(); - loader1.loadLibrary(); - ASSERT_TRUE(loader1.isLibraryLoaded()); + loader1->loadLibrary(); + loader1->loadLibrary(); + ASSERT_TRUE(loader1->isLibraryLoaded()); - loader1.unloadLibrary(); - ASSERT_TRUE(loader1.isLibraryLoaded()); + loader1->unloadLibrary(); + ASSERT_TRUE(loader1->isLibraryLoaded()); - loader1.unloadLibrary(); - ASSERT_TRUE(loader1.isLibraryLoaded()); + loader1->unloadLibrary(); + ASSERT_TRUE(loader1->isLibraryLoaded()); - loader1.unloadLibrary(); - ASSERT_FALSE(loader1.isLibraryLoaded()); + loader1->unloadLibrary(); + ASSERT_FALSE(loader1->isLibraryLoaded()); - loader1.unloadLibrary(); - ASSERT_FALSE(loader1.isLibraryLoaded()); + loader1->unloadLibrary(); + ASSERT_FALSE(loader1->isLibraryLoaded()); - loader1.loadLibrary(); - ASSERT_TRUE(loader1.isLibraryLoaded()); + loader1->loadLibrary(); + ASSERT_TRUE(loader1->isLibraryLoaded()); return; } catch (const class_loader::ClassLoaderException &) { @@ -275,33 +275,33 @@ TEST(ClassLoaderTest, loadRefCountingNonLazy) { TEST(ClassLoaderTest, loadRefCountingLazy) { try { - class_loader::ClassLoader loader1(LIBRARY_1, true); - ASSERT_FALSE(loader1.isLibraryLoaded()); + auto loader1 = class_loader::ClassLoader::Make(LIBRARY_1, true); + ASSERT_FALSE(loader1->isLibraryLoaded()); { - std::shared_ptr obj = loader1.createInstance("Dog"); - ASSERT_TRUE(loader1.isLibraryLoaded()); + std::shared_ptr obj = loader1->createInstance("Dog"); + ASSERT_TRUE(loader1->isLibraryLoaded()); } - ASSERT_FALSE(loader1.isLibraryLoaded()); + ASSERT_FALSE(loader1->isLibraryLoaded()); - loader1.loadLibrary(); - ASSERT_TRUE(loader1.isLibraryLoaded()); + loader1->loadLibrary(); + ASSERT_TRUE(loader1->isLibraryLoaded()); - loader1.loadLibrary(); - ASSERT_TRUE(loader1.isLibraryLoaded()); + loader1->loadLibrary(); + ASSERT_TRUE(loader1->isLibraryLoaded()); - loader1.unloadLibrary(); - ASSERT_TRUE(loader1.isLibraryLoaded()); + loader1->unloadLibrary(); + ASSERT_TRUE(loader1->isLibraryLoaded()); - loader1.unloadLibrary(); - ASSERT_FALSE(loader1.isLibraryLoaded()); + loader1->unloadLibrary(); + ASSERT_FALSE(loader1->isLibraryLoaded()); - loader1.unloadLibrary(); - ASSERT_FALSE(loader1.isLibraryLoaded()); + loader1->unloadLibrary(); + ASSERT_FALSE(loader1->isLibraryLoaded()); - loader1.loadLibrary(); - ASSERT_TRUE(loader1.isLibraryLoaded()); + loader1->loadLibrary(); + ASSERT_TRUE(loader1->isLibraryLoaded()); return; } catch (const class_loader::ClassLoaderException &) { @@ -369,9 +369,9 @@ TEST(MultiClassLoaderTest, noWarningOnLazyLoad) { TEST(ClassLoaderGraveyardTest, loadUnloadLoadFromGraveyard) { // This first load/unload adds the plugin to the graveyard try { - class_loader::ClassLoader loader(GLOBAL_PLUGINS, false); - loader.createInstance("Kangaroo")->saySomething(); - loader.unloadLibrary(); + auto loader = class_loader::ClassLoader::Make(GLOBAL_PLUGINS, false); + loader->createInstance("Kangaroo")->saySomething(); + loader->unloadLibrary(); } catch (class_loader::ClassLoaderException & e) { FAIL() << "ClassLoaderException: " << e.what() << "\n"; } @@ -385,13 +385,13 @@ TEST(ClassLoaderGraveyardTest, loadUnloadLoadFromGraveyard) { // This load will cause system to use globally relocatable library. // For testing purposes, this will cause ClassLoader to revive the library from the graveyard. try { - class_loader::ClassLoader loader(GLOBAL_PLUGINS, false); - loader.createInstance("Panda")->saySomething(); - loader.unloadLibrary(); + auto loader = class_loader::ClassLoader::Make(GLOBAL_PLUGINS, false); + loader->createInstance("Panda")->saySomething(); + loader->unloadLibrary(); - loader.loadLibrary(); - loader.createInstance("Hyena")->saySomething(); - loader.unloadLibrary(); + loader->loadLibrary(); + loader->createInstance("Hyena")->saySomething(); + loader->unloadLibrary(); } catch (class_loader::ClassLoaderException & e) { FAIL() << "ClassLoaderException: " << e.what() << "\n"; } @@ -399,9 +399,9 @@ TEST(ClassLoaderGraveyardTest, loadUnloadLoadFromGraveyard) { dlclose(handle); // With all libraries closed, this should act like a normal load/unload. try { - class_loader::ClassLoader loader(GLOBAL_PLUGINS, false); - loader.createInstance("Alpaca")->saySomething(); - loader.unloadLibrary(); + auto loader = class_loader::ClassLoader::Make(GLOBAL_PLUGINS, false); + loader->createInstance("Alpaca")->saySomething(); + loader->unloadLibrary(); } catch (class_loader::ClassLoaderException & e) { FAIL() << "ClassLoaderException: " << e.what() << "\n"; }