Skip to content

manual port of unique ptr pr #58

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 31, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 106 additions & 33 deletions include/class_loader/class_loader.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking at the upstream PR, some comment content has not been propagated:

     * Creating an unmanaged instance disables dynamically unloading libraries when
     * managed pointers go out of scope for all class loaders in this process.

*
* 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);
}

/**
Expand Down Expand Up @@ -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
Expand Down
118 changes: 91 additions & 27 deletions include/class_loader/multi_library_class_loader.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand All @@ -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
Expand All @@ -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);
}

/**
Expand All @@ -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);
}
Expand Down Expand Up @@ -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
*/
Expand Down
5 changes: 5 additions & 0 deletions src/class_loader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
21 changes: 21 additions & 0 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
1 change: 1 addition & 0 deletions test/base.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
class Base
{
public:
virtual ~Base() {}
virtual void saySomething() = 0;
};

Expand Down
Loading