Skip to content

Commit

Permalink
Protect get and set accesses to boost::function objects in BgpPeerTest
Browse files Browse the repository at this point in the history
Use a mutex to protect the get and set from parallel access. Also rename the
variables to lower case and make them private.

Change-Id: I090379db01a5344cd23cd5570c6a02c09656c8d6
Closes-Bug: 1706747
  • Loading branch information
ananth-at-camphor-networks committed Jul 26, 2017
1 parent f6b3470 commit 9a5bed0
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 24 deletions.
9 changes: 6 additions & 3 deletions src/bgp/l3vpn/test/inetvpn_peer_test.cc
Expand Up @@ -301,7 +301,8 @@ class L3VPNPeerTest : public ::testing::TestWithParam<const char *> {
a_peer_red_ = dynamic_cast<BgpPeerTest *>(
a_red->peer_manager()->PeerLocate(
a_.get(), a_peer_red_config_.get()));
a_peer_red_->IsReady_fnc_ = boost::bind(&L3VPNPeerTest::IsReady, this);
a_peer_red_->set_is_ready_fnc(
boost::bind(&L3VPNPeerTest::IsReady, this));
a_peer_red_->Register(a_red_inet_, policy);

a_peer_blue_config_.reset(new BgpNeighborConfig());
Expand All @@ -315,7 +316,8 @@ class L3VPNPeerTest : public ::testing::TestWithParam<const char *> {
a_peer_blue_ = dynamic_cast<BgpPeerTest *>(
a_blue->peer_manager()->PeerLocate(
a_.get(), a_peer_blue_config_.get()));
a_peer_blue_->IsReady_fnc_ = boost::bind(&L3VPNPeerTest::IsReady, this);
a_peer_blue_->set_is_ready_fnc(
boost::bind(&L3VPNPeerTest::IsReady, this));
a_peer_blue_->Register(a_blue_inet_, policy);

b_peer_blue_config_.reset(new BgpNeighborConfig());
Expand All @@ -329,7 +331,8 @@ class L3VPNPeerTest : public ::testing::TestWithParam<const char *> {
b_peer_blue_ = dynamic_cast<BgpPeerTest *>(
b_blue->peer_manager()->PeerLocate(
b_.get(), b_peer_blue_config_.get()));
b_peer_blue_->IsReady_fnc_ = boost::bind(&L3VPNPeerTest::IsReady, this);
b_peer_blue_->set_is_ready_fnc(
boost::bind(&L3VPNPeerTest::IsReady, this));
b_peer_blue_->Register(b_blue_inet_, policy);

WaitForIdle();
Expand Down
12 changes: 6 additions & 6 deletions src/bgp/test/bgp_peer_close_test.cc
Expand Up @@ -765,14 +765,14 @@ void BgpPeerCloseTest::AddPeers(const BgpInstanceConfig *instance_config) {

// Override certain default routines to customize behavior that we want
// in this test.
npeer->peer()->SendUpdate_fnc_ =
npeer->peer()->set_send_update_fnc(
boost::bind(&BgpPeerCloseTest::SendUpdate, this, npeer->peer(),
_1, _2);
npeer->peer()->MpNlriAllowed_fnc_ =
_1, _2));
npeer->peer()->set_mp_nlri_allowed_fnc(
boost::bind(&BgpPeerCloseTest::MpNlriAllowed, this, npeer->peer(),
_1, _2);
npeer->peer()->IsReady_fnc_ =
boost::bind(&BgpPeerCloseTest::IsReady, this, true);
_1, _2));
npeer->peer()->set_is_ready_fnc(
boost::bind(&BgpPeerCloseTest::IsReady, this, true));
peers_.push_back(npeer);
}
}
Expand Down
12 changes: 6 additions & 6 deletions src/bgp/test/bgp_server_test_util.cc
Expand Up @@ -195,12 +195,12 @@ BgpPeerTest::BgpPeerTest(BgpServer *server, RoutingInstance *rtinst,
const BgpNeighborConfig *config)
: BgpPeer(server, rtinst, config), id_(0),
work_queue_(TaskScheduler::GetInstance()->GetTaskId("bgp::Config"), 0,
boost::bind(&BgpPeerTest::ProcessRequest, this, _1)) {
SendUpdate_fnc_ = boost::bind(&BgpPeerTest::BgpPeerSendUpdate, this,
_1, _2);
MpNlriAllowed_fnc_ = boost::bind(&BgpPeerTest::BgpPeerMpNlriAllowed, this,
_1, _2);
IsReady_fnc_ = boost::bind(&BgpPeerTest::BgpPeerIsReady, this);
boost::bind(&BgpPeerTest::ProcessRequest, this, _1)),
send_update_fnc_(boost::bind(&BgpPeerTest::BgpPeerSendUpdate, this,
_1, _2)),
mp_nlri_allowed_fnc_(boost::bind(&BgpPeerTest::BgpPeerMpNlriAllowed,
this, _1, _2)),
is_ready_fnc_(boost::bind(&BgpPeerTest::BgpPeerIsReady, this)) {
}

