manual port of unique ptr pr (#58)

wjwwood · mikaelarguedas · commit adcb526f86a6 · 2017-10-31T17:05:43.000-06:00
* manual port of #38 * add static private setter function for unmanaged flag * missing newline * use NULL unless within a "if C++11" block * fixups
diff --git a/include/class_loader/class_loader.h b/include/class_loader/class_loader.h
@@ -75,6 +75,14 @@ std::string systemLibraryFormat(const std::string & library_name);
 class ClassLoader
 {
 public:
+#if __cplusplus >= 201103L
+  template<typename Base>
+  using DeleterType = std::function<void (Base *)>;
+
+  template<typename Base>
+  using UniquePtr = std::unique_ptr<Base, DeleterType<Base>>;
+#endif
+
   /**
    * @brief  Constructor for ClassLoader
    * @param library_path - The path of the runtime library to load
@@ -100,54 +108,66 @@ class ClassLoader
   }
 
   /**
-   * @brief  Generates an instance of loadable classes (i.e. class_loader). It is not necessary for the user to call loadLibrary() as it will be invoked automatically if the library is not yet loaded (which typically happens when in "On Demand Load/Unload" mode).
+   * @brief  Generates an instance of loadable classes (i.e. class_loader).
+   *
+   * It is not necessary for the user to call loadLibrary() as it will be invoked automatically
+   * if the library is not yet loaded (which typically happens when in "On Demand Load/Unload" mode).
+   *
    * @param  derived_class_name The name of the class we want to create (@see getAvailableClasses())
    * @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)
   {
-    if (ClassLoader::hasUnmanagedInstanceBeenCreated() && isOnDemandLoadUnloadEnabled()) {
-      CONSOLE_BRIDGE_logInform(
-        "class_loader::ClassLoader: "
-        "An attempt is being made to create a managed plugin instance (i.e. std::shared_ptr), "
-        "however an unmanaged instance was created within this process address space. "
-        "This means libraries for the managed instances will not be shutdown automatically on "
-        "final plugin destruction if on demand (lazy) loading/unloading mode is used.");
-      }
-
-    if (!isLibraryLoaded()) {
-      loadLibrary();
-    }
-
-    Base * obj = class_loader::impl::createInstance<Base>(derived_class_name, this);
-    assert(obj != nullptr);  // Unreachable assertion if createInstance() throws on failure
-
-    std::lock_guard<std::recursive_mutex> lock(plugin_ref_count_mutex_);
-    ++plugin_ref_count_;
+    return std::shared_ptr<Base>(
+      createRawInstance<Base>(derived_class_name, true),
+      std::bind(&ClassLoader::onPluginDeletion<Base>, this, std::placeholders::_1)
+    );
+  }
 
-    std::shared_ptr<Base> smart_obj(
-      obj, std::bind(&class_loader::ClassLoader::onPluginDeletion<Base>, this, std::placeholders::_1));
-    return smart_obj;
+#if __cplusplus >= 201103L
+  /// Generates an instance of loadable classes (i.e. class_loader).
+  /**
+   * It is not necessary for the user to call loadLibrary() as it will be
+   * invoked automatically if the library is not yet loaded (which typically
+   * happens when in "On Demand Load/Unload" mode).
+   *
+   * If you release the wrapped pointer you must manually call the original
+   * deleter when you want to destroy the released pointer.
+   *
+   * @param derived_class_name
+   *   The name of the class we want to create (@see getAvailableClasses()).
+   * @return A std::unique_ptr<Base> to newly created plugin object.
+   */
+  template<class Base>
+  UniquePtr<Base> createUniqueInstance(const std::string& derived_class_name)
+  {
+    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)
+    );
   }
+#endif
 
