Improve memory management

Signed-off-by: ahcorde <ahcorde@gmail.com>

Author: ahcorde

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<ClassLoader>
 {
 public:
   template<typename Base>
@@ -122,12 +122,20 @@ class ClassLoader
    * @return A std::shared_ptr<Base> to newly created plugin object
    */
   template<class Base>
-  std::shared_ptr<Base> createInstance(const std::string & derived_class_name)
+  std::shared_ptr<Base> createInstance(const std::string & derived_class_name,
+    bool is_shared_ptr = false)
   {
-    return std::shared_ptr<Base>(
-      createRawInstance<Base>(derived_class_name, true),
-      std::bind(&ClassLoader::onPluginDeletion<Base>, this, std::placeholders::_1)
-    );
+    if (is_shared_ptr) {
+      return std::shared_ptr<Base>(
+        createRawInstance<Base>(derived_class_name, true),
+        std::bind(&ClassLoader::onPluginDeletion<Base>, shared_from_this(), std::placeholders::_1)
+      );
+    } else {
+      return std::shared_ptr<Base>(
+        createRawInstance<Base>(derived_class_name, true),
+        std::bind(&ClassLoader::onPluginDeletion<Base>, this, std::placeholders::_1)
+      );
+    }
   }
 
   /// Generates an instance of loadable classes (i.e. class_loader).
@@ -144,13 +152,21 @@ class ClassLoader
    * @return A std::unique_ptr<Base> to newly created plugin object.
    */
   template<class Base>
-  UniquePtr<Base> createUniqueInstance(const std::string & derived_class_name)
+  UniquePtr<Base> createUniqueInstance(const std::string & derived_class_name,
+    bool is_shared_ptr = false)
   {
     Base * raw = createRawInstance<Base>(derived_class_name, true);
-    return std::unique_ptr<Base, DeleterType<Base>>(
-      raw,
-      std::bind(&ClassLoader::onPluginDeletion<Base>, this, std::placeholders::_1)
-    );
+    if (is_shared_ptr) {
+      return std::unique_ptr<Base, DeleterType<Base>>(
+        raw,
+        std::bind(&ClassLoader::onPluginDeletion<Base>, this, std::placeholders::_1)
+      );
+    } else {
+      return std::unique_ptr<Base, DeleterType<Base>>(
+        raw,
+        std::bind(&ClassLoader::onPluginDeletion<Base>, shared_from_this(), std::placeholders::_1)
+      );
+    }
   }
 
   /// Generates an instance of loadable classes (i.e. class_loader).

include/class_loader/multi_library_class_loader.hpp

@@ -56,8 +56,8 @@ namespace class_loader
 {
 
 typedef std::string LibraryPath;
-typedef std::map<LibraryPath, class_loader::ClassLoader *> LibraryToClassLoaderMap;
-typedef std::vector<ClassLoader *> ClassLoaderVector;
+typedef std::map<LibraryPath, std::shared_ptr<class_loader::ClassLoader>> LibraryToClassLoaderMap;
+typedef std::vector<std::shared_ptr<ClassLoader>> 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<Base>(class_name);
+    std::shared_ptr<class_loader::ClassLoader> loader = getClassLoaderForClass<Base>(class_name);
     if (nullptr == loader) {
       throw class_loader::CreateClassException(
               "MultiLibraryClassLoader: Could not create object of class type " +
@@ -105,7 +105,7 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader
               "was explicitly loaded through MultiLibraryClassLoader::loadLibrary()");
     }
 
-    return loader->createInstance<Base>(class_name);
+    return loader->createInstance<Base>(class_name, true);
   }
 
   /**
@@ -121,14 +121,14 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader
   std::shared_ptr<Base> createInstance(
     const std::string & class_name, const std::string & library_path)
   {
-    ClassLoader * loader = getClassLoaderForLibrary(library_path);
+    std::shared_ptr<class_loader::ClassLoader> loader = getClassLoaderForLibrary(library_path);
     if (nullptr == loader) {
       throw class_loader::NoClassLoaderExistsException(
               "Could not create instance as there is no ClassLoader in "
               "MultiLibraryClassLoader bound to library " + library_path +
               " Ensure you called MultiLibraryClassLoader::loadLibrary()");
     }
-    return loader->createInstance<Base>(class_name);
+    return loader->createInstance<Base>(class_name, true);
   }
 
   /// Creates an instance of an object of given class name with ancestor class Base
@@ -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<Base>(class_name);
+    std::shared_ptr<class_loader::ClassLoader> loader = getClassLoaderForClass<Base>(class_name);
     if (nullptr == loader) {
       throw class_loader::CreateClassException(
               "MultiLibraryClassLoader: Could not create object of class type " + class_name +
@@ -170,14 +170,14 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader
   ClassLoader::UniquePtr<Base>
   createUniqueInstance(const std::string & class_name, const std::string & library_path)
   {
-    ClassLoader * loader = getClassLoaderForLibrary(library_path);
+    std::shared_ptr<class_loader::ClassLoader> loader = getClassLoaderForLibrary(library_path);
     if (nullptr == loader) {
       throw class_loader::NoClassLoaderExistsException(
               "Could not create instance as there is no ClassLoader in "
               "MultiLibraryClassLoader bound to library " + library_path +
               " Ensure you called MultiLibraryClassLoader::loadLibrary()");
     }
-    return loader->createUniqueInstance<Base>(class_name);
+    return loader->createUniqueInstance<Base>(class_name, true);
   }
 
   /**
@@ -193,7 +193,7 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader
   template<class Base>
   Base * createUnmanagedInstance(const std::string & class_name)
   {
-    ClassLoader * loader = getClassLoaderForClass<Base>(class_name);
+    std::shared_ptr<class_loader::ClassLoader> loader = getClassLoaderForClass<Base>(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<class Base>
   Base * createUnmanagedInstance(const std::string & class_name, const std::string & library_path)
   {
-    ClassLoader * loader = getClassLoaderForLibrary(library_path);
+    std::shared_ptr<class_loader::ClassLoader> 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<class Base>
   std::vector<std::string> getAvailableClassesForLibrary(const std::string & library_path)
   {
-    ClassLoader * loader = getClassLoaderForLibrary(library_path);
+    std::shared_ptr<class_loader::ClassLoader> loader = getClassLoaderForLibrary(library_path);
     if (nullptr == loader) {
       throw class_loader::NoClassLoaderExistsException(
               "There is no ClassLoader in MultiLibraryClassLoader bound to library " +
@@ -321,15 +321,15 @@ 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<class_loader::ClassLoader> getClassLoaderForLibrary(const std::string & library_path);
 
   /// Gets a handle to the class loader corresponding to a specific class.
   /**
    * @param class_name name of class for which we want to create instance.
    * @return A pointer to the ClassLoader, or NULL if not found.
    */
   template<typename Base>
-  ClassLoader * getClassLoaderForClass(const std::string & class_name)
+  std::shared_ptr<class_loader::ClassLoader> getClassLoaderForClass(const std::string & class_name)
   {
     ClassLoaderVector loaders = getAllAvailableClassLoaders();
     for (ClassLoaderVector::iterator i = loaders.begin(); i != loaders.end(); ++i) {

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(

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_;
 }
 

src/multi_library_class_loader.cpp

@@ -68,7 +68,8 @@ std::vector<std::string> MultiLibraryClassLoader::getRegisteredLibraries() const
   return libraries;
 }
 
-ClassLoader * MultiLibraryClassLoader::getClassLoaderForLibrary(const std::string & library_path)
+std::shared_ptr<class_loader::ClassLoader> 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());
+      std::make_shared<class_loader::ClassLoader>(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);
+    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;