BgpPeerTest::~BgpPeerTest() {
Expand Down
38 changes: 31 additions & 7 deletions src/bgp/test/bgp_server_test_util.h
Expand Up @@ -282,12 +282,26 @@ class BgpPeerTest : public BgpPeer {

bool BgpPeerSendUpdate(const uint8_t *msg, size_t msgsize);
virtual bool SendUpdate(const uint8_t *msg, size_t msgsize) {
return SendUpdate_fnc_(msg, msgsize);
tbb::mutex::scoped_lock lock(fnc_mutex_);
return send_update_fnc_(msg, msgsize);
}

void set_send_update_fnc(
boost::function<bool(const uint8_t *, size_t)> send_update_fnc) {
tbb::mutex::scoped_lock lock(fnc_mutex_);
send_update_fnc_ = send_update_fnc;
}

bool BgpPeerMpNlriAllowed(uint16_t afi, uint8_t safi);
virtual bool MpNlriAllowed(uint16_t afi, uint8_t safi) {
return MpNlriAllowed_fnc_(afi, safi);
tbb::mutex::scoped_lock lock(fnc_mutex_);
return mp_nlri_allowed_fnc_(afi, safi);
}

void set_mp_nlri_allowed_fnc(
boost::function<bool(uint16_t, uint8_t)> mp_nlri_allowed_fnc) {
tbb::mutex::scoped_lock lock(fnc_mutex_);
mp_nlri_allowed_fnc_ = mp_nlri_allowed_fnc;
}

bool BgpPeerIsReady();
Expand All @@ -302,7 +316,16 @@ class BgpPeerTest : public BgpPeer {
}
}

virtual bool IsReady() const { return IsReady_fnc_(); }
virtual bool IsReady() const {
tbb::mutex::scoped_lock lock(fnc_mutex_);
return is_ready_fnc_();
}

void set_is_ready_fnc(boost::function<bool()> is_ready_fnc) {
tbb::mutex::scoped_lock lock(fnc_mutex_);
is_ready_fnc_ = is_ready_fnc;
}

void set_vpn_tables_registered(bool flag) { vpn_tables_registered_ = flag; }
const int id() const { return id_; }
void set_id(int id) { id_ = id; }
Expand All @@ -324,10 +347,6 @@ class BgpPeerTest : public BgpPeer {
cond_var_.wait(lock);
}

boost::function<bool(const uint8_t *, size_t)> SendUpdate_fnc_;
boost::function<bool(uint16_t, uint8_t)> MpNlriAllowed_fnc_;
boost::function<bool()> IsReady_fnc_;

BgpTestUtil util_;

private:
Expand All @@ -345,6 +364,11 @@ class BgpPeerTest : public BgpPeer {
WorkQueue<Request *> work_queue_;
tbb::mutex work_mutex_;
tbb::interface5::condition_variable cond_var_;

mutable tbb::mutex fnc_mutex_;
boost::function<bool(const uint8_t *, size_t)> send_update_fnc_;
boost::function<bool(uint16_t, uint8_t)> mp_nlri_allowed_fnc_;
boost::function<bool()> is_ready_fnc_;
};

class PeerManagerTest : public PeerManager {
Expand Down
4 changes: 2 additions & 2 deletions src/bgp/test/show_route_test.cc
Expand Up @@ -115,8 +115,8 @@ class ShowRouteTestBase : public ::testing::Test {
BgpConfigManager::kMasterInstance, uuid) != NULL);
peers_[j] = a_->FindPeerByUuid(
BgpConfigManager::kMasterInstance, uuid);
peers_[j]->IsReady_fnc_ =
boost::bind(&ShowRouteTestBase::IsReady, this);
peers_[j]->set_is_ready_fnc(
boost::bind(&ShowRouteTestBase::IsReady, this));
}

InetTable *table_a =
Expand Down

0 comments on commit 9a5bed0

Please sign in to comment.