721925 - compaction of local folders > 4GB can cause problems

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[David :Bienvenu]

The issue is here:

http://mxr.mozilla.org/comm-central/source/mailnews/base/src/nsMsgFolderCompactor.cpp#1126

This code should check if m_startOfMsg is > 4GB, and if so, pass -1 in for the message key, to get an auto-assigned msgKey.

Comment 1

[:aceman]

How can the folder get > 4GB? I thought that is not supported. Isn't there a problem somewhere else too?

Comment 2

[David :Bienvenu]

As part of the pluggable store work, I made the berkeley mailbox format support folders > 4GB

Comment 3

[David :Bienvenu]

Attachment: proposed fix

we have a unit test that covers compaction of folders > 4GB in general, but not this specific issue, which is hard to craft a test case for, so if it still passes, this patch hasn't broken compaction of folders > 4GB.

Comment 4

[Mark Banner (:standard8)]

Comment on attachment 593946 [details] [diff] [review]
proposed fix

>diff --git a/mailnews/base/src/nsMsgFolderCompactor.cpp b/mailnews/base/src/nsMsgFolderCompactor.cpp
>--- a/mailnews/base/src/nsMsgFolderCompactor.cpp
>+++ b/mailnews/base/src/nsMsgFolderCompactor.cpp
>@@ -1118,18 +1118,22 @@ nsFolderCompactState::EndCopy(nsISupport
>   /**
>    * Done with the current message; copying the existing message header
>    * to the new database.
>    * XXX This will need to be changed when we support local mail folders
>    * > 4GB. We'll need to set the messageOffset attribute on the new header,
>    * and assign the nsMsgKey some other way.

This XXX comment is obsolete or part-obsolete now I think?

>    */
>   if (m_curSrcHdr)
>-    m_db->CopyHdrFromExistingHdr((nsMsgKey) m_startOfNewMsg, m_curSrcHdr, true,
>+  {
>+    // if mbox is close to 4GB, auto-assign the msg key.
>+    nsMsgKey key = m_startOfNewMsg > 0xFFFFFF00 ? nsMsgKey_None : (nsMsgKey) m_startOfNewMsg;
>+    m_db->CopyHdrFromExistingHdr(key, m_curSrcHdr, true,
>                                getter_AddRefs(newMsgHdr));
>+  }

Please fix the indentation of the getter_AddRefs line.

r=me with that fixed.

Comment 5

[David :Bienvenu]

fixed on trunk, with comments addressed - http://hg.mozilla.org/comm-central/rev/93cd3473e73f

Comment 6

[Wayne Mery (:wsmwk)]

bienvenu, what precisely was the impact(s) of not having this patch?

Comment 7

[David :Bienvenu]

(In reply to Wayne Mery (:wsmwk) from comment #6)
> bienvenu, what precisely was the impact(s) of not having this patch?

I think messages after the 4GB mark could get lost during compaction, in TB 12.