Fix sdl sends unexpected disconnect with resource constraint (#3516)

* Rework processing of the OnExitApplication

(RESOURCE_CONSTRAINT) notification

* Update UTs according to changes

Co-authored-by: sniukalov <sniukaov@luxoft.com>
Co-authored-by: Aleksandr Kutsan <AKutsan@luxoft.com>
Co-authored-by: jacobkeeler <jacob.keeler@livioradio.com>
Co-authored-by: Andrii Kalinich <AKalinich@luxoft.com>

14 files changed, 211 insertions, 13 deletions

diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/on_exit_application_notification.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/on_exit_application_notification.cc
index 2b95d2e3e8..4a6a3df3b4 100644
--- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/on_exit_application_notification.cc
+++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/on_exit_application_notification.cc
@@ -119,7 +119,6 @@ void OnExitApplicationNotification::Run() {
           MessageHelper::GetOnAppInterfaceUnregisteredNotificationToMobile(
               app_id, AppInterfaceUnregisteredReason::RESOURCE_CONSTRAINT);
       SendNotificationToMobile(message);
-      application_manager_.UnregisterApplication(app_id, Result::SUCCESS);
       return;
     }
 
diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/hmi/hmi_notifications_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/hmi/hmi_notifications_test.cc
index 3efdee405a..400481f506 100644
--- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/hmi/hmi_notifications_test.cc
+++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/hmi/hmi_notifications_test.cc
@@ -1028,14 +1028,12 @@ TEST_F(HMICommandsNotificationsTest,
   using ExitReason = hmi_apis::Common_ApplicationExitReason::eType;
   std::vector<ExitReason> reason_list = {
       ExitReason::UNAUTHORIZED_TRANSPORT_REGISTRATION,
-      ExitReason::UNSUPPORTED_HMI_RESOURCE,
-      ExitReason::RESOURCE_CONSTRAINT};
+      ExitReason::UNSUPPORTED_HMI_RESOURCE};
 
   using UnregisteredReason = mobile_apis::AppInterfaceUnregisteredReason::eType;
   std::vector<UnregisteredReason> mobile_reason_list = {
       UnregisteredReason::APP_UNAUTHORIZED,
-      UnregisteredReason::UNSUPPORTED_HMI_RESOURCE,
-      UnregisteredReason::RESOURCE_CONSTRAINT};
+      UnregisteredReason::UNSUPPORTED_HMI_RESOURCE};
 
   std::vector<mobile_apis::AppInterfaceUnregisteredReason::eType>::iterator
       it_mobile_reason = mobile_reason_list.begin();