+  /// Generates an instance of loadable classes (i.e. class_loader).
   /**
-   * @brief  Generates an instance of loadable classes (i.e. class_loader). It is not necessary for the user to call loadLibrary() as it will be invoked automatically if the library is not yet loaded (which typically happens when in "On Demand Load/Unload" mode).
-   * @param derived_class_name The name of the class we want to create (@see getAvailableClasses())
+   * It is not necessary for the user to call loadLibrary() as it will be
+   * invoked automatically if the library is not yet loaded (which typically
+   * happens when in "On Demand Load/Unload" mode).
+   *
+   * Creating an unmanaged instance disables dynamically unloading libraries
+   * when managed pointers go out of scope for all class loaders in this
+   * process.
+   *
+   * @param derived_class_name
+   *   The name of the class we want to create (@see getAvailableClasses()).
    * @return An unmanaged (i.e. not a shared_ptr) Base* to newly created plugin object.
    */
   template <class Base>
   Base * createUnmanagedInstance(const std::string & derived_class_name)
   {
-    has_unmananged_instance_been_created_ = true;
-    if (!isLibraryLoaded()) {
-      loadLibrary();
-    }
-
-    Base * obj = class_loader::impl::createInstance<Base>(derived_class_name, this);
-    assert(obj != nullptr);  // Unreachable assertion if createInstance() throws on failure
-
-    return obj;
+    return createRawInstance<Base>(derived_class_name, false);
   }
 
   /**
@@ -236,12 +256,65 @@ class ClassLoader
     }
   }
 
+  /// Generates an instance of loadable classes (i.e. class_loader).
+  /**
+   * It is not necessary for the user to call loadLibrary() as it will be
+   * invoked automatically if the library is not yet loaded (which typically
+   * happens when in "On Demand Load/Unload" mode).
+   *
+   * @param derived_class_name
+   *   The name of the class we want to create (@see getAvailableClasses()).
+   * @param managed
+   *   If true, the returned pointer is assumed to be wrapped in a smart
+   *   pointer by the caller.
+   * @return A Base* to newly created plugin object.
+   */
+  template <class Base>
+  Base* createRawInstance(const std::string& derived_class_name, bool managed)
+  {
+    if (!managed) {
+      this->setUnmanagedInstanceBeenCreated(true);
+    }
+
+    if (
+      managed &&
+      ClassLoader::hasUnmanagedInstanceBeenCreated() &&
+      isOnDemandLoadUnloadEnabled())
+    {
+      CONSOLE_BRIDGE_logInform(
+        "class_loader::ClassLoader: "
+        "An attempt is being made to create a managed plugin instance (i.e. boost::shared_ptr), "
+        "however an unmanaged instance was created within this process address space. "
+        "This means libraries for the managed instances will not be shutdown automatically on "
+        "final plugin destruction if on demand (lazy) loading/unloading mode is used."
+      );
+    }
+
+    if (!isLibraryLoaded()) {
+      loadLibrary();
+    }
+
+    Base* obj = class_loader::impl::createInstance<Base>(derived_class_name, this);
+    assert(obj != NULL);  // Unreachable assertion if createInstance() throws on failure.
+
+    if (managed)
+    {
+      std::lock_guard<std::recursive_mutex> lock(plugin_ref_count_mutex_);
+      ++plugin_ref_count_;
+    }
+
+    return obj;
+  }
+
   /**
    * @brief Getter for if an unmanaged (i.e. unsafe) instance has been created flag
    */
   CLASS_LOADER_PUBLIC
   static bool hasUnmanagedInstanceBeenCreated();
 
