From: Pawel Wasowski Date: Wed, 19 Dec 2018 18:05:20 +0000 (+0100) Subject: [Messaging] Prevent crash on getMessageServices call X-Git-Tag: submit/tizen/20190109.233706^2^2~1 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=refs%2Fchanges%2F28%2F196828%2F3;p=platform%2Fcore%2Fapi%2Fwebapi-plugins.git [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 --- diff --git a/src/messaging/messaging_manager.cc b/src/messaging/messaging_manager.cc index 566f1985..d0e20f9c 100644 --- 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& user_data) { +std::string getAccountName(const email_account_t& email_account) { ScopeLogger(); - std::shared_ptr 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& user_data) { +} // namespace + +PlatformResult MessagingManager::initEmailServices() { ScopeLogger(); - std::shared_ptr response = user_data->json; - picojson::object& obj = response->get(); - 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> 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 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& email_services = *(user_data->services_map); - std::for_each(email_services.begin(), email_services.end(), - [](std::pair el) { delete el.second; }); - email_services.clear(); - - std::vector 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( - 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 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(new picojson::value(picojson::object())); - picojson::object& obj = json->get(); - obj[JSON_CALLBACK_ID] = picojson::value(callbackId); + picojson::value response{picojson::object{}}; + picojson::object& response_object = response.get(); + 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(); + 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(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(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 220c23fd..79ffb19c 100644 --- 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 m_email_services; std::pair m_sms_service;