From d028a5ddc48cf2a342b51f61deb08fdf7e4153e3 Mon Sep 17 00:00:00 2001
From: Pawel Wasowski
Date: Thu, 9 May 2019 13:31:49 +0200
Subject: [PATCH] [messaging] Fix synchronization issues
Due to the delay between updating email with native C API functions and
updates of the corresponding message/thread records in the mail
database, race conditions occured in the previous implementation,
resulting in inconsitencies between thread_ids in the DB and those of
messages in JS layer. Active waits have been added to mitigate this
problems.
[Verification] tct-tizen-messaging-email pass rate: 100%
Change-Id: Ie0771ffc40e6cdee696756a8e831ae56bbe8f84d
Signed-off-by: Pawel Wasowski
(cherry picked from commit 1440977fb5202077c98d86da90ee2046bc213132)
---
src/messaging/email_manager.cc | 155 +++++++++++++++++++++++++++++++++++++++--
src/messaging/message.cc | 5 ++
src/messaging/message.h | 1 +
3 files changed, 155 insertions(+), 6 deletions(-)
diff --git a/src/messaging/email_manager.cc b/src/messaging/email_manager.cc
index 8c1207b..5b89dd6 100644
--- a/src/messaging/email_manager.cc
+++ b/src/messaging/email_manager.cc
@@ -17,8 +17,11 @@
//#include
//#include
//#include
+#include
+#include
#include
#include
+#include
#include "common/logger.h"
#include "common/platform_exception.h"
#include "common/scope_exit.h"
@@ -62,6 +65,7 @@
using namespace common;
using namespace extension::tizen;
+using namespace std::chrono_literals;
namespace extension {
namespace messaging {
@@ -69,6 +73,16 @@ namespace messaging {
namespace {
const int ACCOUNT_ID_NOT_INITIALIZED = -1;
const std::string FIND_FOLDERS_ATTRIBUTE_ACCOUNTID_NAME = "serviceId";
+
+bool isFirstInThread(const Message* message) {
+ ScopeLogger();
+ return message->getId() == message->getConversationId();
+}
+
+bool isFirstInThread(const email_mail_data_t* mail_data) {
+ ScopeLogger();
+ return mail_data->mail_id == mail_data->thread_id;
+}
} // anonymous namespace
EmailManager::EmailManager() : m_slot_size(-1), m_is_initialized(false) {
@@ -251,11 +265,68 @@ PlatformResult EmailManager::addMessagePlatform(int account_id, std::shared_ptr<
}
}
- err = email_get_mail_data(message->getId(), &mail_data_final);
- if (EMAIL_ERROR_NONE != err) {
- return LogAndCreateResult(ErrorCode::UNKNOWN_ERR, "Couldn't retrieve added mail data",
- ("email_get_mail_data error: %d (%s)", err, get_error_message(err)));
+ /*
+ * If the message is first in its thread (its mail_id == thread_id)
+ * and it has an attachment, there is some delay between addEmailAttachments
+ * return and the corresponding update of message's record in the internal
+ * email DB.
+ *
+ * The internal DB is queried for mail_data_final below, but due to
+ * the mentioned delay, it may return mail_data_final, that is not
+ * up-to-date, i.e with mail_id != thread_id).
+ * If such situation occurs, up to MAX_RETRIES retries of calling
+ * email_get_mail_data are performed.
+ * If returned mail_data_final is up-to-date before reaching retries limit,
+ * the returned value is used to set mail properties.
+ * If returned mail_data_final is still not up-to-date, thread_id of the message
+ * is set manually to mail_id.
+ */
+
+ if (isFirstInThread(mail_data)) {
+ int retry = 0;
+ const int MAX_RETRIES = 5;
+ for (; retry < MAX_RETRIES; ++retry) {
+ err = email_get_mail_data(message->getId(), &mail_data_final);
+
+ if (EMAIL_ERROR_NONE != err) {
+ /*
+ * TODO: in the case of email_get_mail_data failure,
+ * the message should be removed from the databse
+ */
+ return LogAndCreateResult(
+ ErrorCode::UNKNOWN_ERR, "Couldn't add message to draft mailbox",
+ ("email_get_mail_data error: %d (%s)", err, get_error_message(err)));
+ }
+
+ if (isFirstInThread(mail_data) && isFirstInThread(mail_data_final)) {
+ LoggerD("Message adding process finished after %d retries. mail_id == thread_id", retry);
+ break;
+ }
+
+ int free_error = email_free_mail_data(&mail_data_final, 1);
+ if (EMAIL_ERROR_NONE != free_error) {
+ LoggerW("email_free_mail_data error: %d, %s", free_error, get_error_message(free_error));
+ }
+ LoggerD("Retry number %d failed", retry);
+ std::this_thread::sleep_for(100ms);
+ }
+
+ if (MAX_RETRIES == retry) {
+ LoggerD(
+ "Message adding process not finished after %d retries. Setting proper conversationId "
+ "manually",
+ retry);
+ mail_data_final->thread_id = mail_data_final->mail_id;
+ }
+ } else {
+ err = email_get_mail_data(message->getId(), &mail_data_final);
+ if (EMAIL_ERROR_NONE != err) {
+ return LogAndCreateResult(
+ ErrorCode::UNKNOWN_ERR, "Couldn't add message to draft mailbox",
+ ("email_get_mail_data error: %d (%s)", err, get_error_message(err)));
+ }
}
+
ret = message->updateEmailMessage(*mail_data_final);
if (ret.IsError()) {
return ret;
@@ -915,6 +986,7 @@ PlatformResult EmailManager::UpdateMessagesPlatform(MessagesCallbackUserData* ca
std::lock_guard lock(m_mutex);
std::vector> messages = callback->getMessages();
+ std::list> firstInThreadAndHasAttachment;
MessageType type = callback->getMessageServiceType();
for (auto it = messages.begin(); it != messages.end(); ++it) {
if ((*it)->getType() != type) {
@@ -927,7 +999,11 @@ PlatformResult EmailManager::UpdateMessagesPlatform(MessagesCallbackUserData* ca
if (ret.IsError()) return ret;
if ((*it)->getHasAttachment()) {
- LoggerD("Message has attachments. Workaround need to be used.");
+ if (isFirstInThread(it->get())) {
+ // Messages with attachments, that are first in their threads are added
+ // to the container, to be processed later in this function
+ firstInThreadAndHasAttachment.push_back(*it);
+ }
// Update of mail on server using function email_update_mail() is not possible.
// Attachment is updated only locally (can't be later loaded from server),
// so use of workaround is needed:
@@ -944,8 +1020,8 @@ PlatformResult EmailManager::UpdateMessagesPlatform(MessagesCallbackUserData* ca
// storing old message id
(*it)->setOldId(mail->mail_id);
// deleting old mail
- LoggerD("mail deleted = [%d]\n", mail->mail_id);
error = email_delete_mail(mail->mailbox_id, &mail->mail_id, 1, 1);
+ LoggerD("mail deleted = [%d]\n", mail->mail_id);
if (EMAIL_ERROR_NONE != error) {
return LogAndCreateResult(
ErrorCode::UNKNOWN_ERR, "Error while deleting old mail on update",
@@ -961,6 +1037,73 @@ PlatformResult EmailManager::UpdateMessagesPlatform(MessagesCallbackUserData* ca
}
}
}
+
+ if (firstInThreadAndHasAttachment.size() == 0) {
+ return PlatformResult(ErrorCode::NO_ERROR);
+ }
+
+ /*
+ * If a message with attachment, that is first in its thread was updated,
+ * the update process consists of adding a new message - updated version
+ * of the original and removing the original.
+ *
+ * After the original mail is deleted, the thread_id of the updated version
+ * changes - it is now identical to the mail_id of the new message.
+ * The change should be reflected in the updated message, sent to the JS
+ * layer. The code below sets proper thread_id for such messages and unsets
+ * inResponseTo fields.
+ *
+ * A few retries with sleeps between them are performed, to ensure, that after
+ * this function returns, messages returned to the JS layer and
+ * corresponding records in the mail DB have the same thread_ids. Due to a
+ * delay between calling email_delete_mail function and the update of records
+ * in the database, some time is needed.
+ */
+ email_mail_data_t* mail_data{nullptr};
+ int retry = 0;
+ const int MAX_RETRIES = 5;
+ for (; retry < MAX_RETRIES; ++retry) {
+ std::this_thread::sleep_for(100ms);
+ auto it = firstInThreadAndHasAttachment.begin();
+ while (it != firstInThreadAndHasAttachment.end()) {
+ error = email_get_mail_data((*it)->getId(), &mail_data);
+ if (EMAIL_ERROR_NONE != error) {
+ return LogAndCreateResult(
+ ErrorCode::UNKNOWN_ERR, "Couldn't add message to draft mailbox",
+ ("email_get_mail_data error: %d (%s)", error, get_error_message(error)));
+ }
+
+ if (isFirstInThread(mail_data)) {
+ (*it)->setConversationId(mail_data->thread_id);
+ (*it)->unsetInResponseTo();
+ it = firstInThreadAndHasAttachment.erase(it);
+ } else {
+ ++it;
+ }
+
+ int free_error = email_free_mail_data(&mail_data, 1);
+ if (EMAIL_ERROR_NONE != free_error) {
+ LoggerW("email_free_mail_data error: %d, %s", free_error, get_error_message(free_error));
+ }
+ }
+
+ if (firstInThreadAndHasAttachment.size() == 0) {
+ LoggerD("Message updating process finished after %d retries", retry);
+ break;
+ }
+
+ LoggerD("Message update retry number %d finished", retry);
+ }
+
+ if (MAX_RETRIES == retry) {
+ LoggerW("Message updating process not finished after %d retries.", retry);
+ for (auto& message : firstInThreadAndHasAttachment) {
+ LoggerD("Setting proper conversationId manually, message id: %d", message->getId());
+ message->setConversationId(message->getId());
+ message->unsetInResponseTo();
+ }
+ }
+
return PlatformResult(ErrorCode::NO_ERROR);
}
diff --git a/src/messaging/message.cc b/src/messaging/message.cc
index 305985d..89d0a05 100644
--- a/src/messaging/message.cc
+++ b/src/messaging/message.cc
@@ -279,6 +279,11 @@ void Message::setInResponseTo(int inresp) {
m_in_response_set = true;
}
+void Message::unsetInResponseTo() {
+ m_in_response = -1;
+ m_in_response_set = false;
+}
+
void Message::setMessageStatus(MessageStatus status) {
m_status = status;
}
diff --git a/src/messaging/message.h b/src/messaging/message.h
index c4c5c15..7562fc3 100644
--- a/src/messaging/message.h
+++ b/src/messaging/message.h
@@ -98,6 +98,7 @@ class Message : public FilterableObject {
virtual void setIsHighPriority(bool highpriority);
virtual void setSubject(std::string subject);
virtual void setInResponseTo(int inresp);
+ virtual void unsetInResponseTo();
virtual void setMessageStatus(MessageStatus status);
virtual void setMessageAttachments(AttachmentPtrVector& attachments);
virtual void setServiceId(int service_id);
--
2.7.4