+  CLASS_LOADER_PUBLIC
+  static void setUnmanagedInstanceBeenCreated(bool state);
+
   /**
    * @brief As the library may be unloaded in "on-demand load/unload" mode, unload maybe called from createInstance(). The problem is that createInstance() locks the plugin_ref_count as does unloadLibrary(). This method is the implementation of unloadLibrary but with a parameter to decide if plugin_ref_mutex_ should be locked
    * @param lock_plugin_ref_count - Set to true if plugin_ref_count_mutex_ should be locked, else false
diff --git a/include/class_loader/multi_library_class_loader.h b/include/class_loader/multi_library_class_loader.h
@@ -72,20 +72,17 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader
   template <class Base>
   std::shared_ptr<Base> createInstance(const std::string & class_name)
   {
-    CONSOLE_BRIDGE_logDebug(
-      "class_loader::MultiLibraryClassLoader: "
-      "Attempting to create instance of class type %s.", class_name.c_str());
-    for (auto & loader : getAllAvailableClassLoaders()) {
-      if (loader->isClassAvailable<Base>(class_name)) {
-        return loader->createInstance<Base>(class_name);
-      }
+    ClassLoader * loader = getClassLoaderForClass<Base>(class_name);
+
+    if (loader == NULL) {
+      throw class_loader::CreateClassException(
+        "MultiLibraryClassLoader: Could not create object of class type " +
+        class_name +
+        " as no factory exists for it. Make sure that the library exists and "
+        "was explicitly loaded through MultiLibraryClassLoader::loadLibrary()");
     }
 
-    throw class_loader::CreateClassException(
-      "MultiLibraryClassLoader: "
-      "Could not create object of class type '" + class_name + "' as no factory exists for it. "
-      "Make sure that the library exists and was explicitly loaded through "
-      "MultiLibraryClassLoader::loadLibrary()");
+    return loader->createInstance<Base>(class_name);
   }
 
   /**
@@ -100,15 +97,63 @@ 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);
-    if (!loader) {
+    if (loader == NULL) {
       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()");
+        "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);
   }
 
+#if __cplusplus >= 201103L
+  /// Creates an instance of an object of given class name with ancestor class Base
+  /**
+   * This version does not look in a specific library for the factory, but rather the first open library that defines the classs
+   * @param Base - polymorphic type indicating base class
+   * @param class_name - the name of the concrete plugin class we want to instantiate
+   * @return A unique pointer to newly created plugin
+   */
+  template <class Base>
+  ClassLoader::UniquePtr<Base> createUniqueInstance(const std::string& class_name)
+  {
+    CONSOLE_BRIDGE_logDebug(
+      "class_loader::MultiLibraryClassLoader: Attempting to create instance of class type %s.",
+      class_name.c_str());
+    ClassLoader * loader = getClassLoaderForClass<Base>(class_name);
+    if (loader == nullptr) {
+      throw class_loader::CreateClassException(
+        "MultiLibraryClassLoader: Could not create object of class type " + class_name +
+        " as no factory exists for it. "
+        "Make sure that the library exists and was explicitly loaded through "
+        "MultiLibraryClassLoader::loadLibrary()");
+    }
+    return loader->createUniqueInstance<Base>(class_name);
+  }
+
+  /// Creates an instance of an object of given class name with ancestor class Base
+  /**
+   * This version takes a specific library to make explicit the factory being used
+   * @param Base - polymorphic type indicating base class
+   * @param class_name - the name of the concrete plugin class we want to instantiate
+   * @param library_path - the library from which we want to create the plugin
+   * @return A unique pointer to newly created plugin
+   */
+  template <class Base>
+  ClassLoader::UniquePtr<Base>
+  createUniqueInstance(const std::string& class_name, const std::string& library_path)
+  {
+    ClassLoader * loader = getClassLoaderForLibrary(library_path);
+    if (loader == nullptr) {
+      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);
+  }
+#endif
+
   /**
    * @brief Creates an instance of an object of given class name with ancestor class Base
    * This version does not look in a specific library for the factory, but rather the first open library that defines the classs
@@ -120,14 +165,12 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader
   template <class Base>
   Base* createUnmanagedInstance(const std::string & class_name)
   {
-    for (auto & loader : getAllAvailableClassLoaders()) {
-      if (loader->isClassAvailable<Base>(class_name)) {
-        return loader->createUnmanagedInstance<Base>(class_name);
-      }
+    ClassLoader * loader = getClassLoaderForClass<Base>(class_name);
+    if (loader == NULL) {
+      throw class_loader::CreateClassException(
+        "MultiLibraryClassLoader: Could not create class of type " + class_name);
     }
-
-    throw class_loader::CreateClassException(
-      "MultiLibraryClassLoader: Could not create class of type '" + class_name + "'");
+    return loader->createUnmanagedInstance<Base>(class_name);
   }
 
   /**
@@ -144,9 +187,9 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader
     ClassLoader * loader = getClassLoaderForLibrary(library_path);
     if (!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()");
+        "Could not create instance as there is no ClassLoader in "
+        "MultiLibraryClassLoader bound to library " + library_path +
+        " Ensure you called MultiLibraryClassLoader::loadLibrary()");
     }
     return loader->createUnmanagedInstance<Base>(class_name);
   }
@@ -231,10 +274,31 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader
   /**
    * @brief Gets a handle to the class loader corresponding to a specific runtime library
    * @param library_path - the library from which we want to create the plugin
-   * @return A pointer to the ClassLoader *, == nullptr if not found
+   * @return A pointer to the ClassLoader *, == NULL if not found
    */
   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)
