Skip to content

Commit

Permalink
Fix to not use the pointer which is released. Decommission peer list …
Browse files Browse the repository at this point in the history
…contains shared pointer.

This in turn releases the memory as it is the last reference. So dont use the pointer from same.
Take the required data before releasing.
Bug# https://bugs.launchpad.net/juniperopenstack/+bug/1328843
Change-Id: I481e0496094d5b2571aa5b3855a373ae3fa05062
  • Loading branch information
manishsing committed Jul 16, 2014
1 parent da404f8 commit 41f9f5a
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 4 deletions.
8 changes: 7 additions & 1 deletion src/vnsw/agent/controller/controller_init.cc
Expand Up @@ -472,18 +472,24 @@ void VNController::AddToDecommissionedPeerList(BgpPeerPtr peer) {
* destruction of same.
*/
void VNController::ControllerPeerHeadlessAgentDelDone(BgpPeer *bgp_peer) {
// Retain the disconnect state for peer as bgp_peer will be freed
// below.
bool is_disconnect_walk = bgp_peer->is_disconnect_walk();
for (BgpPeerIterator it = decommissioned_peer_list_.begin();
it != decommissioned_peer_list_.end(); ++it) {
BgpPeer *peer = static_cast<BgpPeer *>((*it).get());
if (peer == bgp_peer) {
//Release BGP peer, ideally this should be the last reference being
//released for peer.
decommissioned_peer_list_.remove(*it);
break;
}
}

// Delete walk for peer was issued via shutdown of agentxmppchannel
// If all bgp peers are gone(i.e. walk for delpeer for all decommissioned
// peer is over), go ahead with cleanup.
if (decommissioned_peer_list_.empty() && bgp_peer->is_disconnect_walk()) {
if (decommissioned_peer_list_.empty() && is_disconnect_walk) {
agent()->controller()->Cleanup();
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/vnsw/agent/pkt/test/test_pkt_fip.cc
Expand Up @@ -610,10 +610,10 @@ TEST_F(FlowTest, VmToServer_ecmp_to_nat) {

FlowEntry *entry = FlowGet(vnet[1]->vrf()->vrf_id(), vnet_addr[1], "169.254.169.254",
IPPROTO_TCP, 10000, 80);
FlowEntry *rev_old = NULL;
//FlowEntry *rev_old = NULL;
EXPECT_TRUE(entry);
if (entry) {
rev_old = entry->reverse_flow_entry();
//rev_old = entry->reverse_flow_entry();
EXPECT_TRUE(entry->data().component_nh_idx !=
CompositeNH::kInvalidComponentNHIdx);
}
Expand Down
10 changes: 9 additions & 1 deletion src/vnsw/agent/test/test_xmpp.cc
Expand Up @@ -171,19 +171,24 @@ class AgentXmppUnitTest : public ::testing::Test {
virtual void SetUp() {
xs = new XmppServer(&evm_, XmppInit::kControlNodeJID);
xc = new XmppClient(&evm_);
xmpp_init = new XmppInit();

xs->Initialize(0, false);

thread_.Start();
}

virtual void TearDown() {
ASSERT_TRUE(agent_->GetAgentXmppChannel(0) != NULL);

xs->Shutdown();
bgp_peer.reset();
client->WaitForIdle();
xc->Shutdown();
client->WaitForIdle();

agent_->SetAgentXmppClient(NULL, 0);
agent_->SetAgentXmppInit(NULL, 0);
TaskScheduler::GetInstance()->Stop();
Agent::GetInstance()->controller()->unicast_cleanup_timer().cleanup_timer_->Fire();
Agent::GetInstance()->controller()->multicast_cleanup_timer().cleanup_timer_->Fire();
Expand Down Expand Up @@ -427,7 +432,9 @@ class AgentXmppUnitTest : public ::testing::Test {
xc->RegisterConnectionEvent(xmps::BGP,
boost::bind(&AgentBgpXmppPeerTest::HandleXmppChannelEvent,
bgp_peer.get(), _2));
Agent::GetInstance()->SetAgentXmppChannel(bgp_peer.get(), 0);
agent_->SetAgentXmppChannel(bgp_peer.get(), 0);
agent_->SetAgentXmppClient(xc, 0);
agent_->SetAgentXmppInit(xmpp_init, 0);

// server connection
WAIT_FOR(1000, 10000,
Expand All @@ -443,6 +450,7 @@ class AgentXmppUnitTest : public ::testing::Test {

XmppServer *xs;
XmppClient *xc;
XmppInit *xmpp_init;

XmppConnection *sconnection;
XmppChannel *cchannel;
Expand Down

0 comments on commit 41f9f5a

Please sign in to comment.