Cpp fix of python segfault, reverted pybind workaround (#10749)

* test fix of segfault

* styles applied

* added keep_alive to pybind

* remove redundant code

* fix json tests

* review remarks

* introduced correct path to dlls in CI

* removing passing path via env variable

* introduced cpp solution

* remove keep alive

* review remarks

* remove explicit removing model

* removed shared_objects from ir frontend

* core test updated

* unified approach to handle extensions by frontends

* added nullptr check

* Revert "added nullptr check"

This reverts commit 666f5e4489.

* Revert "unified approach to handle extensions by frontends"

This reverts commit bf85ac24a6.

* m_extensions declaration in Frontend

* added assert
This commit is contained in:
Mateusz Bencer 2022-03-18 14:29:46 +01:00 committed by GitHub
parent a5362a4d58
commit fe406d1606
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 32 additions and 38 deletions

View File

@ -38,7 +38,6 @@ void regclass_frontend_FrontEnd(py::module m) {
fem.def("convert",
static_cast<std::shared_ptr<ov::Model> (FrontEnd::*)(const InputModel::Ptr&) const>(&FrontEnd::convert),
py::arg("model"),
py::keep_alive<0, 1>(),
R"(
Completely convert and normalize entire function, throws if it is not possible.
@ -63,7 +62,6 @@ void regclass_frontend_FrontEnd(py::module m) {
fem.def("convert_partially",
&FrontEnd::convert_partially,
py::arg("model"),
py::keep_alive<0, 1>(),
R"(
Convert only those parts of the model that can be converted leaving others as-is.
Converted parts are not normalized by additional transformations; normalize function or
@ -78,7 +76,6 @@ void regclass_frontend_FrontEnd(py::module m) {
fem.def("decode",
&FrontEnd::decode,
py::arg("model"),
py::keep_alive<0, 1>(),
R"(
Convert operations with one-to-one mapping with decoding nodes.
Each decoding node is an nGraph node representing a single FW operation node with

View File

@ -315,10 +315,8 @@ def test_add_extension_template_extension(device):
new_shapes = {"in_data": after_reshape}
assert model.input().partial_shape == before_reshape
model.reshape(new_shapes)
assert model.input().partial_shape == after_reshape
# CVS-74584
del model
compiled = core.compile_model(model, device)
assert compiled.input().partial_shape == after_reshape
def test_add_extension():

View File

@ -148,6 +148,8 @@ protected:
virtual InputModel::Ptr load_impl(const std::vector<ov::Any>& variants) const;
std::vector<ov::Extension::Ptr> m_extensions;
private:
static std::shared_ptr<ov::Model> create_copy(const std::shared_ptr<ov::Model>& ov_model,
const std::shared_ptr<void>& shared_object);

View File

@ -9,6 +9,7 @@
#include "openvino/frontend/extension/op.hpp"
#include "openvino/frontend/manager.hpp"
#include "openvino/frontend/place.hpp"
#include "plugin_loader.hpp"
#include "so_extension.hpp"
#include "utils.hpp"
@ -74,6 +75,7 @@ void FrontEnd::normalize(const std::shared_ptr<Model>& model) const {
void FrontEnd::add_extension(const std::shared_ptr<ov::Extension>& extension) {
if (m_actual) {
add_extension_to_shared_data(m_shared_object, extension);
m_actual->add_extension(extension);
return;
}
@ -82,28 +84,17 @@ void FrontEnd::add_extension(const std::shared_ptr<ov::Extension>& extension) {
}
void FrontEnd::add_extension(const std::vector<std::shared_ptr<ov::Extension>>& extensions) {
if (m_actual) {
m_actual->add_extension(extensions);
return;
}
for (const auto& ext : extensions)
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));
}
#ifdef OPENVINO_ENABLE_UNICODE_PATH_SUPPORT
void FrontEnd::add_extension(const std::wstring& library_path) {
if (m_actual) {
m_actual->add_extension(library_path);
return;
}
add_extension(ov::detail::load_extensions(library_path));
}
#endif

View File

@ -45,7 +45,7 @@ public:
if (plugin_it != m_plugins.end()) {
if (plugin_it->load()) {
auto fe_obj = std::make_shared<FrontEnd>();
fe_obj->m_shared_object = plugin_it->get_so_pointer();
fe_obj->m_shared_object = std::make_shared<FrontEndSharedData>(plugin_it->get_so_pointer());
fe_obj->m_actual = plugin_it->get_creator().m_creator();
return fe_obj;
}
@ -56,7 +56,7 @@ public:
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<FrontEnd>();
fe_obj->m_shared_object = plugin.get_so_pointer();
fe_obj->m_shared_object = std::make_shared<FrontEndSharedData>(plugin.get_so_pointer());
fe_obj->m_actual = plugin.get_creator().m_creator();
return fe_obj;
}
@ -93,7 +93,7 @@ public:
OPENVINO_ASSERT(fe, "Frontend error: frontend '", plugin.get_creator().m_name, "' created null FrontEnd");
if (fe->supported(variants)) {
auto fe_obj = std::make_shared<FrontEnd>();
fe_obj->m_shared_object = plugin.get_so_pointer();
fe_obj->m_shared_object = std::make_shared<FrontEndSharedData>(plugin.get_so_pointer());
fe_obj->m_actual = fe;
return fe_obj;
}
@ -185,7 +185,7 @@ private:
if (fe && fe->supported(variants)) {
// Priority FE (e.g. IR) is found and is suitable
auto fe_obj = std::make_shared<FrontEnd>();
fe_obj->m_shared_object = plugin_info.get_so_pointer();
fe_obj->m_shared_object = std::make_shared<FrontEndSharedData>(plugin_it->get_so_pointer());
fe_obj->m_actual = fe;
return fe_obj;
}

View File

@ -15,6 +15,23 @@ static const char PathSeparator[] = ":";
namespace ov {
namespace frontend {
/// \brief Internal data structure holding by each frontend. Includes library handle and extensions.
class FrontEndSharedData {
friend inline void add_extension_to_shared_data(std::shared_ptr<void>& obj,
const std::shared_ptr<ov::Extension>& ext);
std::shared_ptr<void> m_so;
std::vector<std::shared_ptr<ov::Extension>> m_loaded_extensions = {};
public:
explicit FrontEndSharedData(const std::shared_ptr<void>& so) : m_so(so) {}
};
inline void add_extension_to_shared_data(std::shared_ptr<void>& obj, const std::shared_ptr<ov::Extension>& ext) {
auto obj_data = std::static_pointer_cast<FrontEndSharedData>(obj);
OPENVINO_ASSERT(obj_data, "internal error: not allowed type of shared data used");
obj_data->m_loaded_extensions.push_back(ext);
}
/// \brief Internal data structure holding plugin information including library handle, file names and paths, etc.
class PluginInfo {
std::shared_ptr<void> m_so; // Library shared object, must be first data member to be destroyed last

View File

@ -45,8 +45,6 @@ protected:
InputModel::Ptr load_impl(const std::vector<ov::Any>& params) const override;
private:
std::vector<std::shared_ptr<void>> shared_objects;
std::vector<ov::Extension::Ptr> extensions;
std::shared_ptr<TelemetryExtension> m_telemetry;
};

View File

@ -105,11 +105,10 @@ void FrontEnd::add_extension(const ov::Extension::Ptr& ext) {
m_telemetry = telemetry;
} else if (auto so_ext = std::dynamic_pointer_cast<ov::detail::SOExtension>(ext)) {
if (std::dynamic_pointer_cast<ov::BaseOpExtension>(so_ext->extension())) {
shared_objects.emplace_back(so_ext->shared_object());
extensions.emplace_back(so_ext->extension());
m_extensions.emplace_back(so_ext->extension());
}
} else if (std::dynamic_pointer_cast<ov::BaseOpExtension>(ext))
extensions.emplace_back(ext);
m_extensions.emplace_back(ext);
}
InputModel::Ptr FrontEnd::load_impl(const std::vector<ov::Any>& variants) const {
@ -119,7 +118,7 @@ InputModel::Ptr FrontEnd::load_impl(const std::vector<ov::Any>& variants) const
auto create_extensions_map = [&]() -> std::unordered_map<ov::DiscreteTypeInfo, ov::BaseOpExtension::Ptr> {
std::unordered_map<ov::DiscreteTypeInfo, ov::BaseOpExtension::Ptr> exts;
for (const auto& ext : extensions) {
for (const auto& ext : m_extensions) {
if (auto base_ext = std::dynamic_pointer_cast<ov::BaseOpExtension>(ext))
exts.insert({base_ext->get_type_info(), base_ext});
}

View File

@ -77,10 +77,6 @@ protected:
std::function<std::map<std::string, OutputVector>(const std::map<std::string, Output<Node>>&,
const std::shared_ptr<OpPlace>&)> func);
// m_extensions should be the first member here,
// m_extensions can contain SO Extension (holder for other Extensions),
// so it should be released last.
std::vector<Extension::Ptr> m_extensions;
TelemetryExtension::Ptr m_telemetry;
std::vector<DecoderTransformationExtension::Ptr> m_transformation_extensions;
std::vector<ConversionExtensionBase::Ptr> m_conversion_extensions;

View File

@ -73,10 +73,6 @@ protected:
bool no_conversion,
std::shared_ptr<ov::Model>& ng_function) const;
// m_extensions should be the first member here,
// m_extensions can contain SO Extension (holder for other Extensions),
// so it should be released last.
std::vector<Extension::Ptr> m_extensions;
TelemetryExtension::Ptr m_telemetry;
std::vector<DecoderTransformationExtension::Ptr> m_transformation_extensions;
std::vector<ConversionExtensionBase::Ptr> m_conversion_extensions;