+  {
+    ClassLoaderVector loaders = getAllAvailableClassLoaders();
+    for (ClassLoaderVector::iterator i = loaders.begin(); i != loaders.end(); ++i)
+    {
+      if (!(*i)->isLibraryLoaded()) {
+        (*i)->loadLibrary();
+      }
+      if ((*i)->isClassAvailable<Base>(class_name)) {
+        return *i;
+      }
+    }
+    return NULL;
+  }
+
   /**
    * @brief Gets all class loaders loaded within scope
    */
diff --git a/src/class_loader.cpp b/src/class_loader.cpp
@@ -41,6 +41,11 @@ bool ClassLoader::hasUnmanagedInstanceBeenCreated()
   return ClassLoader::has_unmananged_instance_been_created_;
 }
 
+void ClassLoader::setUnmanagedInstanceBeenCreated(bool state)
+{
+  ClassLoader::has_unmananged_instance_been_created_ = state;
+}
+
 std::string systemLibraryPrefix()
 {
 #if !defined(WIN32)
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
@@ -25,4 +25,25 @@ if(TARGET ${PROJECT_NAME}_utest)
   add_dependencies(${PROJECT_NAME}_utest ${PROJECT_NAME}_TestPlugins1 ${PROJECT_NAME}_TestPlugins2)
 endif()
 
+include(CheckCXXCompilerFlag)
+CHECK_CXX_COMPILER_FLAG("-std=c++11" COMPILER_SUPPORTS_CXX11)
+if(COMPILER_SUPPORTS_CXX11)
+  catkin_add_gtest(${PROJECT_NAME}_unique_ptr_test unique_ptr_test.cpp)
+  if(TARGET ${PROJECT_NAME}_unique_ptr_test)
+    target_link_libraries(${PROJECT_NAME}_unique_ptr_test
+      ${Boost_LIBRARIES}
+      ${class_loader_LIBRARIES}
+    )
+    set_target_properties(${PROJECT_NAME}_unique_ptr_test
+      PROPERTIES
+        COMPILE_FLAGS -std=c++11
+        LINK_FLAGS -std=c++11
+    )
+    add_dependencies(${PROJECT_NAME}_unique_ptr_test
+      ${PROJECT_NAME}_TestPlugins1
+      ${PROJECT_NAME}_TestPlugins2
+    )
+  endif()
+endif()
+
 add_subdirectory(fviz_case_study)
diff --git a/test/base.h b/test/base.h
@@ -4,6 +4,7 @@
 class Base
 {
 public:
+  virtual ~Base() {}
   virtual void saySomething() = 0;
 };
 
diff --git a/test/unique_ptr_test.cpp b/test/unique_ptr_test.cpp

Original file line number	Diff line number	Diff line change
`@@ -41,6 +41,11 @@ bool ClassLoader::hasUnmanagedInstanceBeenCreated()`
`41`	`41`	`return ClassLoader::has_unmananged_instance_been_created_;`
`42`	`42`	`}`
`43`	`43`
	`44`	`+void ClassLoader::setUnmanagedInstanceBeenCreated(bool state)`
	`45`	`+{`
	`46`	`+ ClassLoader::has_unmananged_instance_been_created_ = state;`
	`47`	`+}`
	`48`	`+`
`44`	`49`	`std::string systemLibraryPrefix()`
`45`	`50`	`{`
`46`	`51`	`#if !defined(WIN32)`
Original file line number	Diff line number	Diff line change
`@@ -4,6 +4,7 @@`
`4`	`4`	`class Base`
`5`	`5`	`{`
`6`	`6`	`public:`
	`7`	`+ virtual ~Base() {}`
`7`	`8`	`virtual void saySomething() = 0;`
`8`	`9`	`};`
`9`	`10`