Skip to content

Commit

Permalink
Improve memory management
Browse files Browse the repository at this point in the history
Signed-off-by: ahcorde <ahcorde@gmail.com>
  • Loading branch information
ahcorde committed Jun 25, 2021
1 parent 471c398 commit 0969655
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 29 deletions.
38 changes: 27 additions & 11 deletions include/class_loader/class_loader.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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).
Expand All @@ -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).
Expand Down
28 changes: 14 additions & 14 deletions include/class_loader/multi_library_class_loader.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 " +
Expand All @@ -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);
}

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

/**
Expand All @@ -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);
Expand All @@ -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 "
Expand Down Expand Up @@ -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 " +
Expand Down Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions src/class_loader_core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
3 changes: 3 additions & 0 deletions src/meta_object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
}

Expand Down
8 changes: 4 additions & 4 deletions src/multi_library_class_loader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
Expand All @@ -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());
}
}

Expand All @@ -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;
Expand Down

0 comments on commit 0969655

Please sign in to comment.