Race condition between route handling and VRF subscription

As part of fix for 1563550, fix for one corner case is missed.

Consider the case where
 1. route add operation is enqueued to DB,
 2. unsubscribe for the instance is received,
 3. membership manager API to unregister table is called, membership managers
 starts leave and part of the table is walked but membership manager has not
 yet removed IPeerRib,
 4. Route add request is processed and path gets added.

Fix:
1. Reset the subscription gen id when unregister request is handled. i.e.
before starting the walk. This will ensure that delayed route add is rejected.

2. Add UT cases to validate above mentioned scenario

Change-Id: Ic12259b852227c1f199a67611da49a3563010026
Closes-Bug: #1574169

src/bgp/bgp_peer_membership.cc

@@ -507,6 +507,8 @@ void PeerRibMembershipManager::Leave(BgpTable *table,
         if (peer_rib) {
             // Ignore peer ribs which are already in close process
             if (peer_rib->IsRibOutActive()) peer_rib->DeactivateRibOut();
+            // Reset the subscription generation id
+            peer_rib->set_subscription_gen_id(0);
         }
     }
 

src/bgp/test/bgp_xmpp_deferq_test.cc

@@ -737,6 +737,64 @@ TEST_F(BgpXmppUnitTest, BasicDelayedInput_2) {
     task_util::WaitForIdle();
 }
 
+//
+// Verify the case where route add is processed when peer membership manager is
+// half way through the unregister process.
+//
+TEST_F(BgpXmppUnitTest, BasicDelayedInput_3) {
+    DBTableWalker::SetIterationToYield(1);
+    Configure();
+    task_util::WaitForIdle();
+
+    // create an XMPP client in server A
+    agent_a_.reset(
+        new test::NetworkAgentMock(&evm_, SUB_ADDR, xs_a_->GetPort()));
+
+    TASK_UTIL_EXPECT_TRUE(bgp_channel_manager_->channel_ != NULL);
+    TASK_UTIL_EXPECT_TRUE(agent_a_->IsEstablished());
+
+    agent_a_->Subscribe("blue", 1);
+    task_util::WaitForIdle();
+
+    // Add multiple routes to each DBTablePartition
+    for (int idx = 0; idx < 255; idx++) {
+        string prefix = string("10.1.") + integerToString(idx / 255) +
+            "." + integerToString(idx % 255) + "/32";
+        agent_a_->AddRoute("blue", prefix);
+    }
+    TASK_UTIL_EXPECT_EQ(255, agent_a_->RouteCount());
+    ASSERT_TRUE(agent_a_->RouteCount() == 255);
+
+    // Disable DB input processing
+    a_->database()->SetQueueDisable(true);
+
+    agent_a_->AddRoute("blue", "10.0.1.1/32");
+    task_util::WaitForIdle();
+    DBTableBase *blue_tbl = a_->database()->FindTable("blue.inet.0");
+    uint64_t walk_count =  blue_tbl->walk_request_count();
+
+    // Send unsubscribe for the VRF with pending route add
+    agent_a_->Unsubscribe("blue", -1, false);
+
+    // Enable the DB input partition when DB walk request for Leave is
+    // in progress
+    TASK_UTIL_EXPECT_NE(blue_tbl->walk_request_count(), walk_count);
+
+    // Enable DB Partition
+    a_->database()->SetQueueDisable(false);
+
+    // The unsubscribe request should have been processed by the membership
+    // manager and a response returned.
+    TASK_UTIL_EXPECT_FALSE(
+        PeerHasPendingMembershipRequests(bgp_channel_manager_->channel_));
+    TASK_UTIL_EXPECT_TRUE(
+        PeerNotRegistered(bgp_channel_manager_->channel_, "blue"));
+
+    TASK_UTIL_EXPECT_EQ(0, agent_a_->RouteCount());
+    ASSERT_TRUE(agent_a_->RouteCount() == 0);
+    DBTableWalker::SetIterationToYield(DBTableWalker::kIterationToYield);
+}
+
 TEST_F(BgpXmppUnitTest, RegisterWithoutRoutingInstance) {
     ConfigureWithoutRoutingInstances();
     task_util::WaitForIdle();

src/db/db_table_walker.cc

@@ -14,6 +14,7 @@
 #include "db/db_table_partition.h"
 
 int DBTableWalker::walker_task_id_ = -1;
+int DBTableWalker::max_iteration_to_yield_ = kIterationToYield;
 
 class DBTableWalker::Walker {
 public:

src/db/db_table_walker.h

@@ -30,6 +30,7 @@ class DBTableWalker {
 
     typedef int WalkId;
 
+    static const int kIterationToYield = 1024;
     static const WalkId kInvalidWalkerId = -1;
 
     DBTableWalker();
@@ -44,25 +45,27 @@ class DBTableWalker {
     // the walker function itself.
     void WalkCancel(WalkId id);
 
+    static void SetIterationToYield(int count) {
+        max_iteration_to_yield_ = count;
+    }
 private:
     static int walker_task_id_;
-    static const int kIterationToYield = 1024;
+    static int max_iteration_to_yield_;
 
     static int GetIterationToYield() {
-        static int iter_ = kIterationToYield;
         static bool init_ = false;
 
         if (!init_) {
 
             // XXX To be used for testing purposes only.
             char *count_str = getenv("DB_ITERATION_TO_YIELD");
             if (count_str) {
-                iter_ = strtol(count_str, NULL, 0);
+                max_iteration_to_yield_ = strtol(count_str, NULL, 0);
             }
             init_ = true;
         }
 
-        return iter_;
+        return max_iteration_to_yield_;
     }
 
     // A Walker allocated to iterator through a DBTable