Fix race condition in Singleton and Application Manager failure to stop

When SDL is started, but the HMI is not, and we try to register a mobile
app (RegisterAppInterfaceRequest), then in this situation SDL can't be
stopped with ctrl+C - it enters in an endless cycle.
The reason for the problem is that we can't delete the AM, because the
RequestController thread of AM is still running (waiting in a cycle for
the HMI to respond). Also a second AM is created, because in the
Singleton we set to 0 the instance pointer before deleting it and
someone calls instance() before destroy() finishes, because there is no
common lock. The separate locks create a race condition.
The fix is to use a single mutex for the Singleton methods, introduce a
is_stopping_ flag in AM, set it to true in AM's Stop() method, and
also destroy RequestController's thread pool there.
Then check stop flag in RegisterAppInterfaceRequest::Run() and exit the
HMI waiting cycle there.

5 files changed, 41 insertions, 10 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 5c5e29e06..2961202bd 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
@@ -1077,6 +1077,10 @@ class ApplicationManagerImpl : public ApplicationManager,
      */
     bool IsAppsQueriedFrom(const connection_handler::DeviceHandle handle) const;
 
+    bool IsStopping() const {
+      return is_stopping_;
+    }
+
   private:
     /**
      * @brief PullLanguagesInfo allows to pull information about languages.
@@ -1398,6 +1402,8 @@ class ApplicationManagerImpl : public ApplicationManager,
 
     bool is_low_voltage_;
 
+    bool is_stopping_;
+
     DISALLOW_COPY_AND_ASSIGN(ApplicationManagerImpl);
 
     FRIEND_BASE_SINGLETON_CLASS(ApplicationManagerImpl);
diff --git a/src/components/application_manager/src/application_manager_impl.cc b/src/components/application_manager/src/application_manager_impl.cc
index 3523b4747..0c686c79a 100644
--- a/src/components/application_manager/src/application_manager_impl.cc
+++ b/src/components/application_manager/src/application_manager_impl.cc
@@ -110,7 +110,9 @@ ApplicationManagerImpl::ApplicationManagerImpl()
                                       this,
                                       &ApplicationManagerImpl::OnTimerSendTTSGlobalProperties,
                                       true),
-    is_low_voltage_(false) {
+    is_low_voltage_(false),
+    is_stopping_(false) {
+
     std::srand(std::time(0));
     AddPolicyObserver(this);
 
@@ -130,6 +132,7 @@ ApplicationManagerImpl::ApplicationManagerImpl()
 ApplicationManagerImpl::~ApplicationManagerImpl() {
   LOG4CXX_INFO(logger_, "Destructing ApplicationManager.");
 
+  is_stopping_ = true;
   SendOnSDLClose();
   media_manager_ = NULL;
   hmi_handler_ = NULL;
@@ -157,6 +160,7 @@ ApplicationManagerImpl::~ApplicationManagerImpl() {
 
 bool ApplicationManagerImpl::Stop() {
   LOG4CXX_INFO(logger_, "Stop ApplicationManager.");
+  is_stopping_ = true;
   application_list_update_timer_->stop();
   try {
     UnregisterAllApplications();
@@ -165,6 +169,7 @@ bool ApplicationManagerImpl::Stop() {
                   "An error occurred during unregistering applications.");
   }
 
+  request_ctrl_.DestroyThreadpool();
 
   // for PASA customer policy backup should happen :AllApp(SUSPEND)
   LOG4CXX_INFO(logger_, "Unloading policy library.");
diff --git a/src/components/application_manager/src/commands/mobile/register_app_interface_request.cc b/src/components/application_manager/src/commands/mobile/register_app_interface_request.cc
index ac6971ac3..36b2c27f7 100644
--- a/src/components/application_manager/src/commands/mobile/register_app_interface_request.cc
+++ b/src/components/application_manager/src/commands/mobile/register_app_interface_request.cc
@@ -157,12 +157,26 @@ void RegisterAppInterfaceRequest::Run() {
   // FIXME(EZamakhov): on shutdown - get freez
 
   // wait till HMI started
-  while (!ApplicationManagerImpl::instance()->IsHMICooperating()) {
-    sleep(1);
-    // TODO(DK): timer_->StartWait(1);
+  while (ApplicationManagerImpl::exists() &&
+         !ApplicationManagerImpl::instance()->IsStopping() &&
+         !ApplicationManagerImpl::instance()->IsHMICooperating()) {
+    LOG4CXX_DEBUG(logger_, "Waiting for the HMI... conn_key="
+                 << connection_key() << ", correlation_id=" << correlation_id()
+                 << ", default_timeout=" << default_timeout()
+                 << ", thread=" << pthread_self());
     ApplicationManagerImpl::instance()->updateRequestTimeout(connection_key(),
                                                              correlation_id(),
                                                              default_timeout());
+    sleep(1);
+    // TODO(DK): timer_->StartWait(1);
+  }
+
+  if (!ApplicationManagerImpl::exists()) {
+    LOG4CXX_WARN(logger_, "The ApplicationManager doesn't exist!");
+    return;
+  } else if (ApplicationManagerImpl::instance()->IsStopping()) {
+    LOG4CXX_WARN(logger_, "The ApplicationManager is stopping!");
+    return;
   }
 
   const std::string mobile_app_id = (*message_)[strings::msg_params][strings::app_id]
diff --git a/src/components/application_manager/test/mock/include/application_manager/application_manager_impl.h b/src/components/application_manager/test/mock/include/application_manager/application_manager_impl.h
index cad956ead..bc6a4e6ba 100644
--- a/src/components/application_manager/test/mock/include/application_manager/application_manager_impl.h
+++ b/src/components/application_manager/test/mock/include/application_manager/application_manager_impl.h
@@ -313,6 +313,7 @@ class ApplicationManagerImpl : public ApplicationManager,
   MOCK_METHOD3(set_state, void(ApplicationSharedPtr app,
                                mobile_apis::HMILevel::eType,
                                mobile_apis::AudioStreamingState::eType));
+MOCK_CONST_METHOD0(IsStopping, bool());
 
   struct ApplicationsAppIdSorter {
     bool operator() (const ApplicationSharedPtr lhs,
diff --git a/src/components/utils/include/utils/singleton.h b/src/components/utils/include/utils/singleton.h
index 41face4f2..fff7294d1 100644
--- a/src/components/utils/include/utils/singleton.h
+++ b/src/components/utils/include/utils/singleton.h
@@ -111,18 +111,24 @@ class Singleton {
 
   static T** instance_pointer();
   static Deleter* deleter();
+
+  static sync_primitives::Lock lock_;
 };
 
+
+template<typename T, class Deleter>
+sync_primitives::Lock Singleton<T, Deleter>::lock_;
+
+
 template<typename T, class Deleter>
 T* Singleton<T, Deleter>::instance() {
-  static sync_primitives::Lock lock;
 
   T* local_instance;
   atomic_pointer_assign(local_instance, *instance_pointer());
   memory_barrier();
 
   if (!local_instance) {
-    lock.Acquire();
+    lock_.Acquire();
     local_instance = *instance_pointer();
     if (!local_instance) {
       local_instance = new T();
@@ -130,7 +136,7 @@ T* Singleton<T, Deleter>::instance() {
       atomic_pointer_assign(*instance_pointer(), local_instance);
       deleter()->grab(local_instance);
     }
-    lock.Release();
+    lock_.Release();
   }
 
   return local_instance;
@@ -138,14 +144,13 @@ T* Singleton<T, Deleter>::instance() {
 
 template<typename T, class Deleter>
 void Singleton<T, Deleter>::destroy() {
-  static sync_primitives::Lock lock;
 
   T* local_instance;
   atomic_pointer_assign(local_instance, *instance_pointer());
   memory_barrier();
 
   if (local_instance) {
-    lock.Acquire();
+    lock_.Acquire();
     local_instance = *instance_pointer();
     if (local_instance) {
       atomic_pointer_assign(*instance_pointer(), 0);
@@ -153,7 +158,7 @@ void Singleton<T, Deleter>::destroy() {
       delete local_instance;
       deleter()->grab(0);
     }
-    lock.Release();
+    lock_.Release();
   }
 }