Fix EventDispatcher crash by rejecting duplicate correlation_ids

src/components/application_manager/include/application_manager/request_controller.h

@@ -289,6 +289,13 @@ class RequestController {
   */
   std::list<RequestPtr> notification_list_;
 
+  /**
+   * @brief Map keeping track of how many duplicate messages were sent for a
+   * given correlation id, to prevent early termination of a request
+   */
+  std::map<uint32_t, uint32_t> duplicate_message_count_;
+  sync_primitives::Lock duplicate_message_count_lock_;
+
   /*
    * timer for checking requests timeout
    */

src/components/application_manager/src/request_controller.cc

@@ -50,6 +50,7 @@ RequestController::RequestController(const RequestControlerSettings& settings)
     : pool_state_(UNDEFINED)
     , pool_size_(settings.thread_pool_size())
     , request_tracker_(settings)
+    , duplicate_message_count_()
     , timer_("AM RequestCtrlTimer",
              new timer::TimerTaskImpl<RequestController>(
                  this, &RequestController::TimeoutThread))
@@ -230,6 +231,21 @@ void RequestController::TerminateRequest(const uint32_t correlation_id,
                     << correlation_id << " connection_key = " << connection_key
                     << " function_id = " << function_id
                     << " force_terminate = " << force_terminate);
+  {
+    AutoLock auto_lock(duplicate_message_count_lock_);
+    auto dup_it = duplicate_message_count_.find(correlation_id);
+    if (duplicate_message_count_.end() != dup_it) {
+      duplicate_message_count_[correlation_id]--;
+      if (0 == duplicate_message_count_[correlation_id]) {
+        duplicate_message_count_.erase(dup_it);
+      }
+      LOG4CXX_DEBUG(logger_,
+                    "Ignoring termination request due to duplicate correlation "
+                    "ID being sent");
+      return;
+    }
+  }
+
   RequestInfoPtr request =
       waiting_for_response_.Find(connection_key, correlation_id);
   if (!request) {
@@ -474,7 +490,24 @@ void RequestController::Worker::threadMain() {
     RequestInfoPtr request_info_ptr =
         utils::MakeShared<MobileRequestInfo>(request_ptr, timeout_in_mseconds);
 
-    request_controller_->waiting_for_response_.Add(request_info_ptr);
+    if (!request_controller_->waiting_for_response_.Add(request_info_ptr)) {
+      commands::CommandRequestImpl* cmd_request =
+          dynamic_cast<commands::CommandRequestImpl*>(request_ptr.get());
+      if (cmd_request != NULL) {
+        uint32_t corr_id = cmd_request->correlation_id();
+        request_controller_->duplicate_message_count_lock_.Acquire();
+        auto dup_it =
+            request_controller_->duplicate_message_count_.find(corr_id);
+        if (request_controller_->duplicate_message_count_.end() == dup_it) {
+          request_controller_->duplicate_message_count_[corr_id] = 0;
+        }
+        request_controller_->duplicate_message_count_[corr_id]++;
+        request_controller_->duplicate_message_count_lock_.Release();
+        cmd_request->SendResponse(
+            false, mobile_apis::Result::INVALID_ID, "Duplicate correlation_id");
+      }
+      continue;
+    }
     LOG4CXX_DEBUG(logger_, "timeout_in_mseconds " << timeout_in_mseconds);
 
     if (0 != timeout_in_mseconds) {

src/components/application_manager/test/request_controller/request_controller_test.cc

@@ -62,6 +62,7 @@ using ::application_manager::request_controller::RequestInfo;
 using ::testing::Return;
 using ::testing::ReturnRef;
 using ::testing::NiceMock;
+using ::testing::_;
 
 typedef NiceMock<application_manager_test::MockRequest> MRequest;
 typedef utils::SharedPtr<MRequest> RequestPtr;
@@ -114,6 +115,7 @@ class RequestControllerTestClass : public ::testing::Test {
     RequestPtr output =
         utils::MakeShared<MRequest>(connection_key, correlation_id);
     ON_CALL(*output, default_timeout()).WillByDefault(Return(default_timeout));
+    ON_CALL(*output, CheckPermissions()).WillByDefault(Return(true));
     return output;
   }
 
@@ -158,6 +160,40 @@ class RequestControllerTestClass : public ::testing::Test {
   const TestSettings default_settings_;
 };
 
+TEST_F(RequestControllerTestClass,
+       AddMobileRequest_DuplicateCorrelationId_INVALID_ID) {
+  RequestPtr request_valid = GetMockRequest();
+  TestAsyncWaiter waiter_valid;
+  ON_CALL(*request_valid, default_timeout()).WillByDefault(Return(0));
+  EXPECT_CALL(*request_valid, Init()).WillOnce(Return(true));
+  EXPECT_CALL(*request_valid, Run())
+      .Times(1)
+      .WillRepeatedly(NotifyTestAsyncWaiter(&waiter_valid));
+
+  EXPECT_EQ(RequestController::SUCCESS,
+            AddRequest(default_settings_,
+                       request_valid,
+                       RequestInfo::RequestType::MobileRequest,
+                       mobile_apis::HMILevel::HMI_NONE));
+  EXPECT_TRUE(waiter_valid.WaitFor(1, 1000));
+
+  // The command should not be run if another command with the same
+  // correlation_id is waiting for a response
+  RequestPtr request_dup_corr_id = GetMockRequest();
+  TestAsyncWaiter waiter_dup;
+  ON_CALL(*request_dup_corr_id, default_timeout()).WillByDefault(Return(0));
+  EXPECT_CALL(*request_dup_corr_id, Init()).WillOnce(Return(true));
+  ON_CALL(*request_dup_corr_id, Run())
+      .WillByDefault(NotifyTestAsyncWaiter(&waiter_dup));
+
+  EXPECT_EQ(RequestController::SUCCESS,
+            AddRequest(default_settings_,
+                       request_dup_corr_id,
+                       RequestInfo::RequestType::MobileRequest,
+                       mobile_apis::HMILevel::HMI_NONE));
+  EXPECT_FALSE(waiter_dup.WaitFor(1, 1000));
+}
+
 TEST_F(RequestControllerTestClass,
        CheckPosibilitytoAdd_ZeroValueLimiters_SUCCESS) {
   // Test case than pending_requests_amount,