[Messaging] Prevent crash on getMessageServices call

Calling tizen.messaging.getMessageServices() multiple times at
short intervals used to cause a crash, due to race condition, occurring
between different threads, modifying the same MessagingManager
instance's fields.
The fix moves execution of the underlying implementation to the main
thread - it is not called concurrently. The function execution time
is typically below 2 ms, so moving it to the main thread should
not worsen user experience.

[Verification] tct-tizen-messaging-email-tests pass rate: 100%
               tct-tizen-messaging-sms-tests pass rate: 100%
               tct-tizen-messaging-mms-tests pass rate: 100%
               A code snippet, that used to cause a crash, does not
               cause crash now:
               for (var i = 0; i < 1000; ++i) {
                   tizen.messaging.getMessageServices('messaging.email',
                                                      s=>{console.log(s);},
                                                      e=>{console.error(e);});
               }

Change-Id: I2f140281e64aeffea1ad9ca15f99ee38378693d3
Signed-off-by: Pawel Wasowski <p.wasowski2@partner.samsung.com>

diff --git a/src/messaging/messaging_manager.cc b/src/messaging/messaging_manager.cc
index 566f1985a4fd2a85c557c9af8f9e549de5d5f099..d0e20f9c268027178323608cd4893e75c9399785 100644 (file)
--- a/src/messaging/messaging_manager.cc
+++ b/src/messaging/messaging_manager.cc
@@ -27,6 +27,7 @@
 #include "common/logger.h"
 #include "common/picojson.h"
 #include "common/platform_exception.h"
+#include "common/scope_exit.h"
 #include "common/task-queue.h"
 #include "common/tools.h"
 
@@ -87,170 +88,167 @@ MessagingManager::~MessagingManager() {
     delete m_mms_service.second;
   }
 }
