[CallHistory] Fix for find method

[Bug]Multiple threads could access one vector, which resulted in crash.

[Verification]Pass rate didn't change.

Change-Id: I46a809c0df2965220b35a8e78389f2fdc8746c77
Signed-off-by: Michal Bistyga <m.bistyga@samsung.com>

diff --git a/src/callhistory/callhistory.cc b/src/callhistory/callhistory.cc
index 1d7a91e07c51eb4b7496bf547c99fe2dd43100a7..d6fc1f60d6ccfa8c8afe0d371a1bed7d7d145655 100755 (executable)
--- a/src/callhistory/callhistory.cc
+++ b/src/callhistory/callhistory.cc
@@ -98,10 +98,15 @@ void CallHistory::FindThread(const picojson::object& args, CallHistory* call_his
   LoggerD("Entered");
 
   std::shared_ptr<picojson::value> response{new picojson::value(picojson::object())};
-  int phone_numbers = call_history->getPhoneNumbers().size();
+
+  int phone_numbers_size = 0;
+  {
+    CallHistory::LockedVector phone_numbers = call_history->getPhoneNumbers();
+    phone_numbers_size = phone_numbers.size();
+  }
   const double callback_id = args.find("callbackId")->second.get<double>();
 
-  if (phone_numbers == 0) {
+  if (0 == phone_numbers_size) {
     LogAndReportError(PlatformResult(ErrorCode::UNKNOWN_ERR, "Phone numbers list is empty."),
                 &response->get<picojson::object>());
   } else {
@@ -247,12 +252,18 @@ void CallHistory::FindThread(const picojson::object& args, CallHistory* call_his
 void CallHistory::LoadPhoneNumbers(const picojson::object& args, CallHistory* call_history)
 {
   LoggerD("Entered");
-
+  CallHistory::LockedVector phone_numbers = call_history->getPhoneNumbers();
+  if (0 != phone_numbers.size()) {
+    LoggerD("m_phone_numbers is already filled. Returning.");
+    return;
+  }
   char** cp_list =  tel_get_cp_name_list();
+  if (nullptr == cp_list){
+    LoggerE("Failed to get cp_list");
+  }
 
   if (cp_list) {
     unsigned int modem_num = 0;
-    std::vector<std::string>& phone_numbers = call_history->getPhoneNumbers();
 
     while (cp_list[modem_num]) {
       std::string n = "";
@@ -298,18 +309,12 @@ void CallHistory::LoadPhoneNumbers(const picojson::object& args, CallHistory* ca
 
     g_strfreev(cp_list);
   }
-
-  FindThread(args, call_history);
 }
 
 void CallHistory::find(const picojson::object& args) {
   LoggerD("Entered");
-
-  if (m_phone_numbers.size() == 0) {
-    std::thread(LoadPhoneNumbers, args, this).detach();
-  } else {
-    std::thread(FindThread, args, this).detach();
-  }
+  std::thread(LoadPhoneNumbers, args, this).detach();
+  std::thread(FindThread, args, this).detach();
 }
 
 PlatformResult CallHistory::remove(const picojson::object& args)
@@ -501,9 +506,9 @@ void CallHistory::removeAll(const picojson::object& args)
       data);
 }
 
-std::vector<std::string>& CallHistory::getPhoneNumbers()
+CallHistory::LockedVector CallHistory::getPhoneNumbers()
 {
-  return m_phone_numbers;
+  return std::move(CallHistory::LockedVector{m_phone_numbers, m_phone_numbers_mutex});
 }
 
 CallHistoryUtils& CallHistory::getUtils()

diff --git a/src/callhistory/callhistory.h b/src/callhistory/callhistory.h
index a3662374e477fa2be0b1709590e9670d6745c8c0..651f7499379b2ff1e2527646a7c26304b261ae23 100755 (executable)
--- a/src/callhistory/callhistory.h
+++ b/src/callhistory/callhistory.h
@@ -20,6 +20,8 @@
 #include <string>
 #include <vector>
 #include <future>
+#include <mutex>
+#include <utility>
 
 #include <contacts.h>
 #include <contacts_internal.h>
@@ -40,7 +42,21 @@ class CallHistory
   explicit CallHistory(CallHistoryInstance& instance);
   ~CallHistory();
 
-  std::vector<std::string>& getPhoneNumbers();
+  class LockedVector;
+  LockedVector getPhoneNumbers();
+  //You can access m_phone_numbers only by getPhoneNumbers(). It will be available and locked in current scope.
+  //All section since calling getPhoneNumbers until end of scope will be critical section.
+  class LockedVector {
+    std::unique_lock<std::mutex> guard;
+    typedef std::vector<std::string> StringVector;
+    StringVector& vector;
+    LockedVector( StringVector& vector, std::mutex& lock ) : guard(lock), vector(vector) {};
+    friend LockedVector CallHistory::getPhoneNumbers();
+  public:
+    StringVector::size_type size() { return vector.size(); }
+    StringVector::value_type at(StringVector::size_type i) { return vector.at(i); }
+    void push_back(const StringVector::value_type& value) { vector.push_back(value); }
+  };
   CallHistoryUtils& getUtils();
 
   void find(const picojson::object& args);
@@ -57,7 +73,8 @@ class CallHistory
   static void LoadPhoneNumbers(const picojson::object& args, CallHistory* call_history);
 
   bool m_is_listener_set;
-  std::vector<std::string> m_phone_numbers;
+  std::mutex m_phone_numbers_mutex;
+  std::vector<std::string> m_phone_numbers; //See LockedVector
   CallHistoryInstance& instance_;
   CallHistoryUtils utils_;
 };

diff --git a/src/callhistory/callhistory_utils.cc b/src/callhistory/callhistory_utils.cc
index 1dcf72adfa1c45a4f34eac3deb9d69380214126c..23fddb25c15b8076905b06fadd1a2f3f8ed8dfe6 100755 (executable)
--- a/src/callhistory/callhistory_utils.cc
+++ b/src/callhistory/callhistory_utils.cc
@@ -226,8 +226,11 @@ void CallHistoryUtils::parseCallingParty(contacts_record_h *record, picojson::ob
 {
   LoggerD("Entered");
 
-  const std::vector<std::string>& phone_numbers = history_.getPhoneNumbers();
-  int sim_count = phone_numbers.size();
+  int sim_count = 0;
+  {
+    CallHistory::LockedVector phone_numbers = history_.getPhoneNumbers();
+    sim_count = phone_numbers.size();
+  }
   int sim_index;
 
   int ret = contacts_record_get_int(*record, _contacts_phone_log.sim_slot_no, &sim_index);
@@ -238,6 +241,7 @@ void CallHistoryUtils::parseCallingParty(contacts_record_h *record, picojson::ob
   if (sim_index >= sim_count) {
     LoggerE("sim slot no. [%d] is out of count %d", sim_index, sim_count);
   } else if (sim_index > 0) {
+    CallHistory::LockedVector phone_numbers = history_.getPhoneNumbers();
     obj[STR_CALLING_PARTY] = picojson::value(phone_numbers.at(sim_index));
   }
 }