diff options
author | JackLivio <jack@livio.io> | 2020-11-13 10:09:22 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-11-13 10:09:22 -0800 |
commit | 7871541192bab4a9658d9c0ef90a513edf660a6d (patch) | |
tree | 37ad57d57664045b950d9148b16b595254522489 | |
parent | 43e539e57f7459ce7ab5c683af5580728f7eff49 (diff) | |
parent | bd51e85a28b70f3f2dc44ed572b405b1017dfef9 (diff) | |
download | sdl_core-7871541192bab4a9658d9c0ef90a513edf660a6d.tar.gz |
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()); |