[Snippets] Fixed set for Windows#17859
[Snippets] Fixed set for Windows#17859dmitry-gorokhov merged 2 commits intoopenvinotoolkit:masterfrom
Conversation
|
/azp run openvino-onnxruntime-lin |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run openvino-ngraph-onnx-lin |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| const auto in_port = expr->get_input_port(i); | ||
| const auto& parent_expr = in_port.get_connected_ports().begin()->get_expr(); | ||
| const auto source_port = *in_port.get_connected_ports().begin(); | ||
| const auto& parent_expr = source_port.get_expr(); |
There was a problem hiding this comment.
Please describe in details why original code led to segfault on Windows?
There was a problem hiding this comment.
Honestly, I'm faced with this problem several times on ngraph (get_output_target_inputs has the same behavior) 😄
Let's take a look at previous 109 line:
get_connected_ports() returns sets of ExpressionPort instances (the method creates new set and returns it).
Then we take iterator on first element using begin() (this element is definitely in the set).
And we call get_expr from this first element and takes reference of shared pointer. But the set of connected ports is destroyed since it's local variable and isn't saved in this code frame. Thus, reference of shared pointer refs to destroyed memory as I understand. And we get corrupted data:
But as I said, this element is definitely in the set:
Sure, there is another solution. Save just the copy of shared pointer, not reference:
| const auto& parent_expr = source_port.get_expr(); | |
| const auto parent_expr = in_port.get_connected_ports().begin()->get_expr(); |
for every taste 🙂
There was a problem hiding this comment.
Thank you for the explanation. On my personal taste I would prefer one line solution.
There was a problem hiding this comment.
As you say 😁
Done
4d9bebf to
7089efe
Compare
Details: