Clearing service instance DB State after STOP command

If Delete of service instance is received before it was marked
unusable, to stop the namespace, StopServiceInstance command is issued,
which invokes the deletion of the namespace script. And immediately after
this, DB state is cleared. Clearing of DB State can result in deleting
of service instance DBEntry before namespace script completion. SIGCHILD
event after script completion looks for service instance that triggered the
task, and results in crash as the service isntance is already deleted.
To avoid crash, the DBState is cleared after processing the SIGCHILD
event. Also corresponding to the service instance, there can be many
tasks queued for execution. To identify how any SIGCHILDS to wait for,
running tasks cound is maintained in DBState. This could is manipulated
everytime the task is created, destroyed, error handled corresponding to
the service instance.

Testcases to follow in the next commit

Change-Id: I4b9853de271b8ee54016bb91b7a2e199cbce6b0d
closes-bug: #1449166
closes-bug: #1453956

src/vnsw/agent/oper/instance_manager.cc

@@ -234,13 +234,16 @@ void InstanceManager::SigChldEventHandler(InstanceManagerChildEvent event) {
          if (!task_queue->Empty()) {
              InstanceTask *task = task_queue->Front();
              if (task->pid() == event.pid) {
+
+                 //Get the sevice instance first, to delete the state later
+                 ServiceInstance* svc_instance = GetSvcInstance(task);
                  UpdateStateStatusType(task, event.status);
 
                  task_queue->Pop();
                  delete task;
 
                  task_queue->StopTimer();
-
+                 DeleteState(svc_instance);
                  ScheduleNextTask(task_queue);
                  return;
              }
@@ -258,6 +261,7 @@ void InstanceManager::OnErrorEventHandler(InstanceManagerChildEvent event) {
     if (state != NULL) {
        state->set_errors(event.errors);
     }
+    ScheduleNextTask(event.task_queue);
 }
 
 bool InstanceManager::DequeueEvent(InstanceManagerChildEvent event) {
@@ -335,6 +339,23 @@ void InstanceManager::ClearState(ServiceInstance *svc_instance) {
     svc_instance->ClearState(agent_->service_instance_table(), si_listener_);
 }
 
+bool InstanceManager::DeleteState(ServiceInstance *svc_instance) {
+
+    if (!svc_instance || !svc_instance->IsDeleted()) {
+        return false;
+    }
+
+    InstanceState *state = GetState(svc_instance);
+    if (state && !state->tasks_running()) {
+        ClearState(svc_instance);
+        delete state;
+        ClearLastCmdType(svc_instance);
+        return true;
+    }
+
+    return false;
+}
+
 void InstanceManager::InitSigHandler(AgentSignal *signal) {
     signal->RegisterChildHandler(
         boost::bind(&InstanceManager::HandleSigChild, this, _1, _2, _3, _4));
@@ -351,6 +372,7 @@ void InstanceManager::StateClear() {
         if (state != NULL) {
             entry->ClearState(agent_->service_instance_table(), si_listener_);
             delete state;
+            ClearLastCmdType(static_cast<ServiceInstance *>(entry));
         }
         next = partition->GetNext(entry);
     }
@@ -445,11 +467,17 @@ void InstanceManager::ScheduleNextTask(InstanceTaskQueue *task_queue) {
 
             if (delay > (netns_timeout_ * 2)) {
                task->Terminate();
-
                task_queue->StopTimer();
                task_queue->Pop();
+
+               ServiceInstance* svc_instance = GetSvcInstance(task);
+               if (state && svc_instance)
+                   state->decr_tasks_running();
+
                task_svc_instances_.erase(task);
 
+               DeleteState(svc_instance);
+
                delete task;
             } else {
                task->Stop();
@@ -471,6 +499,9 @@ ServiceInstance *InstanceManager::GetSvcInstance(InstanceTask *task) const {
 void InstanceManager::RegisterSvcInstance(InstanceTask *task,
                                           ServiceInstance *svc_instance) {
     task_svc_instances_.insert(std::make_pair(task, svc_instance));
+    InstanceState *state = GetState(svc_instance);
+    assert(state);
+    state->incr_tasks_running();
 }
 
 ServiceInstance *InstanceManager::UnregisterSvcInstance(InstanceTask *task) {
@@ -479,6 +510,9 @@ ServiceInstance *InstanceManager::UnregisterSvcInstance(InstanceTask *task) {
          iter != task_svc_instances_.end(); ++iter) {
         if (task == iter->first) {
             ServiceInstance *svc_instance = iter->second;
+            InstanceState *state = GetState(svc_instance);
+            assert(state);
+            state->decr_tasks_running();
             task_svc_instances_.erase(iter);
             return svc_instance;
         }
@@ -488,11 +522,16 @@ ServiceInstance *InstanceManager::UnregisterSvcInstance(InstanceTask *task) {
 }
 
 void InstanceManager::UnregisterSvcInstance(ServiceInstance *svc_instance) {
+
+    InstanceState *state = GetState(svc_instance);
+    assert(state);
+
     std::map<InstanceTask *, ServiceInstance*>::iterator iter =
         task_svc_instances_.begin();
     while(iter != task_svc_instances_.end()) {
         if (svc_instance == iter->second) {
             task_svc_instances_.erase(iter++);
+            state->decr_tasks_running();
         } else {
             ++iter;
         }
@@ -629,14 +668,15 @@ void InstanceManager::EventObserver(
 
     InstanceState *state = GetState(svc_instance);
     if (svc_instance->IsDeleted()) {
-        UnregisterSvcInstance(svc_instance);
         if (state) {
             if (GetLastCmdType(svc_instance) == Start) {
                 StopServiceInstance(svc_instance, state);
+                SetLastCmdType(svc_instance, Stop);
             }
 
-            ClearState(svc_instance);
-            delete state;
+            if (DeleteState(svc_instance)) {
+                return;
+            }
         }
         ClearLastCmdType(svc_instance);
     } else {
@@ -717,7 +757,7 @@ void InstanceManager::SetNamespaceStorePath(std::string path) {
  * InstanceState class
  */
 InstanceState::InstanceState() : DBState(),
-        pid_(0), status_(0), status_type_(0) {
+        pid_(0), status_(0), status_type_(0), tasks_running_(0) {
 }
 
 void InstanceState::Clear() {

src/vnsw/agent/oper/instance_manager.h

@@ -110,6 +110,7 @@ class InstanceManager {
     InstanceState *GetState(InstanceTask* task) const;
     void SetState(ServiceInstance *svc_instance, InstanceState *state);
     void ClearState(ServiceInstance *svc_instance);
+    bool DeleteState(ServiceInstance *svc_instance);
     void UpdateStateStatusType(InstanceTask* task, int status);
 
     void SetLastCmdType(ServiceInstance *svc_instance, int last_cmd_type);
@@ -215,6 +216,21 @@ class InstanceState : public DBState {
         return status_type_;
     }
 
+    int tasks_running() const {
+        return tasks_running_;
+    }
+
+    int incr_tasks_running() {
+        return ++tasks_running_;
+    }
+
+    int decr_tasks_running() {
+        tasks_running_--;
+        assert(!(tasks_running_ < 0));
+        return tasks_running_;
+    }
+
+
     void Clear();
 
  private:
@@ -223,6 +239,7 @@ class InstanceState : public DBState {
     std::string errors_;
     std::string cmd_;
     int status_type_;
+    int tasks_running_;
 
     ServiceInstance::Properties properties_;