Sandesh session delete happens in the sandesh client state

machine context and the session is used to enqueue messages
from client context and these can be executing concurrently
causing the session to be deleted when being used from the
client context. The fix is to enqueue the messages from the
client to the state machine - EvSandeshSend and from the
state machine context enqueue on to the session send queue.

Enabled sandesh client state machine unit tests.

Change-Id: I2d33c9348f71c99108bfc5fdfc2966b42231b214
Closes-Bug: #1460946
(cherry picked from commit 31994e0)

library/cpp/sandesh_client.cc

@@ -114,17 +114,7 @@ void SandeshClient::Shutdown() {
 }
 
 bool SandeshClient::SendSandesh(Sandesh *snh) {
-    if (!sm_->session() || sm_->session()->IsClosed()) {
-        return false;
-    }
-    SandeshClientSM::State state = sm_->state();
-    if (state != SandeshClientSM::CLIENT_INIT && 
-        state != SandeshClientSM::ESTABLISHED) {
-        return false;
-    }
-    // XXX No bounded work queue
-    sm_->session()->send_queue()->Enqueue(snh);
-    return true; 
+    return sm_->SendSandesh(snh);
 }
 
 bool SandeshClient::ReceiveCtrlMsg(const std::string &msg,

library/cpp/sandesh_client.h

@@ -47,7 +47,7 @@ class SandeshClient : public TcpServer, public SandeshClientSM::Mgr {
     void Initiate();
     void Shutdown();
 
-    SandeshSession *CreateSMSession(
+    virtual SandeshSession *CreateSMSession(
             TcpSession::EventObserver eocb,
             SandeshReceiveMsgCb rmcb,
             TcpServer::Endpoint ep);
@@ -82,6 +82,10 @@ class SandeshClient : public TcpServer, public SandeshClientSM::Mgr {
         return sm_->session();
     }
 
+    SandeshClientSM* state_machine() const {
+        return sm_.get();
+    }
+
     void SetSessionWaterMarkInfo(Sandesh::QueueWaterMarkInfo &scwm);
     void ResetSessionWaterMarkInfo();
     void GetSessionWaterMarkInfo(

library/cpp/sandesh_client_sm.cc

@@ -491,14 +491,21 @@ struct ClientInit : public sc::state<ClientInit, SandeshClientSMImpl> {
 
     sc::result react(const EvSandeshSend &event) {
         SandeshClientSMImpl *state_machine = &context<SandeshClientSMImpl>();
-        SM_LOG(DEBUG, state_machine->StateName() << " : " << event.Name() << " : " << event.snh->Name());
-        if (dynamic_cast<SandeshUVE *>(event.snh)) {
-            SM_LOG(INFO, "Received UVE message in wrong state : " << event.snh->Name());
-            event.snh->Release();
+        Sandesh *snh(event.snh);
+        SM_LOG(DEBUG, state_machine->StateName() << " : " << event.Name() <<
+            " : " << snh->Name());
+        if (dynamic_cast<SandeshUVE *>(snh)) {
+            if (snh->IsLoggingDroppedAllowed()) {
+                SANDESH_LOG(ERROR, "SANDESH: Send FAILED: " <<
+                    snh->ToString());
+            }
+            Sandesh::UpdateSandeshStats(snh->Name(), 0, true, true);
+            SM_LOG(INFO, "Received UVE message in wrong state : " << snh->Name());
+            snh->Release();
             return discard_event(); 
         }
-        if (!state_machine->send_session(event.snh)) {
-            SM_LOG(ERROR, "Could not EnQ Sandesh " << event.snh->Name());
+        if (!state_machine->send_session(snh)) {
+            SM_LOG(INFO, "Could not EnQ Sandesh :" << snh->Name());
             // If Enqueue encounters an error, it will release the Sandesh
         }
         return discard_event();
@@ -634,7 +641,7 @@ struct Established : public sc::state<Established, SandeshClientSMImpl> {
 
 void SandeshClientSMImpl::SetAdminState(bool down) {
     if (down) {
-        Enqueue(scm::EvStop(false));
+        Enqueue(scm::EvStop());
     } else {
         reset_idle_hold_time();
         // On fresh restart of state machine, all previous state should be reset
@@ -659,8 +666,14 @@ void SandeshClientSMImpl::OnIdle(const Ev &event) {
 
 template <class Ev>
 void SandeshClientSMImpl::ReleaseSandesh(const Ev &event) {
-    SM_LOG(DEBUG, "Wrong state: " << StateName() << " for event: " << event.Name());
-    event.snh->Release();
+    Sandesh *snh(event.snh);
+    if (snh->IsLoggingDroppedAllowed()) {
+        SANDESH_LOG(ERROR, "SANDESH: Send FAILED: " << snh->ToString());
+    }
+    Sandesh::UpdateSandeshStats(snh->Name(), 0, true, true);
+    SM_LOG(DEBUG, "Wrong state: " << StateName() << " for event: " <<
+       event.Name() << " message: " << snh->Name());
+    snh->Release();
 }
 
 template <class Ev>
@@ -766,6 +779,11 @@ bool SandeshClientSMImpl::SendSandeshUVE(Sandesh * snh) {
     return true;
 }
 
+bool SandeshClientSMImpl::SendSandesh(Sandesh * snh) {
+    Enqueue(scm::EvSandeshSend(snh));
+    return true;
+}
+
 void SandeshClientSMImpl::OnMessage(SandeshSession *session,
                                     const std::string &msg) {
     // Demux based on Sandesh message type
@@ -796,6 +814,11 @@ static const std::string state_names[] = {
     "Established",
 };
 
+const string &SandeshClientSMImpl::StateName(
+    SandeshClientSM::State state) const {
+    return state_names[state];
+}
+
 const string &SandeshClientSMImpl::StateName() const {
     return state_names[state_];
 }

library/cpp/sandesh_client_sm.h

@@ -80,6 +80,9 @@ class SandeshClientSM {
     // This function is used to send UVE sandesh's to the server
     virtual bool SendSandeshUVE(Sandesh* snh) = 0;
 
+    // This function is used to send sandesh's to the server
+    virtual bool SendSandesh(Sandesh* snh) = 0;
+
     virtual ~SandeshClientSM() {}
 
 protected:

library/cpp/sandesh_client_sm_priv.h

@@ -63,6 +63,7 @@ class SandeshClientSMImpl : public SandeshClientSM,
     void GetCandidates(TcpServer::Endpoint& active,
             TcpServer::Endpoint &backup) const;
     bool SendSandeshUVE(Sandesh* snh);
+    bool SendSandesh(Sandesh* snh);
     void EnqueDelSession(SandeshSession * session);
 
     // Feed session events into the state machine.
@@ -95,6 +96,7 @@ class SandeshClientSMImpl : public SandeshClientSM,
     int GetConnectTime() const;
 
     const std::string &StateName() const;
+    const std::string &StateName(SandeshClientSM::State state) const;
     const std::string &LastStateName() const;
 
     void set_state(State state) {
@@ -178,6 +180,8 @@ class SandeshClientSMImpl : public SandeshClientSM,
     std::string generator_key_;
     SandeshEventStatistics event_stats_;
 
+    friend class SandeshClientStateMachineTest;
+
     DISALLOW_COPY_AND_ASSIGN(SandeshClientSMImpl);
 };
 

library/cpp/test/SConscript

@@ -122,17 +122,18 @@ sandesh_http_test = env.UnitTest('sandesh_http_test',
 env.Alias('src/sandesh:sandesh_http_test', sandesh_http_test)
 env.Requires(sandesh_http_test, '#/build/include/libxml2/libxml/tree.h')
 
+sandesh_test_common_obj = env.Object('sandesh_test_common.cc')
 sandesh_state_machine_test = env.UnitTest('sandesh_state_machine_test',
-                                          ['sandesh_state_machine_test.cc']
+                                          ['sandesh_state_machine_test.cc'] +
+                                           sandesh_test_common_obj 
                                           )
 env.Alias('src/sandesh:sandesh_state_machine_test', sandesh_state_machine_test)
 
-'''
 sandesh_client_test = env.UnitTest('sandesh_client_test',
-                                          ['sandesh_client_test.cc']
+                                          ['sandesh_client_test.cc'] +
+                                           sandesh_test_common_obj 
                                           )
 env.Alias('src/sandesh:sandesh_client_test', sandesh_client_test)
-'''
 
 test_suite = [sandesh_message_test,
               sandesh_rw_test,
@@ -141,7 +142,7 @@ test_suite = [sandesh_message_test,
               sandesh_http_test,
               sandesh_state_machine_test,
               sandesh_perf_test,
-#             sandesh_client_test,
+              sandesh_client_test,
            ]
 
 test = env.TestSuite('sandesh-test', test_suite)