diff --git a/include/class_loader/multi_library_class_loader.hpp b/include/class_loader/multi_library_class_loader.hpp index d37cdd9..f04afc8 100644 --- a/include/class_loader/multi_library_class_loader.hpp +++ b/include/class_loader/multi_library_class_loader.hpp @@ -56,7 +56,7 @@ namespace class_loader { typedef std::string LibraryPath; -typedef std::map> LibraryToClassLoaderMap; +typedef std::map LibraryToClassLoaderMap; typedef std::vector ClassLoaderVector; class MultiLibraryClassLoaderImpl; @@ -355,7 +355,7 @@ class CLASS_LOADER_PUBLIC MultiLibraryClassLoader */ void shutdownAllClassLoaders(); - bool enable_ondemand_loadunload_; + MultiLibraryClassLoaderImpl * impl_; }; diff --git a/src/class_loader_core.cpp b/src/class_loader_core.cpp index e5458a5..632f8c2 100644 --- a/src/class_loader_core.cpp +++ b/src/class_loader_core.cpp @@ -521,9 +521,12 @@ void unloadLibrary(const std::string & library_path, ClassLoader * loader) throw class_loader::LibraryUnloadException( "Could not unload library (rcpputils exception = " + std::string(e.what()) + ")"); } + } else { + CONSOLE_BRIDGE_logDebug( + "class_loader.impl: " + "Attempt to unload library %s that class_loader is unaware of or is already unloaded", + library_path.c_str()); } - throw class_loader::LibraryUnloadException( - "Attempt to unload library that class_loader is unaware of."); } } diff --git a/src/multi_library_class_loader.cpp b/src/multi_library_class_loader.cpp index b98acc0..ccf3743 100644 --- a/src/multi_library_class_loader.cpp +++ b/src/multi_library_class_loader.cpp @@ -37,6 +37,8 @@ namespace class_loader { +typedef std::vector> ClassLoaderPtrVector; + class ClassLoaderDependency { protected: @@ -54,34 +56,43 @@ class ClassLoaderDependency } }; -class MultiLibraryClassLoaderImpl : public ClassLoaderDependency +class ClassLoaderPtrVectorImpl : public ClassLoaderDependency { public: - LibraryToClassLoaderMap active_class_loaders_; + ClassLoaderPtrVector class_loader_ptrs_; std::recursive_mutex loader_mutex_; }; -MultiLibraryClassLoaderImpl & getMultiLibraryClassLoaderImpl() +class MultiLibraryClassLoaderImpl +{ +public: + bool enable_ondemand_loadunload_; + LibraryToClassLoaderMap active_class_loaders_; + std::mutex loader_mutex_; +}; + +ClassLoaderPtrVectorImpl & getClassLoaderPtrVectorImpl() { - static MultiLibraryClassLoaderImpl instance; + static ClassLoaderPtrVectorImpl instance; return instance; } MultiLibraryClassLoader::MultiLibraryClassLoader(bool enable_ondemand_loadunload) -: enable_ondemand_loadunload_(enable_ondemand_loadunload) +: impl_(new MultiLibraryClassLoaderImpl()) { + impl_->enable_ondemand_loadunload_ = enable_ondemand_loadunload; } MultiLibraryClassLoader::~MultiLibraryClassLoader() { shutdownAllClassLoaders(); + delete impl_; } std::vector MultiLibraryClassLoader::getRegisteredLibraries() const { std::vector libraries; - std::lock_guard lock(getMultiLibraryClassLoaderImpl().loader_mutex_); - for (auto & it : getMultiLibraryClassLoaderImpl().active_class_loaders_) { + for (auto & it : impl_->active_class_loaders_) { if (it.second != nullptr) { libraries.push_back(it.first); } @@ -91,16 +102,14 @@ std::vector MultiLibraryClassLoader::getRegisteredLibraries() const ClassLoader * MultiLibraryClassLoader::getClassLoaderForLibrary(const std::string & library_path) { - std::lock_guard lock(getMultiLibraryClassLoaderImpl().loader_mutex_); - return getMultiLibraryClassLoaderImpl().active_class_loaders_[library_path].get(); + return impl_->active_class_loaders_[library_path]; } ClassLoaderVector MultiLibraryClassLoader::getAllAvailableClassLoaders() const { ClassLoaderVector loaders; - std::lock_guard lock(getMultiLibraryClassLoaderImpl().loader_mutex_); - for (auto & it : getMultiLibraryClassLoaderImpl().active_class_loaders_) { - loaders.push_back(it.second.get()); + for (auto & it : impl_->active_class_loaders_) { + loaders.push_back(it.second); } return loaders; } @@ -114,16 +123,18 @@ bool MultiLibraryClassLoader::isLibraryAvailable(const std::string & library_nam void MultiLibraryClassLoader::loadLibrary(const std::string & library_path) { - std::lock_guard lock(getMultiLibraryClassLoaderImpl().loader_mutex_); if (!isLibraryAvailable(library_path)) { - getMultiLibraryClassLoaderImpl().active_class_loaders_[library_path] = - std::make_shared(library_path, isOnDemandLoadUnloadEnabled()); + std::lock_guard lock(getClassLoaderPtrVectorImpl().loader_mutex_); + getClassLoaderPtrVectorImpl().class_loader_ptrs_.emplace_back( + std::make_shared(library_path, isOnDemandLoadUnloadEnabled()) + ); + impl_->active_class_loaders_[library_path] = + getClassLoaderPtrVectorImpl().class_loader_ptrs_.back().get(); } } void MultiLibraryClassLoader::shutdownAllClassLoaders() { - std::lock_guard lock(getMultiLibraryClassLoaderImpl().loader_mutex_); for (auto & library_path : getRegisteredLibraries()) { unloadLibrary(library_path); } @@ -131,13 +142,20 @@ void MultiLibraryClassLoader::shutdownAllClassLoaders() int MultiLibraryClassLoader::unloadLibrary(const std::string & library_path) { - std::lock_guard lock(getMultiLibraryClassLoaderImpl().loader_mutex_); int remaining_unloads = 0; if (isLibraryAvailable(library_path)) { ClassLoader * loader = getClassLoaderForLibrary(library_path); remaining_unloads = loader->unloadLibrary(); if (remaining_unloads == 0) { - getMultiLibraryClassLoaderImpl().active_class_loaders_[library_path] = nullptr; + impl_->active_class_loaders_[library_path] = nullptr; + std::lock_guard lock(getClassLoaderPtrVectorImpl().loader_mutex_); + auto & class_loader_ptrs = getClassLoaderPtrVectorImpl().class_loader_ptrs_; + for (auto iter = class_loader_ptrs.begin(); iter != class_loader_ptrs.end(); ++iter) { + if (iter->get() == loader) { + class_loader_ptrs.erase(iter); + break; + } + } } } return remaining_unloads; @@ -145,7 +163,7 @@ int MultiLibraryClassLoader::unloadLibrary(const std::string & library_path) bool MultiLibraryClassLoader::isOnDemandLoadUnloadEnabled() const { - return enable_ondemand_loadunload_; + return impl_->enable_ondemand_loadunload_; } } // namespace class_loader diff --git a/test/unique_ptr_test.cpp b/test/unique_ptr_test.cpp index 35d13c6..55a8172 100644 --- a/test/unique_ptr_test.cpp +++ b/test/unique_ptr_test.cpp @@ -94,9 +94,6 @@ TEST(ClassLoaderUniquePtrTest, basicLoadFailures) { EXPECT_THROW( class_loader::impl::loadLibrary("LIBRARY_1", &loader1), class_loader::LibraryLoadException); - EXPECT_THROW( - class_loader::impl::unloadLibrary("LIBRARY_1", &loader1), - class_loader::LibraryUnloadException); } TEST(ClassLoaderUniquePtrTest, MultiLibraryClassLoaderFailures) { diff --git a/test/utest.cpp b/test/utest.cpp index b80817a..f83296a 100644 --- a/test/utest.cpp +++ b/test/utest.cpp @@ -121,9 +121,6 @@ TEST(ClassLoaderUniquePtrTest, basicLoadFailures) { EXPECT_THROW( class_loader::impl::loadLibrary("LIBRARY_1", &loader1), class_loader::LibraryLoadException); - EXPECT_THROW( - class_loader::impl::unloadLibrary("LIBRARY_1", &loader1), - class_loader::LibraryUnloadException); } TEST(ClassLoaderUniquePtrTest, MultiLibraryClassLoaderFailures) {