@@ -1069,6 +1067,47 @@ TEST_F(HMICommandsNotificationsTest,
 }
 
 TEST_F(HMICommandsNotificationsTest,
+       OnExitApplicationNotificationResourceConstraintReason) {
+  auto message = CreateMessage();
+  (*message)[am::strings::msg_params][am::strings::app_id] = kAppId_;
+  const auto notification = std::make_shared<smart_objects::SmartObject>();
+  (*notification)[am::strings::params][am::strings::function_id] =
+      static_cast<int32_t>(
+          mobile_apis::FunctionID::OnAppInterfaceUnregisteredID);
+  (*notification)[am::strings::params][am::strings::message_type] =
+      static_cast<int32_t>(am::MessageType::kNotification);
+  (*notification)[am::strings::params][am::strings::connection_key] = kAppId_;
+
+  using ExitReason = hmi_apis::Common_ApplicationExitReason::eType;
+  auto hmi_reason = ExitReason::RESOURCE_CONSTRAINT;
+
+  using UnregisteredReason = mobile_apis::AppInterfaceUnregisteredReason::eType;
+  auto mobile_reason = UnregisteredReason::RESOURCE_CONSTRAINT;
+
+  (*message)[am::strings::msg_params][am::strings::reason] = hmi_reason;
+  const auto command = CreateCommand<OnExitApplicationNotification>(message);
+
+  (*notification)[am::strings::msg_params][am::strings::reason] =
+      static_cast<int32_t>(mobile_reason);
+
+  am::plugin_manager::MockRPCPluginManager mock_rpc_plugin_manager_;
+  EXPECT_CALL(app_mngr_, GetPluginManager())
+      .WillRepeatedly(ReturnRef(mock_rpc_plugin_manager_));
+
+  EXPECT_CALL(app_mngr_, application(kAppId_)).WillRepeatedly(Return(app_));
+  EXPECT_CALL(
+      mock_message_helper_,
+      GetOnAppInterfaceUnregisteredNotificationToMobile(kAppId_, mobile_reason))
+      .WillOnce(Return(notification));
+  EXPECT_CALL(mock_rpc_service_,
+              ManageMobileCommand(notification, Command::SOURCE_SDL));
+  EXPECT_CALL(app_mngr_, UnregisterApplication(_, _, _, _)).Times(0);
+
+  ASSERT_TRUE(command->Init());
+  command->Run();
+}
+
+TEST_F(HMICommandsNotificationsTest,
        OnExitApplicationNotificationUnhandledReason) {
   MessageSharedPtr message = CreateMessage();
   (*message)[am::strings::msg_params][am::strings::app_id] = kAppId_;
diff --git a/src/components/application_manager/src/application_manager_impl.cc b/src/components/application_manager/src/application_manager_impl.cc
index 26557163d0..6b0ea39804 100644
--- a/src/components/application_manager/src/application_manager_impl.cc
+++ b/src/components/application_manager/src/application_manager_impl.cc
@@ -2181,6 +2181,12 @@ void ApplicationManagerImpl::OnServiceEndedCallback(
         is_unexpected_disconnect = false;
         break;
       }
+      case CloseSessionReason::kFinalMessage: {
+        reason = Result::SUCCESS;
+        is_resuming = false;
+        is_unexpected_disconnect = false;
+        break;
+      }
       default: {
         reason = Result::INVALID_ENUM;
         is_resuming = true;
diff --git a/src/components/application_manager/src/rpc_service_impl.cc b/src/components/application_manager/src/rpc_service_impl.cc
index 248db00729..34a0bed7af 100644
--- a/src/components/application_manager/src/rpc_service_impl.cc
+++ b/src/components/application_manager/src/rpc_service_impl.cc
@@ -467,8 +467,8 @@ void RPCServiceImpl::Handle(const impl::MessageToMobile message) {
   SDL_LOG_INFO("Message for mobile given away");
 
   if (close_session) {
-    app_manager_.connection_handler().CloseSession(message->connection_key(),
-                                                   connection_handler::kCommon);
+    app_manager_.connection_handler().CloseSession(
+        message->connection_key(), connection_handler::kFinalMessage);
   }
 }
 
diff --git a/src/components/connection_handler/include/connection_handler/connection.h b/src/components/connection_handler/include/connection_handler/connection.h
index 1a93509aca..8b286e1309 100644
--- a/src/components/connection_handler/include/connection_handler/connection.h
+++ b/src/components/connection_handler/include/connection_handler/connection.h
@@ -177,6 +177,17 @@ class Connection {
   uint32_t RemoveSession(uint8_t session_id);
 
   /**
+   * @brief Called upon final message being sent for a session
+   */
+  void OnFinalMessageCallback();
+
+  /**
+   * @brief Check whether final message was sent from this connection
+   * @return true if final message was sent by any session of this connection
+   */
+  bool IsFinalMessageSent() const;
+
+  /**
    * @brief Adds uprotected service to session or
    * check protection to service has been started before
    * @param session_id session ID
@@ -378,6 +389,7 @@ class Connection {
   HeartBeatMonitor* heartbeat_monitor_;
   uint32_t heartbeat_timeout_;
   threads::Thread* heart_beat_monitor_thread_;
+  bool final_message_sent_;
 
   DISALLOW_COPY_AND_ASSIGN(Connection);
 };
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 08b64ec9b8..3bfa5b34b8 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
@@ -35,6 +35,7 @@
 
 #include <list>
 #include <map>
+#include <set>
 #include <string>
 #include <vector>
 
@@ -272,6 +273,8 @@ class ConnectionHandlerImpl
    */
   void OnMalformedMessageCallback(const uint32_t& connection_key) OVERRIDE;
 
+  void OnFinalMessageCallback(const uint32_t& connection_key) OVERRIDE;
+
   /**
    * @brief Converts connection handle to transport type string used in
    * smartDeviceLink.ini file, e.g. "TCP_WIFI"
@@ -646,6 +649,10 @@ class ConnectionHandlerImpl
    **/
   void RemoveConnection(const ConnectionHandle connection_handle);
 
+  /**
+   * @brief Called when connection is closed.
+   * @param connection_id Connection unique identifier.
+   */
   void OnConnectionEnded(const transport_manager::ConnectionUID connection_id);
 
   const uint8_t GetSessionIdFromSecondaryTransport(
diff --git a/src/components/connection_handler/src/connection.cc b/src/components/connection_handler/src/connection.cc
index 03915516c3..5bd72773c2 100644
--- a/src/components/connection_handler/src/connection.cc
+++ b/src/components/connection_handler/src/connection.cc
@@ -82,7 +82,8 @@ Connection::Connection(ConnectionHandle connection_handle,
     , connection_handle_(connection_handle)
     , connection_device_handle_(connection_device_handle)
     , primary_connection_handle_(0)
-    , heartbeat_timeout_(heartbeat_timeout) {
+    , heartbeat_timeout_(heartbeat_timeout)
+    , final_message_sent_(false) {
   SDL_LOG_AUTO_TRACE();
   DCHECK(connection_handler_);
 
@@ -162,6 +163,15 @@ uint32_t Connection::RemoveSession(uint8_t session_id) {
   return session_id;
 }
 
+void Connection::OnFinalMessageCallback() {
+  SDL_LOG_AUTO_TRACE();
+  final_message_sent_ = true;
+}
+
+bool Connection::IsFinalMessageSent() const {
+  return final_message_sent_;
+}
+
 bool Connection::AddNewService(uint8_t session_id,
                                protocol_handler::ServiceType service_type,
                                const bool request_protection,
diff --git a/src/components/connection_handler/src/connection_handler_impl.cc b/src/components/connection_handler/src/connection_handler_impl.cc
index af32a741e1..549450dbe0 100644
--- a/src/components/connection_handler/src/connection_handler_impl.cc
+++ b/src/components/connection_handler/src/connection_handler_impl.cc
@@ -366,7 +366,7 @@ void ConnectionHandlerImpl::OnUnexpectedDisconnect(
     transport_manager::ConnectionUID connection_id,
     const transport_manager::CommunicationError& error) {
   SDL_LOG_AUTO_TRACE();
-
+  UNUSED(error);
   OnConnectionEnded(connection_id);
 }
 
@@ -675,6 +675,25 @@ void ConnectionHandlerImpl::OnMalformedMessageCallback(
   CloseConnection(connection_handle);
 }
 
+void ConnectionHandlerImpl::OnFinalMessageCallback(
+    const uint32_t& connection_key) {
+  SDL_LOG_AUTO_TRACE();
+
+  transport_manager::ConnectionUID connection_handle = 0;
+  uint8_t session_id = 0;
+  PairFromKey(connection_key, &connection_handle, &session_id);
+
+  sync_primitives::AutoWriteLock connection_list_lock(connection_list_lock_);
+  ConnectionList::iterator connection_it =
+      connection_list_.find(connection_handle);
+
+  if (connection_list_.end() != connection_it) {
+    SDL_LOG_DEBUG("OnFinalMessageCallback found connection "
+                  << connection_handle);
+    connection_it->second->OnFinalMessageCallback();
+  }
+}
+
 uint32_t ConnectionHandlerImpl::OnSessionEndedCallback(
     const transport_manager::ConnectionUID connection_handle,
     const uint8_t session_id,
@@ -1716,6 +1735,10 @@ void ConnectionHandlerImpl::OnConnectionEnded(
     ending_connection_ = connection.get();
     const SessionMap session_map = connection->session_map();
 
+    const CloseSessionReason close_reason =
+        connection->IsFinalMessageSent() ? CloseSessionReason::kFinalMessage
+                                         : CloseSessionReason::kCommon;
+
     for (SessionMap::const_iterator session_it = session_map.begin();
          session_map.end() != session_it;
          ++session_it) {
@@ -1731,9 +1754,7 @@ void ConnectionHandlerImpl::OnConnectionEnded(
           service_list.rbegin();
       for (; service_list_itr != service_list.rend(); ++service_list_itr) {
         connection_handler_observer_->OnServiceEndedCallback(
-            session_key,
-            service_list_itr->service_type,
-            CloseSessionReason::kCommon);
+            session_key, service_list_itr->service_type, close_reason);
       }
     }
     ending_connection_ = NULL;
diff --git a/src/components/connection_handler/test/connection_handler_impl_test.cc b/src/components/connection_handler/test/connection_handler_impl_test.cc
index 3fbdf62474..9305eee2ee 100644
--- a/src/components/connection_handler/test/connection_handler_impl_test.cc
+++ b/src/components/connection_handler/test/connection_handler_impl_test.cc
@@ -855,6 +855,24 @@ TEST_F(ConnectionHandlerTest, OnConnectionClosed) {
   connection_handler_->OnConnectionClosed(uid_);
 }
 
+TEST_F(ConnectionHandlerTest, OnFinalMessageCallback_OnConnectionClosed) {
+  AddTestDeviceConnection();
+  AddTestSession();
+
+  connection_handler_test::MockConnectionHandlerObserver
+      mock_connection_handler_observer;
+  connection_handler_->set_connection_handler_observer(
+      &mock_connection_handler_observer);
+
+  EXPECT_CALL(mock_connection_handler_observer,
+              OnServiceEndedCallback(connection_key_, kBulk, kFinalMessage));
+  EXPECT_CALL(mock_connection_handler_observer,
+              OnServiceEndedCallback(connection_key_, kRpc, kFinalMessage));
+
+  connection_handler_->OnFinalMessageCallback(connection_key_);
+  connection_handler_->OnConnectionClosed(uid_);
+}
+
 TEST_F(ConnectionHandlerTest, OnUnexpectedDisconnect) {
   AddTestDeviceConnection();
   AddTestSession();
@@ -874,6 +892,27 @@ TEST_F(ConnectionHandlerTest, OnUnexpectedDisconnect) {
   connection_handler_->OnUnexpectedDisconnect(uid_, err);
 }
 
+TEST_F(ConnectionHandlerTest, OnFinalMessageCallback_OnUnexpectedDisconnect) {
+  AddTestDeviceConnection();
+  AddTestSession();
+
+  connection_handler_test::MockConnectionHandlerObserver
+      mock_connection_handler_observer;
+  connection_handler_->set_connection_handler_observer(
+      &mock_connection_handler_observer);
+
+  EXPECT_CALL(mock_connection_handler_observer,
+              OnServiceEndedCallback(
+                  connection_key_, kBulk, CloseSessionReason::kFinalMessage));
+  EXPECT_CALL(mock_connection_handler_observer,
+              OnServiceEndedCallback(
+                  connection_key_, kRpc, CloseSessionReason::kFinalMessage));
+
+  connection_handler_->OnFinalMessageCallback(connection_key_);
+  transport_manager::CommunicationError err;
+  connection_handler_->OnUnexpectedDisconnect(uid_, err);
+}
+
 TEST_F(ConnectionHandlerTest, ConnectToDevice) {
   // Precondition
   const uint32_t dev_handle1 = 1;
diff --git a/src/components/include/connection_handler/connection_handler.h b/src/components/include/connection_handler/connection_handler.h
index c5f8c175e6..c5995fbbb8 100644
--- a/src/components/include/connection_handler/connection_handler.h
+++ b/src/components/include/connection_handler/connection_handler.h
@@ -48,7 +48,13 @@
  */
 namespace connection_handler {
 
-enum CloseSessionReason { kCommon = 0, kFlood, kMalformed, kUnauthorizedApp };
+enum CloseSessionReason {
+  kCommon = 0,
+  kFlood,
+  kMalformed,
+  kUnauthorizedApp,
+  kFinalMessage
+};
 
 class ConnectionHandlerObserver;
 
diff --git a/src/components/include/protocol_handler/session_observer.h b/src/components/include/protocol_handler/session_observer.h
index cdf4267188..593ce8408c 100644
--- a/src/components/include/protocol_handler/session_observer.h
+++ b/src/components/include/protocol_handler/session_observer.h
@@ -179,6 +179,13 @@ class SessionObserver {
   virtual void OnMalformedMessageCallback(const uint32_t& connection_key) = 0;
 
   /**
+   * @brief Callback function used by ProtocolHandler when the last message was
+   * sent for a mobile connection
+   * @param connection_key used by other components as an application identifier
+   */
+  virtual void OnFinalMessageCallback(const uint32_t& connection_key) = 0;
+
+  /**
    * @brief Converts connection handle to transport type string used in
    * smartDeviceLink.ini file, e.g. "TCP_WIFI"
    * @param connection_handle A connection identifier
diff --git a/src/components/include/test/protocol_handler/mock_session_observer.h b/src/components/include/test/protocol_handler/mock_session_observer.h
index 3414153fc7..6bd67f6516 100644
--- a/src/components/include/test/protocol_handler/mock_session_observer.h
+++ b/src/components/include/test/protocol_handler/mock_session_observer.h
@@ -83,6 +83,7 @@ class MockSessionObserver : public ::protocol_handler::SessionObserver {
                void(const uint32_t& connection_key));
   MOCK_METHOD1(OnMalformedMessageCallback,
                void(const uint32_t& connection_key));
+  MOCK_METHOD1(OnFinalMessageCallback, void(const uint32_t& connection_key));
   MOCK_CONST_METHOD1(
       TransportTypeProfileStringFromConnHandle,
       const std::string(transport_manager::ConnectionUID connection_handle));
diff --git a/src/components/protocol_handler/src/protocol_handler_impl.cc b/src/components/protocol_handler/src/protocol_handler_impl.cc
index 7d83d6cd85..1db599e656 100644
--- a/src/components/protocol_handler/src/protocol_handler_impl.cc
+++ b/src/components/protocol_handler/src/protocol_handler_impl.cc
@@ -1069,6 +1069,7 @@ void ProtocolHandlerImpl::OnTMMessageSend(const RawMessagePtr message) {
 
   if (ready_to_close_connections_.end() != connection_it) {
     ready_to_close_connections_.erase(connection_it);
+    session_observer_.OnFinalMessageCallback(connection_handle);
     transport_manager_.Disconnect(connection_handle);
     return;
   }
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 6edb77d0cd..41c16817b5 100644
--- a/src/components/protocol_handler/test/protocol_handler_tm_test.cc
+++ b/src/components/protocol_handler/test/protocol_handler_tm_test.cc
@@ -3550,6 +3550,56 @@ TEST_F(ProtocolHandlerImplTest, MalformedLimitVerification_NullCount) {
   EXPECT_TRUE(waiter->WaitFor(times, kAsyncExpectationsTimeout));
 }
 
+TEST_F(ProtocolHandlerImplTest, OnFinalMessageCallback) {
+  const auto protection = false;
+  const auto is_final_message = true;
+  const uint32_t total_data_size = 1;
+  UCharDataVector data(total_data_size);
+
+  ProtocolFramePtr ptr(new protocol_handler::ProtocolPacket(connection_id,
+                                                            PROTOCOL_VERSION_3,
+                                                            protection,
+                                                            FRAME_TYPE_SINGLE,
+                                                            kControl,
+                                                            FRAME_DATA_SINGLE,
+                                                            session_id,
+                                                            total_data_size,
+                                                            message_id,
+                                                            &data[0]));
+
+  using RawFordMessageToMobile = protocol_handler::impl::RawFordMessageToMobile;
+  using Handler = protocol_handler::impl::ToMobileQueue::Handler;
+
+  auto message =
+      std::make_shared<RawFordMessageToMobile>(ptr, is_final_message);
+  auto raw_message = ptr->serializePacket();
+
+  auto handler = std::static_pointer_cast<Handler>(protocol_handler_impl);
+  auto transport_manager_listener =
+      std::static_pointer_cast<TransportManagerListener>(protocol_handler_impl);
+
+  EXPECT_CALL(session_observer_mock,
+              ProtocolVersionUsed(connection_id, session_id, An<uint8_t&>()))
+      .WillRepeatedly(Return(false));
+
+  EXPECT_CALL(session_observer_mock,
+              PairFromKey(raw_message->connection_key(), _, _))
+      .WillRepeatedly(
+          DoAll(SetArgPointee<1>(connection_id), SetArgPointee<2>(session_id)));
+
+  EXPECT_CALL(transport_manager_mock, SendMessageToDevice(_))
+      .WillOnce(Return(E_SUCCESS));
+
+  EXPECT_CALL(session_observer_mock, OnFinalMessageCallback(connection_id));
+  EXPECT_CALL(transport_manager_mock, Disconnect(connection_id));
+
+  handler->Handle(*message);
+  // Put the message to list of connections ready to close
+  transport_manager_listener->OnTMMessageSend(raw_message);
+  // Processing the list of connections ready to close
+  transport_manager_listener->OnTMMessageSend(raw_message);
+}
+
 TEST_F(ProtocolHandlerImplTest,
        SendEndServicePrivate_NoConnection_MessageNotSent) {
   // Expect check connection with ProtocolVersionUsed