diff --git a/include/class_loader/class_loader.hpp b/include/class_loader/class_loader.hpp index bda67e56..0ed39360 100644 --- a/include/class_loader/class_loader.hpp +++ b/include/class_loader/class_loader.hpp @@ -75,8 +75,10 @@ std::string systemLibraryFormat(const std::string & library_name); * @brief This class allows loading and unloading of dynamically linked libraries which contain class * 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. + * This class inherit from enable_shared_from_this which means we must used smart pointers, + * otherwise the code might work but it may run into a leak. */ -class ClassLoader +class ClassLoader : public std::enable_shared_from_this { public: template @@ -124,10 +126,25 @@ class ClassLoader template std::shared_ptr createInstance(const std::string & derived_class_name) { - return std::shared_ptr( - createRawInstance(derived_class_name, true), - std::bind(&ClassLoader::onPluginDeletion, this, std::placeholders::_1) - ); + try { + return std::shared_ptr( + createRawInstance(derived_class_name, true), + std::bind(&ClassLoader::onPluginDeletion, shared_from_this(), std::placeholders::_1) + ); + } catch (std::bad_weak_ptr &) { // This is not a shared_ptr + static bool create_instance_message = false; + if (!create_instance_message) { + CONSOLE_BRIDGE_logWarn( + "To createInstance() with a class_loader::ClassLoader " + "instance whose lifetime is not managed by an std::shared_ptr " + "is deprecated."); + create_instance_message = true; + } + return std::shared_ptr( + createRawInstance(derived_class_name, true), + std::bind(&ClassLoader::onPluginDeletion, this, std::placeholders::_1) + ); + } } /// Generates an instance of loadable classes (i.e. class_loader). @@ -147,10 +164,25 @@ class ClassLoader UniquePtr createUniqueInstance(const std::string & derived_class_name) { Base * raw = createRawInstance(derived_class_name, true); - return std::unique_ptr>( - raw, - std::bind(&ClassLoader::onPluginDeletion, this, std::placeholders::_1) - ); + try { + return std::unique_ptr>( + raw, + std::bind(&ClassLoader::onPluginDeletion, shared_from_this(), std::placeholders::_1) + ); + } catch (std::bad_weak_ptr &) { // This is not a shared_ptr + static bool create_unique_instance_message = false; + if (!create_unique_instance_message) { + CONSOLE_BRIDGE_logWarn( + "To createUniqueInstance() with a class_loader::ClassLoader " + "instance whose lifetime is not managed by an std::shared_ptr " + "is deprecated."); + create_unique_instance_message = true; + } + return std::unique_ptr>( + raw, + std::bind(&ClassLoader::onPluginDeletion, this, std::placeholders::_1) + ); + } } /// Generates an instance of loadable classes (i.e. class_loader). diff --git a/include/class_loader/multi_library_class_loader.hpp b/include/class_loader/multi_library_class_loader.hpp index f04afc8f..402234c2 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,8 @@ 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 +330,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) { diff --git a/src/class_loader_core.cpp b/src/class_loader_core.cpp index 56cc7704..335c99bc 100644 --- a/src/class_loader_core.cpp +++ b/src/class_loader_core.cpp @@ -545,6 +545,7 @@ void unloadLibrary(const std::string & library_path, ClassLoader * loader) ", keeping library %s open.", library_path.c_str()); } + purgeGraveyardOfMetaobjects(library_path, loader, true); return; } catch (const std::runtime_error & e) { throw class_loader::LibraryUnloadException( diff --git a/src/meta_object.cpp b/src/meta_object.cpp index 002b55e2..388d0435 100644 --- a/src/meta_object.cpp +++ b/src/meta_object.cpp @@ -71,6 +71,9 @@ AbstractMetaObjectBase::~AbstractMetaObjectBase() "class_loader.impl.AbstractMetaObjectBase: " "Destroying MetaObject %p (base = %s, derived = %s, library path = %s)", this, baseClassName().c_str(), className().c_str(), getAssociatedLibraryPath().c_str()); + for (unsigned int i = 0; i < impl_->associated_class_loaders_.size(); i++) { + delete impl_->associated_class_loaders_[i]; + } delete impl_; } diff --git a/src/multi_library_class_loader.cpp b/src/multi_library_class_loader.cpp index c22f4888..336564e7 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 @@ -68,7 +69,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 +95,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()); + std::make_shared(library_path, isOnDemandLoadUnloadEnabled()); } } @@ -108,11 +110,10 @@ int MultiLibraryClassLoader::unloadLibrary(const std::string & library_path) { int remaining_unloads = 0; if (isLibraryAvailable(library_path)) { - ClassLoader * loader = getClassLoaderForLibrary(library_path); + auto loader = getClassLoaderForLibrary(library_path); remaining_unloads = loader->unloadLibrary(); if (remaining_unloads == 0) { impl_->active_class_loaders_[library_path] = nullptr; - delete (loader); } } return remaining_unloads;