Fix corner case in bgp peer close processing

Do not initialize the state machine on admin up if close process is in
progress.  The state machine will eventually be initialized when close
finishes, via BgpPeer::BgpPeerClose::CloseComplete.

Note that this only impacts unit tests since we don't expose admin/up
down functionality to users.

Change-Id: Id3c18186dbe4fb232cdde2e0e1eb749faa3bc19e
Partial-Bug: 1548570

src/bgp/state_machine.cc

@@ -1137,11 +1137,12 @@ void StateMachine::SetAdminState(bool down) {
     if (down) {
         Enqueue(fsm::EvStop(BgpProto::Notification::AdminShutdown));
     } else {
+        // Reset all previous state.
         reset_idle_hold_time();
-        peer_->reset_flap_count();
-        // On fresh restart of state machine, all previous state should be reset
         reset_last_info();
-        Enqueue(fsm::EvStart());
+        peer_->reset_flap_count();
+        if (!peer_->IsCloseInProgress())
+            Enqueue(fsm::EvStart());
     }
 }
 

src/bgp/test/bgp_server_test.cc

@@ -2,7 +2,7 @@
  * Copyright (c) 2013 Juniper Networks, Inc. All rights reserved.
  */
 
-
+#include "base/task_annotations.h"
 #include "bgp/bgp_config_parser.h"
 #include "bgp/bgp_factory.h"
 #include "bgp/bgp_peer_membership.h"
@@ -1224,6 +1224,7 @@ TEST_F(BgpServerUnitTest, ChangePeerAddressFamilies) {
 }
 
 TEST_F(BgpServerUnitTest, AdminDown) {
+    ConcurrencyScope scope("bgp::Config");
     int peer_count = 3;
 
     BgpPeerTest::verbose_name(true);
@@ -1867,6 +1868,7 @@ TEST_F(BgpServerUnitTest, Passive2) {
 }
 
 TEST_F(BgpServerUnitTest, ResetStatsOnFlap) {
+    ConcurrencyScope scope("bgp::Config");
     int peer_count = 3;
 
     StateMachineTest::set_keepalive_time_msecs(10);
@@ -2650,6 +2652,7 @@ TEST_F(BgpServerUnitTest, DeleteInProgress) {
 }
 
 TEST_F(BgpServerUnitTest, CloseInProgress) {
+    ConcurrencyScope scope("bgp::Config");
     int peer_count = 3;
 
     vector<string> families_a;
@@ -2702,6 +2705,7 @@ TEST_F(BgpServerUnitTest, CloseInProgress) {
 }
 
 TEST_F(BgpServerUnitTest, CloseDeferred) {
+    ConcurrencyScope scope("bgp::Config");
     int peer_count = 3;
 
     vector<string> families_a;

src/bgp/test/bgp_server_test_util.cc

@@ -6,6 +6,7 @@
 
 #include <boost/foreach.hpp>
 
+#include "base/task_annotations.h"
 #include "bgp/bgp_config_ifmap.h"
 #include "bgp/bgp_config_parser.h"
 #include "bgp/bgp_factory.h"
@@ -138,13 +139,15 @@ BgpPeer *BgpServerTest::FindMatchingPeer(const string &routing_instance,
 }
 
 void BgpServerTest::DisableAllPeers() {
+    ConcurrencyScope scope("bgp::Config");
     for (BgpPeerList::iterator it = peer_list_.begin();
          it != peer_list_.end(); ++it) {
         it->second->SetAdminState(true);
     }
 }
 
 void BgpServerTest::EnableAllPeers() {
+    ConcurrencyScope scope("bgp::Config");
     for (BgpPeerList::iterator it = peer_list_.begin();
          it != peer_list_.end(); ++it) {
         it->second->SetAdminState(false);

src/bgp/test/bgp_stress_test.cc

@@ -11,6 +11,7 @@
 #include <map>
 #include <string>
 
+#include "base/task_annotations.h"
 #include "base/test/addr_test_util.h"
 
 #include "bgp/bgp_config_parser.h"
@@ -2103,6 +2104,7 @@ void BgpStressTest::DeleteBgpPeers(int npeers) {
 }
 
 void BgpStressTest::ClearBgpPeer(vector<int> peer_ids) {
+    ConcurrencyScope scope("bgp::Config");
     map<int, bool> established;
     map<int, uint32_t> flap_count;
 

src/bgp/test/state_machine_test.cc

@@ -394,10 +394,12 @@ class StateMachineUnitTest : public ::testing::Test {
         sm_->Shutdown(BgpProto::Notification::AdminShutdown);
     }
     void EvAdminUp() {
+        ConcurrencyScope scope("bgp::Config");
         peer_->SetAdminState(false);
         sm_->set_idle_hold_time(1);
     }
     void EvAdminDown() {
+        ConcurrencyScope scope("bgp::Config");
         peer_->SetAdminState(true);
         sm_->set_idle_hold_time(1);
     }