Skip to content

Commit

Permalink
Fix EdgeHolder from incorrectly reporting an active connection (#402)
Browse files Browse the repository at this point in the history
* Prevents `check_active_connection` from mistakenly returning true for a holder where `init_owned_edge` has been called but neither the `init_connected_edge method` or the `add_connector` method have not been called.

Relates to issue #360

Authors:
  - David Gardner (https://github.com/dagardner-nv)

Approvers:
  - Michael Demoret (https://github.com/mdemoret-nv)

URL: #402
  • Loading branch information
dagardner-nv authored Sep 28, 2023
1 parent 8b20469 commit ba483b1
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 2 deletions.
1 change: 0 additions & 1 deletion cpp/mrc/include/mrc/edge/edge_holder.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ class EdgeHolder

void release_edge_connection()
{
m_owned_edge_lifetime.reset();
m_connected_edge.reset();
}

Expand Down
36 changes: 35 additions & 1 deletion cpp/mrc/tests/test_edges.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@

#include "mrc/channel/buffered_channel.hpp" // IWYU pragma: keep
#include "mrc/channel/forward.hpp"
#include "mrc/edge/edge.hpp" // for Edge
#include "mrc/edge/edge_builder.hpp"
#include "mrc/edge/edge_channel.hpp"
#include "mrc/edge/edge_holder.hpp" // for EdgeHolder
#include "mrc/edge/edge_readable.hpp"
#include "mrc/edge/edge_writable.hpp"
#include "mrc/node/generic_source.hpp"
Expand All @@ -40,7 +42,6 @@
#include <rxcpp/rx.hpp> // for observable_member

#include <functional>
#include <map>
#include <memory>
#include <ostream>
#include <stdexcept>
Expand Down Expand Up @@ -996,4 +997,37 @@ TEST_F(TestEdges, EdgeTapWithSpliceRxComponent)

EXPECT_TRUE(node->stream_fn_called);
}

template <typename T>
class TestEdgeHolder : public edge::EdgeHolder<T>
{
public:
bool has_active_connection() const
{
return this->check_active_connection(false);
}

void call_release_edge_connection()
{
this->release_edge_connection();
}

void call_init_owned_edge(std::shared_ptr<edge::Edge<T>> edge)
{
this->init_owned_edge(std::move(edge));
}
};

TEST_F(TestEdges, EdgeHolderIsConnected)
{
TestEdgeHolder<int> edge_holder;
auto edge = std::make_shared<edge::Edge<int>>();
EXPECT_FALSE(edge_holder.has_active_connection());

edge_holder.call_init_owned_edge(edge);
EXPECT_FALSE(edge_holder.has_active_connection());

edge_holder.call_release_edge_connection();
EXPECT_FALSE(edge_holder.has_active_connection());
}
} // namespace mrc

0 comments on commit ba483b1

Please sign in to comment.