Merge pull request #3572 from smartdevicelink/fix/fix_session_heartbeat_starting

Fix session heartbeat starting

11 files changed, 92 insertions, 21 deletions

diff --git a/src/components/application_manager/src/application_manager_impl.cc b/src/components/application_manager/src/application_manager_impl.cc
index 6b0ea39804..94fd85d3d8 100644
--- a/src/components/application_manager/src/application_manager_impl.cc
+++ b/src/components/application_manager/src/application_manager_impl.cc
@@ -674,17 +674,6 @@ ApplicationSharedPtr ApplicationManagerImpl::RegisterApplication(
           message[strings::params][strings::protocol_version].asInt());
   application->set_protocol_version(protocol_version);
 
-  if (protocol_handler::MajorProtocolVersion::PROTOCOL_VERSION_UNKNOWN !=
-      protocol_version) {
-    connection_handler().BindProtocolVersionWithSession(
-        connection_key, static_cast<uint8_t>(protocol_version));
-  }
-  if ((protocol_version ==
-       protocol_handler::MajorProtocolVersion::PROTOCOL_VERSION_3) &&
-      (get_settings().heart_beat_timeout() != 0)) {
-    connection_handler().StartSessionHeartBeat(connection_key);
-  }
-
   // Keep HMI add id in case app is present in "waiting for registration" list
   apps_to_register_list_lock_ptr_->Acquire();
   PolicyAppIdPredicate finder(application->policy_app_id());