+namespace {
 
-static gboolean callbackCompleted(const std::shared_ptr<MsgManagerCallbackData>& user_data) {
+std::string getAccountName(const email_account_t& email_account) {
   ScopeLogger();
-  std::shared_ptr<picojson::value> response = user_data->json;
-  common::Instance::PostMessage(&user_data->instance_, response->serialize().c_str());
-  return false;
+
+  std::string name = "[";
+  if (email_account.account_name) {
+    name += email_account.account_name;
+  }
+  name += "] ";
+  name += email_account.incoming_server_user_name;
+
+  return name;
 }
 
-static void* getMsgServicesThread(const std::shared_ptr<MsgManagerCallbackData>& user_data) {
+}  // namespace
+
+PlatformResult MessagingManager::initEmailServices() {
   ScopeLogger();
 
-  std::shared_ptr<picojson::value> response = user_data->json;
-  picojson::object& obj = response->get<picojson::object>();
-  MessageType type = MessageType::UNDEFINED;
-
-  auto platform_result = MessagingUtil::stringToMessageType(user_data->type, &type);
-  // after extraction of input data, remove it
-
-  if (platform_result) {
-    switch (type) {
-      case MessageType::SMS:
-        LoggerD("MessageService for SMS");
-        {
-          if (user_data->sms_service->second) {
-            delete user_data->sms_service->second;
-          }
-
-          MessageService* service =
-              MessageServiceShortMsg::GetSmsMessageService(user_data->instance_);
-
-          if (!service) {
-            platform_result = LogAndCreateResult(ErrorCode::UNKNOWN_ERR,
-                                                 "MessageService for SMS creation failed");
-          } else {
-            *(user_data->sms_service) = std::make_pair(service->getMsgServiceId(), service);
-
-            picojson::array array;
-            array.push_back(picojson::value(service->toPicoJS()));
-            ReportSuccess(picojson::value(array), obj);
-
-            // service is stored, so it cannot be deleted
-            service = nullptr;
-          }
-        }
-        break;
+  email_account_t* email_accounts = nullptr;
+  int count = 0;
+  SCOPE_EXIT {
+    email_free_account(&email_accounts, count);
+  };
+
+  int ret = email_get_account_list(&email_accounts, &count);
+  if (ret != EMAIL_ERROR_NONE) {
+    return LogAndCreateResult(
+        ErrorCode::UNKNOWN_ERR, "Could not get account list",
+        ("email_get_account_list error: %d (%s)", ret, get_error_message(ret)));
+  }
 
-      case MessageType::MMS:
-        LoggerD("MessageService for MMS");
-        {
-          if (user_data->mms_service->second) {
-            delete user_data->mms_service->second;
-          }
-
-          MessageService* service =
-              MessageServiceShortMsg::GetMmsMessageService(user_data->instance_);
-
-          if (!service) {
-            platform_result = LogAndCreateResult(ErrorCode::UNKNOWN_ERR,
-                                                 "MessageService for SMS creation failed");
-          } else {
-            *(user_data->mms_service) = std::make_pair(service->getMsgServiceId(), service);
-
-            picojson::array array;
-            array.push_back(picojson::value(service->toPicoJS()));
-            ReportSuccess(picojson::value(array), obj);
-
-            // service is stored, so it cannot be deleted
-            service = nullptr;
-          }
-        }
-        break;
+  std::vector<std::unique_ptr<MessageService>> msg_services{};
+  msg_services.reserve(count);
 
-      case MessageType::EMAIL:
-        LoggerD("MessageService for EMAIL");
-        {
-          email_account_t* email_accounts = nullptr;
-          int count = 0;
-
-          int ntv_ret = email_get_account_list(&email_accounts, &count);
-          if (ntv_ret != EMAIL_ERROR_NONE) {
-            platform_result = LogAndCreateResult(
-                ErrorCode::UNKNOWN_ERR, "Error during getting account list",
-                ("email_get_account_list error: %d (%s)", ntv_ret, get_error_message(ntv_ret)));
-          } else {
-            std::vector<MessageService*> msgServices;
-
-            for (int i = 0; i < count && platform_result; ++i) {
-              std::string name = "[";
-              if (email_accounts[i].account_name) {
-                name += email_accounts[i].account_name;
-              }
-              name += "] ";
-              name += email_accounts[i].incoming_server_user_name;
-              LoggerD("Account[%d/%d] id: %d, name: %s", i, count, email_accounts[i].account_id,
-                      name.c_str());
-
-              MessageService* service = new (std::nothrow) MessageServiceEmail(
-                  email_accounts[i].account_id, name.c_str(), user_data->instance_);
-              if (!service) {
-                LoggerD("message service[%d] is NULL", i);
-                std::for_each(msgServices.begin(), msgServices.end(),
-                              [](MessageService* service) { delete service; });
-                msgServices.clear();
-                platform_result = LogAndCreateResult(ErrorCode::UNKNOWN_ERR,
-                                                     "MessageService for email creation failed");
-              } else {
-                msgServices.push_back(service);
-              }
-
-              // service is stored, so it cannot be deleted
-              service = nullptr;
-            }
-
-            email_free_account(&email_accounts, count);
-
-            if (platform_result) {
-              std::map<int, MessageService*>& email_services = *(user_data->services_map);
-              std::for_each(email_services.begin(), email_services.end(),
-                            [](std::pair<int, MessageService*> el) { delete el.second; });
-              email_services.clear();
-
-              std::vector<picojson::value> response;
-              std::for_each(msgServices.begin(), msgServices.end(),
-                            [&response, &email_services](MessageService* service) {
-                              response.push_back(picojson::value(service->toPicoJS()));
-                              email_services.insert(std::pair<int, MessageService*>(
-                                  service->getMsgServiceId(), service));
-                            });
-              ReportSuccess(picojson::value(response), obj);
-            }
-          }
-        }
-        break;
-      default:
-        platform_result = LogAndCreateResult(ErrorCode::UNKNOWN_ERR, "Service type is undefined");
+  for (int i = 0; i < count; ++i) {
+    /* const reference extends the lifetime of the temporary object,
+       so it's still valid after returning from a function */
+    const auto& name = getAccountName(email_accounts[i]);
+    LoggerD("Account[%d/%d] id: %d, name: %s", i, count, email_accounts[i].account_id,
+            name.c_str());
+
+    std::unique_ptr<MessageService> service{
+        new (std::nothrow) MessageServiceEmail{email_accounts[i].account_id, name, instance_}};
+
+    if (!service) {
+      LoggerD("message service[%d] is NULL", i);
+      return PlatformResult{ErrorCode::UNKNOWN_ERR, "An unknown error occurred"};
+    } else {
+      msg_services.push_back(std::move(service));
     }
-  } else {
-    platform_result = LogAndCreateResult(ErrorCode::UNKNOWN_ERR, "Unsupported service type");
   }
 
-  if (!platform_result) {
-    LoggerE("Unknown error");
-    ReportError(platform_result, &obj);
+  // clearing the map to avoid problems with refreshing the map after removing account
+  m_email_services.clear();
+  for (auto& service : msg_services) {
+    m_email_services.insert(std::make_pair(service->getMsgServiceId(), service.get()));
+    service.release();
+  }
+
+  return PlatformResult{ErrorCode::NO_ERROR};
+}
+
+PlatformResult MessagingManager::initSmsService() {
+  ScopeLogger();
+
+  if (m_sms_service.second) {
+    // SMS service is already initialized
+    return PlatformResult{ErrorCode::NO_ERROR};
+  }
+
+  MessageService* service = MessageServiceShortMsg::GetSmsMessageService(instance_);
+
+  if (!service) {
+    return LogAndCreateResult(ErrorCode::UNKNOWN_ERR, "SMS MessageService creation failed");
+  }
+
+  m_sms_service = std::make_pair(service->getMsgServiceId(), service);
+  return PlatformResult{ErrorCode::NO_ERROR};
+}
+
+PlatformResult MessagingManager::initMmsService() {
+  ScopeLogger();
+
+  if (m_mms_service.second) {
+    // MMS service is already initialized
+    return PlatformResult{ErrorCode::NO_ERROR};
+  }
+
+  MessageService* service = MessageServiceShortMsg::GetMmsMessageService(instance_);
+
+  if (!service) {
+    return LogAndCreateResult(ErrorCode::UNKNOWN_ERR, "MMS MessageService creation failed");
   }
 
-  return nullptr;
+  m_mms_service = std::make_pair(service->getMsgServiceId(), service);
+  return PlatformResult{ErrorCode::NO_ERROR};
 }
 
 void MessagingManager::getMessageServices(const std::string& type, double callbackId) {
   ScopeLogger();
 
-  auto json = std::shared_ptr<picojson::value>(new picojson::value(picojson::object()));
-  picojson::object& obj = json->get<picojson::object>();
-  obj[JSON_CALLBACK_ID] = picojson::value(callbackId);
+  picojson::value response{picojson::object{}};
+  picojson::object& response_object = response.get<picojson::object>();
+  response_object[JSON_CALLBACK_ID] = picojson::value(callbackId);
+  MessageType message_service_type = UNDEFINED;
+
+  auto result = MessagingUtil::stringToMessageType(type, &message_service_type);
+  if (!result) {
+    ReportError(PlatformResult{ErrorCode::INVALID_VALUES_ERR, "Unknown service type: " + type},
+                &response_object);
+    common::Instance::PostMessage(&instance_, response.serialize().c_str());
+    return;
+  }
+
+  response_object[JSON_RESULT] = picojson::value{picojson::array{}};
+  picojson::array& service_array = response_object[JSON_RESULT].get<picojson::array>();
+  PlatformResult service_init_result = PlatformResult{ErrorCode::UNKNOWN_ERR};
+
+  switch (message_service_type) {
+    case SMS:
+      service_init_result = initSmsService();
+      if (!service_init_result) {
+        break;
+      }
+      service_array.push_back(picojson::value{(m_sms_service.second)->toPicoJS()});
+      break;
+
+    case MMS:
+      service_init_result = initMmsService();
+      if (!service_init_result) {
+        break;
+      }
+      service_array.push_back(picojson::value{(m_mms_service.second)->toPicoJS()});
+      break;
 
-  auto user_data = std::shared_ptr<MsgManagerCallbackData>(new MsgManagerCallbackData(instance_));
-  user_data->type = type;
-  user_data->json = json;
-  user_data->services_map = &m_email_services;
-  user_data->sms_service = &m_sms_service;
-  user_data->mms_service = &m_mms_service;
+    case EMAIL:
+      service_init_result = initEmailServices();
+      if (!service_init_result) {
+        break;
+      }
+      for (const auto& email_service : m_email_services) {
+        service_array.push_back(picojson::value{(email_service.second)->toPicoJS()});
+      }
+      break;
+
+    case UNDEFINED:
+    default:
+      service_init_result =
+          PlatformResult{ErrorCode::UNKNOWN_ERR, "Could not retrieve " + type + "service"};
+      break;
+  }
+
+  if (service_init_result) {
+    ReportSuccess(response_object);
+  } else {
+    ReportError(service_init_result, &response_object);
+  }
 
-  common::TaskQueue::GetInstance().Queue<MsgManagerCallbackData>(getMsgServicesThread,
-                                                                 callbackCompleted, user_data);
+  common::Instance::PostMessage(&instance_, response.serialize().c_str());
 }
 
 MessageService* MessagingManager::getMessageService(const int id) {

diff --git a/src/messaging/messaging_manager.h b/src/messaging/messaging_manager.h
index 220c23fd3ee81627fcad8c8d431e848c51df261e..79ffb19c5777c13aa9f2bfc03dff723d710e80eb 100644 (file)
--- a/src/messaging/messaging_manager.h
+++ b/src/messaging/messaging_manager.h
@@ -52,6 +52,10 @@ class MessagingManager {
  private:
   void operator=(const MessagingManager&);
 
+  common::PlatformResult initEmailServices();
+  common::PlatformResult initSmsService();
+  common::PlatformResult initMmsService();
+
   msg_handle_t m_msg_handle;
   std::map<int, MessageService*> m_email_services;
   std::pair<int, MessageService*> m_sms_service;