Fix SDL crash during application registration (#3115)

* Add the new ApplicationEvent.

Add the new ApplicationEvent (kRCStatusChanged) for informing RcRPCPlugin
that OnRCStatus notification should be sent.
Split adding RcExtension to application and sending OnRCStatus notification
to the separate actions.

* Fix SDL crash during application registration.

There is an issue when SDL crashes during registration second application
in the time when already exists a registered other one application.
In the current implementation ApplicationManager during application
registering by the ApplicationManager::RegisterApplication method creates
a new instance of the application and adds it to the collection of all
registered applications. This method calls in the
RegisterAppInterfaceRequest, but all needed extensions will be added only
by the ApplicationManager::OnApplicationRegistered method during sending
RegisterAppInterface Response to the mobile.
According to the described behavior could appear case when a first
application is registered successfully and all needed application
extensions are successfully added too. The second application starts
the registration process in someone thread, but in the other one thread,
SDL receives PolicyEvent::kApplicationPolicyUpdated event and tries to
unsubscribe all applications from removed in policy table Vehicle Data
Items. All actions go on in the next sequence:

Precondition:
The first application is successfully registered and all needed
extensions are added.

The first thread:
1. The Mobile device sends RegisterAppInterfaceRequest RPC to the SDL for
registration of the second application.
2. SDL receives the RegisterAppInterfaceRequest and calls method Run.
Within method RegisterAppInterfaceRequest::Run ApplicationManager runs
3. the method RegisterApplication.

The second thread:
4. The SDL receives the PolicyEvent::kApplicationPolicyUpdated event and
forwards the event to each plugin for processing it.
5. VehicleInfoPlugin tries to unsubscribe all registered applications
from removed in policy table Vehicle Data Items.
The plugin finds the first application and unsubscribes it from all
removed Vehicle Data Items.
The plugin finds the second application and doesn't find any extensions
for the application and crashes on the DCHECK with an uninitialized
pointer to the extension.

11 files changed, 80 insertions, 17 deletions

diff --git a/src/components/application_manager/include/application_manager/application_manager_impl.h b/src/components/application_manager/include/application_manager/application_manager_impl.h
index 99264d6ef6..c0d12c526f 100644
--- a/src/components/application_manager/include/application_manager/application_manager_impl.h
+++ b/src/components/application_manager/include/application_manager/application_manager_impl.h
@@ -1138,8 +1138,19 @@ class ApplicationManagerImpl
    */
   protocol_handler::MajorProtocolVersion SupportedSDLVersion() const OVERRIDE;
 
+  void ApplyFunctorForEachPlugin(
+      std::function<void(plugin_manager::RPCPlugin&)> functor) OVERRIDE;
+
  private:
   /**
+   * @brief Adds application to registered applications list and marks it as
+   * registered
+   * @param application Application that should be added to registered
+   * applications list.
+   */
+  void AddAppToRegisteredAppList(const ApplicationSharedPtr application);
+
+  /**
    * @brief Removes service status record for service that failed to start
    * @param app Application whose service status record should be removed
    * @param Service type which status record should be removed
diff --git a/src/components/application_manager/include/application_manager/plugin_manager/rpc_plugin.h b/src/components/application_manager/include/application_manager/plugin_manager/rpc_plugin.h
index 61b146f024..3af87a0c9e 100644
--- a/src/components/application_manager/include/application_manager/plugin_manager/rpc_plugin.h
+++ b/src/components/application_manager/include/application_manager/plugin_manager/rpc_plugin.h
@@ -65,7 +65,8 @@ enum ApplicationEvent {
   kApplicationRegistered,
   kApplicationUnregistered,
   kDeleteApplicationData,
-  kGlobalPropertiesUpdated
+  kGlobalPropertiesUpdated,
+  kRCStatusChanged
 };
 
 class RPCPlugin {
diff --git a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/rc_helpers.cc b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/rc_helpers.cc
index ca0edc90b1..f344dd15a7 100644
--- a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/rc_helpers.cc
+++ b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/rc_helpers.cc
@@ -175,6 +175,7 @@ const std::vector<std::string> RCHelpers::GetModuleTypesList() {
 
 RCAppExtensionPtr RCHelpers::GetRCExtension(
     application_manager::Application& app) {
+  LOG4CXX_AUTO_TRACE(logger_);
   auto extension_interface = app.QueryInterface(RCRPCPlugin::kRCPluginID);
   auto extension =
       std::static_pointer_cast<RCAppExtension>(extension_interface);
diff --git a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/rc_rpc_plugin.cc b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/rc_rpc_plugin.cc
index 9005eb7fd0..4e43cff58d 100644
--- a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/rc_rpc_plugin.cc
+++ b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/rc_rpc_plugin.cc
@@ -107,7 +107,12 @@ void RCRPCPlugin::OnPolicyEvent(
 void RCRPCPlugin::OnApplicationEvent(
     application_manager::plugin_manager::ApplicationEvent event,
     application_manager::ApplicationSharedPtr application) {
+  LOG4CXX_AUTO_TRACE(logger_);
   if (!application->is_remote_control_supported()) {
+    LOG4CXX_DEBUG(
+        logger_,
+        "Remote control is not supported for application with app_id: "
+            << application->app_id());
     return;
   }
   switch (event) {
@@ -119,8 +124,6 @@ void RCRPCPlugin::OnApplicationEvent(
           rc_capabilities_manager_
               ->GetDriverLocationFromSeatLocationCapability();
       extension->SetUserLocation(driver_location);
-      resource_allocation_manager_->SendOnRCStatusNotifications(
-          NotificationTrigger::APP_REGISTRATION, application);
       break;
     }
     case plugins::kApplicationExit: {
@@ -139,6 +142,11 @@ void RCRPCPlugin::OnApplicationEvent(
       extension->SetUserLocation(user_location);
       break;
     }
+    case plugins::kRCStatusChanged: {
+      resource_allocation_manager_->SendOnRCStatusNotifications(
+          NotificationTrigger::APP_REGISTRATION, application);
+      break;
+    }
     default:
       break;
   }
diff --git a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/resource_allocation_manager_impl.cc b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/resource_allocation_manager_impl.cc
index 97ff2b23da..049f9a3cf3 100644
--- a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/resource_allocation_manager_impl.cc
+++ b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/resource_allocation_manager_impl.cc
@@ -156,6 +156,7 @@ bool ResourceAllocationManagerImpl::IsUserLocationValid(
     ModuleUid& module, application_manager::ApplicationSharedPtr app) {
   LOG4CXX_AUTO_TRACE(logger_);
   const auto extension = RCHelpers::GetRCExtension(*app);
+  DCHECK_OR_RETURN(extension, false);
   const auto user_location = extension->GetUserLocation();
   const auto module_service_area =
       rc_capabilities_manager_.GetModuleServiceArea(module);
diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/register_app_interface_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/register_app_interface_request.cc
index 14a94fd1a2..b9ac00aef4 100644
--- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/register_app_interface_request.cc
+++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/register_app_interface_request.cc
@@ -43,6 +43,7 @@
 #include "application_manager/application_manager.h"
 #include "application_manager/helpers/application_helper.h"
 #include "application_manager/message_helper.h"
+#include "application_manager/plugin_manager/plugin_keys.h"
 #include "application_manager/policies/policy_handler.h"
 #include "application_manager/policies/policy_handler_interface.h"
 #include "application_manager/resumption/resume_ctrl.h"
@@ -458,6 +459,12 @@ void RegisterAppInterfaceRequest::Run() {
     }
   }
 
+  auto on_app_registered = [application](plugin_manager::RPCPlugin& plugin) {
+    plugin.OnApplicationEvent(plugin_manager::kApplicationRegistered,
+                              application);
+  };
+  application_manager_.ApplyFunctorForEachPlugin(on_app_registered);
+
   if (msg_params.keyExists(strings::day_color_scheme)) {
     application->set_day_color_scheme(msg_params[strings::day_color_scheme]);
   }
@@ -875,6 +882,12 @@ void RegisterAppInterfaceRequest::SendRegisterAppInterfaceResponseToMobile(
   // Default HMI level should be set before any permissions validation, since it
   // relies on HMI level.
   application_manager_.OnApplicationRegistered(application);
+
+  auto send_rc_status = [application](plugin_manager::RPCPlugin& plugin) {
+    plugin.OnApplicationEvent(plugin_manager::kRCStatusChanged, application);
+  };
+  application_manager_.ApplyFunctorForEachPlugin(send_rc_status);
+
   SendOnAppRegisteredNotificationToHMI(
       application, resumption, need_restore_vr);
   (*notify_upd_manager)();
diff --git a/src/components/application_manager/rpc_plugins/vehicle_info_plugin/src/vehicle_info_plugin.cc b/src/components/application_manager/rpc_plugins/vehicle_info_plugin/src/vehicle_info_plugin.cc
index 40da7501c1..3017b6712f 100644
--- a/src/components/application_manager/rpc_plugins/vehicle_info_plugin/src/vehicle_info_plugin.cc
+++ b/src/components/application_manager/rpc_plugins/vehicle_info_plugin/src/vehicle_info_plugin.cc
@@ -98,6 +98,7 @@ void VehicleInfoPlugin::OnApplicationEvent(
 }
 
 void VehicleInfoPlugin::UnsubscribeFromRemovedVDItems() {
+  LOG4CXX_AUTO_TRACE(logger_);
   typedef std::vector<std::string> StringsVector;
 
   auto get_items_to_unsubscribe = [this]() -> StringsVector {
diff --git a/src/components/application_manager/src/application_manager_impl.cc b/src/components/application_manager/src/application_manager_impl.cc
index d6e673b5b5..d67ffbe348 100644
--- a/src/components/application_manager/src/application_manager_impl.cc
+++ b/src/components/application_manager/src/application_manager_impl.cc
@@ -448,12 +448,6 @@ void ApplicationManagerImpl::OnApplicationRegistered(ApplicationSharedPtr app) {
   sync_primitives::AutoLock lock(applications_list_lock_ptr_);
   const mobile_apis::HMILevel::eType default_level = GetDefaultHmiLevel(app);
   state_ctrl_.OnApplicationRegistered(app, default_level);
-
-  std::function<void(plugin_manager::RPCPlugin&)> on_app_registered =
-      [app](plugin_manager::RPCPlugin& plugin) {
-        plugin.OnApplicationEvent(plugin_manager::kApplicationRegistered, app);
-      };
-  plugin_manager_->ForEachPlugin(on_app_registered);
 }
 
 void ApplicationManagerImpl::OnApplicationSwitched(ApplicationSharedPtr app) {
@@ -743,14 +737,7 @@ ApplicationSharedPtr ApplicationManagerImpl::RegisterApplication(
   // Timer will be started after hmi level resumption.
   resume_controller().OnAppRegistrationStart(policy_app_id, device_mac);
 
-  // Add application to registered app list and set appropriate mark.
-  // Lock has to be released before adding app to policy DB to avoid possible
-  // deadlock with simultaneous PTU processing
-  applications_list_lock_ptr_->Acquire();
-  application->MarkRegistered();
-  applications_.insert(application);
-  apps_size_ = applications_.size();
-  applications_list_lock_ptr_->Release();
+  AddAppToRegisteredAppList(application);
 
   // Update cloud app information, in case any pending apps are unable to be
   // registered due to a mobile app taking precedence
@@ -4190,6 +4177,28 @@ ApplicationManagerImpl::SupportedSDLVersion() const {
       get_settings().max_supported_protocol_version());
 }
 
+void ApplicationManagerImpl::AddAppToRegisteredAppList(
+    const ApplicationSharedPtr application) {
+  LOG4CXX_AUTO_TRACE(logger_);
+  DCHECK_OR_RETURN_VOID(application);
+  sync_primitives::AutoLock lock(applications_list_lock_ptr_);
+
+  // Add application to registered app list and set appropriate mark.
+  application->MarkRegistered();
+  applications_.insert(application);
+  LOG4CXX_DEBUG(
+      logger_,
+      "App with app_id: " << application->app_id()
+                          << " has been added to registered applications list");
+  apps_size_ = static_cast<uint32_t>(applications_.size());
+}
+
+void ApplicationManagerImpl::ApplyFunctorForEachPlugin(
+    std::function<void(plugin_manager::RPCPlugin&)> functor) {
+  LOG4CXX_AUTO_TRACE(logger_);
+  plugin_manager_->ForEachPlugin(functor);
+}
+
 event_engine::EventDispatcher& ApplicationManagerImpl::event_dispatcher() {
   return event_dispatcher_;
 }
diff --git a/src/components/application_manager/test/application_manager_impl_test.cc b/src/components/application_manager/test/application_manager_impl_test.cc
index 152d577cda..aa82c6b86b 100644
--- a/src/components/application_manager/test/application_manager_impl_test.cc
+++ b/src/components/application_manager/test/application_manager_impl_test.cc
@@ -90,6 +90,7 @@ using ::testing::SaveArg;
 using ::testing::SetArgPointee;
 using ::testing::SetArgReferee;
 
+using application_manager::plugin_manager::MockRPCPluginManager;
 using test::components::application_manager_test::MockStateController;
 using test::components::policy_test::MockPolicyHandlerInterface;
 
@@ -1607,6 +1608,9 @@ TEST_F(ApplicationManagerImplTest,
   smart_objects::SmartObjectSPtr request_for_registration_ptr =
       std::make_shared<smart_objects::SmartObject>(request_for_registration);
 
+  std::unique_ptr<plugin_manager::RPCPluginManager> rpc_plugin_manager(
+      new MockRPCPluginManager());
+  app_manager_impl_->SetPluginManager(rpc_plugin_manager);
   ApplicationSharedPtr application =
       app_manager_impl_->RegisterApplication(request_for_registration_ptr);
   EXPECT_STREQ(kAppName.c_str(), application->name().c_str());
@@ -1774,6 +1778,9 @@ TEST_F(ApplicationManagerImplTest,
   smart_objects::SmartObjectSPtr request_for_registration_ptr =
       std::make_shared<smart_objects::SmartObject>(request_for_registration);
 
+  std::unique_ptr<plugin_manager::RPCPluginManager> rpc_plugin_manager(
+      new MockRPCPluginManager());
+  app_manager_impl_->SetPluginManager(rpc_plugin_manager);
   ApplicationSharedPtr application =
       app_manager_impl_->RegisterApplication(request_for_registration_ptr);
 
diff --git a/src/components/include/application_manager/application_manager.h b/src/components/include/application_manager/application_manager.h
index 8369d494f4..df50c43e52 100644
--- a/src/components/include/application_manager/application_manager.h
+++ b/src/components/include/application_manager/application_manager.h
@@ -666,6 +666,13 @@ class ApplicationManager {
   virtual protocol_handler::MajorProtocolVersion SupportedSDLVersion()
       const = 0;
 
+  /**
+   * @brief Applies functor for each plugin
+   * @param functor Functor that will be applied to each plugin
+   */
+  virtual void ApplyFunctorForEachPlugin(
+      std::function<void(plugin_manager::RPCPlugin&)> functor) = 0;
+
   /*
    * @brief Converts connection string transport type representation
    * to HMI Common_TransportType
diff --git a/src/components/include/test/application_manager/mock_application_manager.h b/src/components/include/test/application_manager/mock_application_manager.h
index cb230805fb..22f3ad1219 100644
--- a/src/components/include/test/application_manager/mock_application_manager.h
+++ b/src/components/include/test/application_manager/mock_application_manager.h
@@ -271,6 +271,10 @@ class MockApplicationManager : public application_manager::ApplicationManager {
   MOCK_CONST_METHOD0(SupportedSDLVersion,
                      protocol_handler::MajorProtocolVersion());
   MOCK_METHOD1(
+      ApplyFunctorForEachPlugin,
+      void(std::function<void(application_manager::plugin_manager::RPCPlugin&)>
+               functor));
+  MOCK_METHOD1(
       GetDeviceTransportType,
       hmi_apis::Common_TransportType::eType(const std::string& transport_type));
   MOCK_METHOD1(AddAppToTTSGlobalPropertiesList, void(const uint32_t app_id));