diff --git a/src/components/connection_handler/include/connection_handler/connection.h b/src/components/connection_handler/include/connection_handler/connection.h
index 8b286e1309..4ef2d724c3 100644
--- a/src/components/connection_handler/include/connection_handler/connection.h
+++ b/src/components/connection_handler/include/connection_handler/connection.h
@@ -280,6 +280,14 @@ class Connection {
   void KeepAlive(uint8_t session_id);
 
   /**
+   * @brief Check is heartbeat monitoring started for specified session
+   * @param  session_id session id
+   * @return returns true if heartbeat monitoring started for specified session
+   * otherwise returns false
+   */
+  bool IsSessionHeartbeatTracked(const uint8_t session_id) const;
+
+  /**
    * @brief Start heartbeat for specified session
    * @param  session_id session id
    */
diff --git a/src/components/connection_handler/include/connection_handler/connection_handler_impl.h b/src/components/connection_handler/include/connection_handler/connection_handler_impl.h
index 3bfa5b34b8..a7dfd29b31 100644
--- a/src/components/connection_handler/include/connection_handler/connection_handler_impl.h
+++ b/src/components/connection_handler/include/connection_handler/connection_handler_impl.h
@@ -466,6 +466,14 @@ class ConnectionHandlerImpl
   void SendEndService(uint32_t key, uint8_t service_type) OVERRIDE;
 
   /**
+   * @brief Check is heartbeat monitoring started for specified connection key
+   * @param  connection_key pair of connection and session id
+   * @return returns true if heartbeat monitoring started for specified
+   * connection key otherwise returns false
+   */
+  bool IsSessionHeartbeatTracked(const uint32_t connection_key) const OVERRIDE;
+
+  /**
    * \brief Start heartbeat for specified session
    *
    * \param connection_key pair of connection and session id
diff --git a/src/components/connection_handler/include/connection_handler/heartbeat_monitor.h b/src/components/connection_handler/include/connection_handler/heartbeat_monitor.h
index 9e04929f79..7713d2490f 100644
--- a/src/components/connection_handler/include/connection_handler/heartbeat_monitor.h
+++ b/src/components/connection_handler/include/connection_handler/heartbeat_monitor.h
@@ -69,6 +69,14 @@ class HeartBeatMonitor : public threads::ThreadDelegate {
   void KeepAlive(uint8_t session_id);
 
   /**
+   * @brief Check is heartbeat monitoring started for specified session
+   * @param  session_id session id
+   * @return returns true if heartbeat monitoring started for specified session
+   * otherwise returns false
+   */
+  bool IsSessionHeartbeatTracked(const uint8_t session_id) const;
+
+  /**
    * \brief Thread exit procedure.
    */
   virtual void exitThreadMain();
@@ -111,7 +119,7 @@ class HeartBeatMonitor : public threads::ThreadDelegate {
   typedef std::map<uint8_t, SessionState> SessionMap;
   SessionMap sessions_;
 
-  sync_primitives::RecursiveLock sessions_list_lock_;  // recurcive
+  mutable sync_primitives::RecursiveLock sessions_list_lock_;  // recursive
   sync_primitives::Lock main_thread_lock_;
   mutable sync_primitives::Lock heartbeat_timeout_seconds_lock_;
   sync_primitives::ConditionalVariable heartbeat_monitor_;
diff --git a/src/components/connection_handler/src/connection.cc b/src/components/connection_handler/src/connection.cc
index 5bd72773c2..0ed5182a9b 100644
--- a/src/components/connection_handler/src/connection.cc
+++ b/src/components/connection_handler/src/connection.cc
@@ -527,6 +527,10 @@ bool Connection::ProtocolVersion(uint8_t session_id,
   return true;
 }
 
+bool Connection::IsSessionHeartbeatTracked(const uint8_t session_id) const {
+  return heartbeat_monitor_->IsSessionHeartbeatTracked(session_id);
+}
+
 bool Connection::ProtocolVersion(
     uint8_t session_id, utils::SemanticVersion& full_protocol_version) {
   SDL_LOG_AUTO_TRACE();
diff --git a/src/components/connection_handler/src/connection_handler_impl.cc b/src/components/connection_handler/src/connection_handler_impl.cc
index 549450dbe0..a771f0a104 100644
--- a/src/components/connection_handler/src/connection_handler_impl.cc
+++ b/src/components/connection_handler/src/connection_handler_impl.cc
@@ -1667,6 +1667,21 @@ void ConnectionHandlerImpl::SendEndService(uint32_t key, uint8_t service_type) {
   }
 }
 
+bool ConnectionHandlerImpl::IsSessionHeartbeatTracked(
+    const uint32_t connection_key) const {
+  SDL_LOG_AUTO_TRACE();
+  uint32_t connection_handle = 0;
+  uint8_t session_id = 0;
+  PairFromKey(connection_key, &connection_handle, &session_id);
+
+  sync_primitives::AutoReadLock lock(connection_list_lock_);
+  ConnectionList::const_iterator it = connection_list_.find(connection_handle);
+  if (connection_list_.end() != it) {
+    return it->second->IsSessionHeartbeatTracked(session_id);
+  }
+  return false;
+}
+
 void ConnectionHandlerImpl::StartSessionHeartBeat(uint32_t connection_key) {
   SDL_LOG_AUTO_TRACE();
   uint32_t connection_handle = 0;
diff --git a/src/components/connection_handler/src/heartbeat_monitor.cc b/src/components/connection_handler/src/heartbeat_monitor.cc
index f3e2da2696..8e15c934b9 100644
--- a/src/components/connection_handler/src/heartbeat_monitor.cc
+++ b/src/components/connection_handler/src/heartbeat_monitor.cc
@@ -87,6 +87,13 @@ void HeartBeatMonitor::threadMain() {
 }
 
 void HeartBeatMonitor::AddSession(uint8_t session_id) {
+  if (0 == default_heartbeat_timeout_) {
+    SDL_LOG_INFO("Won't add session with id "
+                 << static_cast<uint32_t>(session_id)
+                 << " because Heartbeat is disabled.");
+    return;
+  }
+
   const uint32_t converted_session_id = static_cast<int32_t>(session_id);
   UNUSED(converted_session_id);
   SDL_LOG_DEBUG("Add session with id " << converted_session_id);
@@ -96,6 +103,7 @@ void HeartBeatMonitor::AddSession(uint8_t session_id) {
                                      << " already exists");
     return;
   }
+
   sessions_.insert(
       std::make_pair(session_id, SessionState(default_heartbeat_timeout_)));
   SDL_LOG_INFO("Start heartbeat for session: " << converted_session_id);
@@ -125,6 +133,14 @@ void HeartBeatMonitor::KeepAlive(uint8_t session_id) {
   }
 }
 
+bool HeartBeatMonitor::IsSessionHeartbeatTracked(
+    const uint8_t session_id) const {
+  SDL_LOG_AUTO_TRACE();
+  AutoLock auto_lock(sessions_list_lock_);
+
+  return sessions_.end() != sessions_.find(session_id);
+}
+
 void HeartBeatMonitor::exitThreadMain() {
   // FIXME (dchmerev@luxoft.com): thread requested to stop should stop as soon
   // as possible,
diff --git a/src/components/include/connection_handler/connection_handler.h b/src/components/include/connection_handler/connection_handler.h
index c5995fbbb8..cb83bec564 100644
--- a/src/components/include/connection_handler/connection_handler.h
+++ b/src/components/include/connection_handler/connection_handler.h
@@ -173,6 +173,15 @@ class ConnectionHandler {
   virtual void SendEndService(uint32_t key, uint8_t service_type) = 0;
 
   /**
+   * @brief Check is heartbeat monitoring started for specified connection key
+   * @param  connection_key pair of connection and session id
+   * @return returns true if heartbeat monitoring started for specified
+   * connection key otherwise returns false
+   */
+  virtual bool IsSessionHeartbeatTracked(
+      const uint32_t connection_key) const = 0;
+
+  /**
    * \brief Start heartbeat for specified session
    *
    * \param connection_key pair of connection and session id
diff --git a/src/components/include/test/connection_handler/mock_connection_handler.h b/src/components/include/test/connection_handler/mock_connection_handler.h
index bf266751d4..78083b0abf 100644
--- a/src/components/include/test/connection_handler/mock_connection_handler.h
+++ b/src/components/include/test/connection_handler/mock_connection_handler.h
@@ -87,6 +87,8 @@ class MockConnectionHandler : public connection_handler::ConnectionHandler {
                     uint8_t session_id,
                     CloseSessionReason close_reason));
   MOCK_METHOD2(SendEndService, void(uint32_t key, uint8_t service_type));
+  MOCK_CONST_METHOD1(IsSessionHeartbeatTracked,
+                     bool(const uint32_t connection_key));
   MOCK_METHOD1(StartSessionHeartBeat, void(uint32_t connection_key));
   MOCK_METHOD2(SendHeartBeat,
                void(ConnectionHandle connection_handle, uint8_t session_id));
diff --git a/src/components/protocol_handler/src/protocol_handler_impl.cc b/src/components/protocol_handler/src/protocol_handler_impl.cc
index c2fd2db4f3..df499cd593 100644
--- a/src/components/protocol_handler/src/protocol_handler_impl.cc
+++ b/src/components/protocol_handler/src/protocol_handler_impl.cc
@@ -418,6 +418,11 @@ void ProtocolHandlerImpl::SendStartSessionAck(
   raw_ford_messages_to_mobile_.PostMessage(
       impl::RawFordMessageToMobile(ptr, false));
 
+  const uint32_t connection_key =
+      session_observer_.KeyFromPair(connection_id, session_id);
+  connection_handler_.BindProtocolVersionWithSession(connection_key,
+                                                     ack_protocol_version);
+
   SDL_LOG_DEBUG("SendStartSessionAck() for connection "
                 << connection_id << " for service_type "
                 << static_cast<int32_t>(service_type) << " session_id "
@@ -2044,16 +2049,23 @@ void ProtocolHandlerImpl::NotifySessionStarted(
 RESULT_CODE ProtocolHandlerImpl::HandleControlMessageHeartBeat(
     const ProtocolPacket& packet) {
   const ConnectionID connection_id = packet.connection_id();
+  const uint32_t session_id = packet.session_id();
   SDL_LOG_DEBUG("Sending heart beat acknowledgment for connection "
-                << connection_id);
+                << connection_id << " session " << session_id);
   uint8_t protocol_version;
   if (session_observer_.ProtocolVersionUsed(
-          connection_id, packet.session_id(), protocol_version)) {
+          connection_id, session_id, protocol_version)) {
     // TODO(EZamakhov): investigate message_id for HeartBeatAck
     if (protocol_version >= PROTOCOL_VERSION_3 &&
         protocol_version <= PROTOCOL_VERSION_5) {
-      return SendHeartBeatAck(
-          connection_id, packet.session_id(), packet.message_id());
+      const uint32_t connection_key =
+          session_observer_.KeyFromPair(connection_id, session_id);
+      if (!connection_handler_.IsSessionHeartbeatTracked(connection_key)) {
+        SDL_LOG_DEBUG("Session heartbeat tracking is not started. "
+                      << "Starting it for session " << session_id);
+        connection_handler_.StartSessionHeartBeat(connection_key);
+      }
+      return SendHeartBeatAck(connection_id, session_id, packet.message_id());
     } else {
       SDL_LOG_WARN("HeartBeat is not supported");
       return RESULT_HEARTBEAT_IS_NOT_SUPPORTED;
diff --git a/src/components/protocol_handler/test/protocol_handler_tm_test.cc b/src/components/protocol_handler/test/protocol_handler_tm_test.cc
index fa2fc07824..b28dee2657 100644
--- a/src/components/protocol_handler/test/protocol_handler_tm_test.cc
+++ b/src/components/protocol_handler/test/protocol_handler_tm_test.cc
@@ -2055,7 +2055,7 @@ void ProtocolHandlerImplTest::VerifySecondaryTransportParamsInStartSessionAck(
   AddSecurityManager();
 
   EXPECT_CALL(session_observer_mock, KeyFromPair(connection_id, session_id))
-      .WillOnce(Return(connection_key));
+      .WillRepeatedly(Return(connection_key));
 
   EXPECT_CALL(session_observer_mock, GetSSLContext(connection_key, kRpc))
       .WillOnce(ReturnNull());
@@ -2167,7 +2167,7 @@ void ProtocolHandlerImplTest::VerifyCloudAppParamsInStartSessionAck(
   AddSecurityManager();
 
   EXPECT_CALL(session_observer_mock, KeyFromPair(connection_id, session_id))
-      .WillOnce(Return(connection_key));
+      .WillRepeatedly(Return(connection_key));
 
   EXPECT_CALL(session_observer_mock, GetSSLContext(connection_key, kRpc))
       .WillOnce(ReturnNull());
@@ -2637,7 +2637,7 @@ TEST_F(ProtocolHandlerImplTest,
   AddSecurityManager();
 
   EXPECT_CALL(session_observer_mock, KeyFromPair(connection_id, session_id))
-      .WillOnce(Return(connection_key));
+      .WillRepeatedly(Return(connection_key));
 
   EXPECT_CALL(session_observer_mock, GetSSLContext(connection_key, kRpc))
       .WillOnce(ReturnNull());
@@ -2823,7 +2823,7 @@ TEST_F(ProtocolHandlerImplTest,
   AddSecurityManager();
 
   EXPECT_CALL(session_observer_mock, KeyFromPair(connection_id, session_id))
-      .WillOnce(Return(connection_key));
+      .WillRepeatedly(Return(connection_key));
 
   EXPECT_CALL(session_observer_mock, GetSSLContext(connection_key, kRpc))
       .WillOnce(ReturnNull());
@@ -2944,7 +2944,7 @@ TEST_F(ProtocolHandlerImplTest,
   AddSecurityManager();
 
   EXPECT_CALL(session_observer_mock, KeyFromPair(connection_id, session_id))
-      .WillOnce(Return(connection_key));
+      .WillRepeatedly(Return(connection_key));
 
   EXPECT_CALL(session_observer_mock, GetSSLContext(connection_key, kRpc))
       .WillOnce(ReturnNull());