diff --git a/src/com/fsck/k9/activity/MessageView.java b/src/com/fsck/k9/activity/MessageView.java index c93ca83b5..5e1f912dd 100644 --- a/src/com/fsck/k9/activity/MessageView.java +++ b/src/com/fsck/k9/activity/MessageView.java @@ -1123,12 +1123,28 @@ public class MessageView extends K9Activity implements OnClickListener { return; } MessageView.this.mMessage = message; + + /* + * Clone the message object because the original could be modified by + * MessagingController later. This could lead to a ConcurrentModificationException + * when that same object is accessed by the UI thread (below). + * + * See issue 3953 + * + * This is just an ugly hack to get rid of the most pressing problem. A proper way to + * fix this is to make Message thread-safe. Or, even better, rewriting the UI code to + * access messages via a ContentProvider. + * + */ + final Message clonedMessage = message.clone(); + runOnUiThread(new Runnable() { public void run() { - if (!message.isSet(Flag.X_DOWNLOADED_FULL) && !message.isSet(Flag.X_DOWNLOADED_PARTIAL)) { + if (!clonedMessage.isSet(Flag.X_DOWNLOADED_FULL) && + !clonedMessage.isSet(Flag.X_DOWNLOADED_PARTIAL)) { mMessageView.loadBodyFromUrl("file:///android_asset/downloading.html"); } - mMessageView.setHeaders(message, account); + mMessageView.setHeaders(clonedMessage, account); mMessageView.setOnFlagListener(new OnClickListener() { @Override public void onClick(View v) { diff --git a/src/com/fsck/k9/mail/Message.java b/src/com/fsck/k9/mail/Message.java index 1afa5ba91..2a7cd5efd 100644 --- a/src/com/fsck/k9/mail/Message.java +++ b/src/com/fsck/k9/mail/Message.java @@ -224,4 +224,32 @@ public abstract class Message implements Part, Body { return 0; } + /** + * Copy the contents of this object into another {@code Message} object. + * + * @param destination + * The {@code Message} object to receive the contents of this instance. + */ + protected void copy(Message destination) { + destination.mUid = mUid; + destination.mInternalDate = mInternalDate; + destination.mFolder = mFolder; + destination.mReference = mReference; + + // mFlags contents can change during the object lifetime, so copy the Set + destination.mFlags = new HashSet(mFlags); + } + + /** + * Creates a new {@code Message} object with the same content as this object. + * + *

+ * Note: + * This method was introduced as a hack to prevent {@code ConcurrentModificationException}s. It + * shouldn't be used unless absolutely necessary. See the comment in + * {@link com.fsck.k9.activity.MessageView.Listener#loadMessageForViewHeadersAvailable(com.fsck.k9.Account, String, String, Message)} + * for more information. + *

+ */ + public abstract Message clone(); } diff --git a/src/com/fsck/k9/mail/internet/MimeHeader.java b/src/com/fsck/k9/mail/internet/MimeHeader.java index 20002feb3..2b87416da 100644 --- a/src/com/fsck/k9/mail/internet/MimeHeader.java +++ b/src/com/fsck/k9/mail/internet/MimeHeader.java @@ -133,9 +133,9 @@ public class MimeHeader { } static class Field { - String name; + final String name; - String value; + final String value; public Field(String name, String value) { this.name = name; @@ -153,4 +153,13 @@ public class MimeHeader { public void setCharset(String charset) { mCharset = charset; } + + public MimeHeader clone() { + MimeHeader header = new MimeHeader(); + header.mCharset = mCharset; + + header.mFields = new ArrayList(mFields); + + return header; + } } diff --git a/src/com/fsck/k9/mail/internet/MimeMessage.java b/src/com/fsck/k9/mail/internet/MimeMessage.java index 92f7b5b4e..0ac949524 100644 --- a/src/com/fsck/k9/mail/internet/MimeMessage.java +++ b/src/com/fsck/k9/mail/internet/MimeMessage.java @@ -553,4 +553,38 @@ public class MimeMessage extends Message { throw new UnsupportedOperationException("Not supported"); } } + + /** + * Copy the contents of this object into another {@code MimeMessage} object. + * + * @param message + * The {@code MimeMessage} object to receive the contents of this instance. + */ + protected void copy(MimeMessage message) { + super.copy(message); + + message.mHeader = mHeader.clone(); + + message.mBody = mBody; + message.mMessageId = mMessageId; + message.mSentDate = mSentDate; + message.mDateFormat = mDateFormat; + message.mSize = mSize; + + // These arrays are not supposed to be modified, so it's okay to reuse the references + message.mFrom = mFrom; + message.mTo = mTo; + message.mCc = mCc; + message.mBcc = mBcc; + message.mReplyTo = mReplyTo; + message.mReferences = mReferences; + message.mInReplyTo = mInReplyTo; + } + + @Override + public MimeMessage clone() { + MimeMessage message = new MimeMessage(); + copy(message); + return message; + } } diff --git a/src/com/fsck/k9/mail/store/LocalStore.java b/src/com/fsck/k9/mail/store/LocalStore.java index 13b4c816e..fbcfafa21 100644 --- a/src/com/fsck/k9/mail/store/LocalStore.java +++ b/src/com/fsck/k9/mail/store/LocalStore.java @@ -3310,6 +3310,25 @@ public class LocalStore extends Store implements Serializable { loadHeaders(); return super.getHeaderNames(); } + + @Override + public LocalMessage clone() { + LocalMessage message = new LocalMessage(); + super.copy(message); + + message.mId = mId; + message.mAttachmentCount = mAttachmentCount; + message.mSubject = mSubject; + message.mPreview = mPreview; + message.mToMeCalculated = mToMeCalculated; + message.mCcMeCalculated = mCcMeCalculated; + message.mToMe = mToMe; + message.mCcMe = mCcMe; + message.mHeadersLoaded = mHeadersLoaded; + message.mMessageDirty = mMessageDirty; + + return message; + } } public static class LocalAttachmentBodyPart extends MimeBodyPart {