From 158eef4e7df5867d44a0c1ae7c614f44cb81f881 Mon Sep 17 00:00:00 2001 From: Mikhail Nosov Date: Tue, 1 Mar 2022 19:22:49 +0300 Subject: [PATCH 1/3] First quick fix Details: - FrontEnd::add_extension(const std::vector>& extensions Here just loop for extensions and call add_extension. No need to use 'm_actual', as vector of extensions will be stored in 'parent' FrontEnd - Same for void FrontEnd::add_extension(const std::string& library_path) Just load extensions and call 'add_extension'. No m_actual is involved In this case, in 'convert' phase - everything will be fine and visible --- .../src/os/lin/lin_shared_object_loader.cpp | 5 ++- src/core/include/openvino/core/model.hpp | 3 +- .../include/openvino/frontend/frontend.hpp | 2 +- src/frontends/common/src/frontend.cpp | 39 +++++++++++-------- 4 files changed, 28 insertions(+), 21 deletions(-) diff --git a/src/common/util/src/os/lin/lin_shared_object_loader.cpp b/src/common/util/src/os/lin/lin_shared_object_loader.cpp index 908b38b0147452..e963b13523e9f6 100644 --- a/src/common/util/src/os/lin/lin_shared_object_loader.cpp +++ b/src/common/util/src/os/lin/lin_shared_object_loader.cpp @@ -13,9 +13,10 @@ namespace ov { namespace util { std::shared_ptr load_shared_object(const char* path) { - auto shared_object = std::shared_ptr{dlopen(path, RTLD_NOW), [&path](void* shared_object) { + std::string p(path); + auto shared_object = std::shared_ptr{dlopen(path, RTLD_NOW), [p](void* shared_object) { if (shared_object != nullptr) { - std::cout << path << " is closed \n"; + std::cout << p << " is closed \n"; if (0 != dlclose(shared_object)) { std::cerr << "dlclose failed"; if (auto error = dlerror()) { diff --git a/src/core/include/openvino/core/model.hpp b/src/core/include/openvino/core/model.hpp index a0cb7c40ea5fd7..e0aaa90a06b97c 100644 --- a/src/core/include/openvino/core/model.hpp +++ b/src/core/include/openvino/core/model.hpp @@ -13,6 +13,7 @@ #include #include "openvino/core/core_visibility.hpp" +#include "openvino/core/extension.hpp" #include "openvino/core/node.hpp" #include "openvino/core/rtti.hpp" #include "openvino/op/assign.hpp" @@ -40,7 +41,7 @@ class OPENVINO_API Model : public std::enable_shared_from_this { friend OPENVINO_API std::shared_ptr clone_model(const Model& func, std::unordered_map>& node_map); std::shared_ptr m_shared_object; // Frontend plugin shared object handle. - std::vector> m_extension_shared_objects = {}; + std::vector> m_extension_shared_objects = {}; public: static const ::ov::DiscreteTypeInfo& get_type_info_static() { diff --git a/src/frontends/common/include/openvino/frontend/frontend.hpp b/src/frontends/common/include/openvino/frontend/frontend.hpp index 0561d53d7b0521..f96114d546659f 100644 --- a/src/frontends/common/include/openvino/frontend/frontend.hpp +++ b/src/frontends/common/include/openvino/frontend/frontend.hpp @@ -23,7 +23,7 @@ class FRONTEND_API FrontEnd { friend class FrontEndManager; std::shared_ptr m_shared_object = {}; // Library handle - /*static*/ std::vector> m_extension_shared_objects; + std::vector> m_loaded_extensions; std::shared_ptr m_actual = {}; public: diff --git a/src/frontends/common/src/frontend.cpp b/src/frontends/common/src/frontend.cpp index 6eeee88c59d1f6..f80a92bb286f68 100644 --- a/src/frontends/common/src/frontend.cpp +++ b/src/frontends/common/src/frontend.cpp @@ -30,7 +30,11 @@ std::shared_ptr FrontEnd::create_copy(const std::shared_ptr& variants) const { if (m_actual) { @@ -49,9 +53,9 @@ InputModel::Ptr FrontEnd::load_impl(const std::vector& variants) const std::shared_ptr FrontEnd::convert(const InputModel::Ptr& model) const { FRONT_END_CHECK_IMPLEMENTED(m_actual, convert); - std::cout << "this address: " << this << " and size: " << m_extension_shared_objects.size() << " during convert call"<< "\n"; +// std::cout << "this address: " << this << " and size: " << m_extension_shared_objects.size() << " during convert call"<< "\n"; auto converted_model = FrontEnd::create_copy(m_actual->convert(model->m_actual), m_shared_object); - for(const auto& ext : m_extension_shared_objects) { + for(const auto& ext : m_loaded_extensions) { std::cout << "assign shared ptr during convert \n"; converted_model->m_extension_shared_objects.push_back(ext); } @@ -80,7 +84,7 @@ void FrontEnd::normalize(const std::shared_ptr& model) const { void FrontEnd::add_extension(const std::shared_ptr& extension) { std::cout << "added extension shared ptr 1 \n"; - m_extension_shared_objects.push_back(extension); + m_loaded_extensions.push_back(extension); if (m_actual) { m_actual->add_extension(extension); return; @@ -92,24 +96,25 @@ void FrontEnd::add_extension(const std::shared_ptr& extension) { //std::vector> FrontEnd::m_extension_shared_objects = {}; static test void FrontEnd::add_extension(const std::vector>& extensions) { - for (const auto& ext : extensions) { - std::cout << "added extension to FrontEnd with address: " << this << "\n"; - m_extension_shared_objects.push_back(ext); - } - if (m_actual) { - m_actual->add_extension(extensions); - return; - } +// for (const auto& ext : extensions) { + std::cout << "added extension to FrontEnd with address: " << this << "\n"; +// m_extension_shared_objects.push_back(ext); +// } +// if (m_actual) { +// m_actual->add_extension(extensions); +// return; +// } for (const auto& ext : extensions) add_extension(ext); } void FrontEnd::add_extension(const std::string& library_path) { - if (m_actual) { - m_actual->add_extension(library_path); - return; - } - add_extension(ov::detail::load_extensions(library_path)); + auto extensions_lib = ov::detail::load_extensions(library_path); + add_extension(extensions_lib); +// if (m_actual) { +// m_actual->add_extension(library_path); +// return; +// } } #ifdef OPENVINO_ENABLE_UNICODE_PATH_SUPPORT From ac285b0f7a9b14c8394544862bd1310ba3a6cfb4 Mon Sep 17 00:00:00 2001 From: Mikhail Nosov Date: Tue, 1 Mar 2022 19:52:16 +0300 Subject: [PATCH 2/3] This is how I proposed to encapsulate this 'extension holder' logic inside 'm_shared_object' So that implementation details will be hidden from public headers --- src/core/include/openvino/core/model.hpp | 1 - .../include/openvino/frontend/frontend.hpp | 1 - src/frontends/common/src/frontend.cpp | 9 +++------ src/frontends/common/src/manager.cpp | 8 ++++---- src/frontends/common/src/plugin_loader.hpp | 19 +++++++++++++++++++ 5 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/core/include/openvino/core/model.hpp b/src/core/include/openvino/core/model.hpp index e0aaa90a06b97c..f1789f04ea267a 100644 --- a/src/core/include/openvino/core/model.hpp +++ b/src/core/include/openvino/core/model.hpp @@ -41,7 +41,6 @@ class OPENVINO_API Model : public std::enable_shared_from_this { friend OPENVINO_API std::shared_ptr clone_model(const Model& func, std::unordered_map>& node_map); std::shared_ptr m_shared_object; // Frontend plugin shared object handle. - std::vector> m_extension_shared_objects = {}; public: static const ::ov::DiscreteTypeInfo& get_type_info_static() { diff --git a/src/frontends/common/include/openvino/frontend/frontend.hpp b/src/frontends/common/include/openvino/frontend/frontend.hpp index f96114d546659f..f406be355bd99d 100644 --- a/src/frontends/common/include/openvino/frontend/frontend.hpp +++ b/src/frontends/common/include/openvino/frontend/frontend.hpp @@ -23,7 +23,6 @@ class FRONTEND_API FrontEnd { friend class FrontEndManager; std::shared_ptr m_shared_object = {}; // Library handle - std::vector> m_loaded_extensions; std::shared_ptr m_actual = {}; public: diff --git a/src/frontends/common/src/frontend.cpp b/src/frontends/common/src/frontend.cpp index f80a92bb286f68..72ffbf0d73efe1 100644 --- a/src/frontends/common/src/frontend.cpp +++ b/src/frontends/common/src/frontend.cpp @@ -11,6 +11,7 @@ #include "openvino/frontend/place.hpp" #include "so_extension.hpp" #include "utils.hpp" +#include "plugin_loader.hpp" using namespace ov; using namespace ov::frontend; @@ -53,12 +54,7 @@ InputModel::Ptr FrontEnd::load_impl(const std::vector& variants) const std::shared_ptr FrontEnd::convert(const InputModel::Ptr& model) const { FRONT_END_CHECK_IMPLEMENTED(m_actual, convert); -// std::cout << "this address: " << this << " and size: " << m_extension_shared_objects.size() << " during convert call"<< "\n"; auto converted_model = FrontEnd::create_copy(m_actual->convert(model->m_actual), m_shared_object); - for(const auto& ext : m_loaded_extensions) { - std::cout << "assign shared ptr during convert \n"; - converted_model->m_extension_shared_objects.push_back(ext); - } return converted_model; } @@ -84,7 +80,8 @@ void FrontEnd::normalize(const std::shared_ptr& model) const { void FrontEnd::add_extension(const std::shared_ptr& extension) { std::cout << "added extension shared ptr 1 \n"; - m_loaded_extensions.push_back(extension); + + add_extension_to_shared_data(m_shared_object, extension); if (m_actual) { m_actual->add_extension(extension); return; diff --git a/src/frontends/common/src/manager.cpp b/src/frontends/common/src/manager.cpp index 1984d78556047b..96db14c3cfafbe 100644 --- a/src/frontends/common/src/manager.cpp +++ b/src/frontends/common/src/manager.cpp @@ -45,7 +45,7 @@ class FrontEndManager::Impl { if (plugin_it != m_plugins.end()) { if (plugin_it->load()) { auto fe_obj = std::make_shared(); - fe_obj->m_shared_object = plugin_it->get_so_pointer(); + fe_obj->m_shared_object = std::make_shared(plugin_it->get_so_pointer()); fe_obj->m_actual = plugin_it->get_creator().m_creator(); return fe_obj; } @@ -56,7 +56,7 @@ class FrontEndManager::Impl { OPENVINO_ASSERT(plugin.load(), "Cannot load frontend ", plugin.get_name_from_file()); if (plugin.get_creator().m_name == framework) { auto fe_obj = std::make_shared(); - fe_obj->m_shared_object = plugin.get_so_pointer(); + fe_obj->m_shared_object = std::make_shared(plugin.get_so_pointer()); fe_obj->m_actual = plugin.get_creator().m_creator(); return fe_obj; } @@ -94,7 +94,7 @@ class FrontEndManager::Impl { if (fe->supported(variants)) { auto fe_obj = std::make_shared(); std::cout << "frontend created with address: " << static_cast(fe_obj.get()) << "\n"; - fe_obj->m_shared_object = plugin.get_so_pointer(); + fe_obj->m_shared_object = std::make_shared(plugin.get_so_pointer()); fe_obj->m_actual = fe; return fe_obj; } @@ -186,7 +186,7 @@ class FrontEndManager::Impl { if (fe && fe->supported(variants)) { // Priority FE (e.g. IR) is found and is suitable auto fe_obj = std::make_shared(); - fe_obj->m_shared_object = plugin_info.get_so_pointer(); + fe_obj->m_shared_object = std::make_shared(plugin_info.get_so_pointer()); fe_obj->m_actual = fe; return fe_obj; } diff --git a/src/frontends/common/src/plugin_loader.hpp b/src/frontends/common/src/plugin_loader.hpp index 0e48b8f9ade17c..934d2d4f3f6f5d 100644 --- a/src/frontends/common/src/plugin_loader.hpp +++ b/src/frontends/common/src/plugin_loader.hpp @@ -15,6 +15,25 @@ static const char PathSeparator[] = ":"; namespace ov { namespace frontend { +/// \brief Internal data structure holding by each frontend. Includes library handle and extensions. +class FrontEndSharedData { + std::shared_ptr m_so; + std::vector> m_loaded_extensions = {}; +public: + explicit FrontEndSharedData(const std::shared_ptr& so): m_so(so) { + std::cout << "FrontEndSharedData constructor\n"; + } + ~FrontEndSharedData() { + std::cout << "~FrontEndSharedData: " << m_loaded_extensions.size() << "\n"; + } + std::vector>& extensions() { return m_loaded_extensions; } +}; + +inline void add_extension_to_shared_data(std::shared_ptr& obj, const std::shared_ptr& ext) { + auto obj_data = std::static_pointer_cast(obj); + obj_data->extensions().push_back(ext); +} + /// \brief Internal data structure holding plugin information including library handle, file names and paths, etc. class PluginInfo { std::shared_ptr m_so; // Library shared object, must be first data member to be destroyed last From 4581c3f8c7a787fdde53cd5ab3b7bfd681fd7596 Mon Sep 17 00:00:00 2001 From: Mikhail Nosov Date: Tue, 1 Mar 2022 20:03:51 +0300 Subject: [PATCH 3/3] Small correction for root 'add_extension'. Otherwise if 'child' frontend calls base's add_extension - nullptr dereferencing can occur --- src/frontends/common/src/frontend.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/frontends/common/src/frontend.cpp b/src/frontends/common/src/frontend.cpp index 72ffbf0d73efe1..e80229b368de11 100644 --- a/src/frontends/common/src/frontend.cpp +++ b/src/frontends/common/src/frontend.cpp @@ -79,10 +79,8 @@ void FrontEnd::normalize(const std::shared_ptr& model) const { } void FrontEnd::add_extension(const std::shared_ptr& extension) { - std::cout << "added extension shared ptr 1 \n"; - - add_extension_to_shared_data(m_shared_object, extension); if (m_actual) { + add_extension_to_shared_data(m_shared_object, extension); m_actual->add_extension(extension); return; }