From: Michal Bistyga <m.bistyga@samsung.com>
Date: Thu, 7 Sep 2017 13:18:04 +0000 (+0200)
Subject: [CallHistory] Fix for find method
X-Git-Tag: submit/tizen_3.0/20170919.131120~5^2
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=40c39a402f6b7c1c22d2623ce5b5029630ae1315;p=platform%2Fcore%2Fapi%2Fwebapi-plugins.git

[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 1d7a91e0..d6fc1f60 100755
--- 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 a3662374..651f7499 100755
--- 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 1dcf72ad..23fddb25 100755
--- 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));
   }
 }