Last Comment Bug 721925 - compaction of local folders > 4GB can cause problems
: compaction of local folders > 4GB can cause problems
Product: MailNews Core
Classification: Components
Component: Backend (show other bugs)
: Trunk
: x86_64 Windows 7
-- normal (vote)
: Thunderbird 13.0
Assigned To: David :Bienvenu
Depends on:
  Show dependency treegraph
Reported: 2012-01-27 17:11 PST by David :Bienvenu
Modified: 2012-06-24 10:42 PDT (History)
6 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

proposed fix (1.17 KB, patch)
2012-02-02 12:50 PST, David :Bienvenu
standard8: review+
Details | Diff | Splinter Review

Description User image David :Bienvenu 2012-01-27 17:11:01 PST
The issue is here:

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 User image :aceman 2012-01-30 05:58:59 PST
How can the folder get > 4GB? I thought that is not supported. Isn't there a problem somewhere else too?
Comment 2 User image David :Bienvenu 2012-01-30 06:59:29 PST
As part of the pluggable store work, I made the berkeley mailbox format support folders > 4GB
Comment 3 User image David :Bienvenu 2012-02-02 12:50:59 PST
Created attachment 593946 [details] [diff] [review]
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 User image Mark Banner (:standard8) 2012-02-21 06:37:55 PST
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 User image David :Bienvenu 2012-02-21 10:24:53 PST
fixed on trunk, with comments addressed -
Comment 6 User image Wayne Mery (:wsmwk, NI for questions) 2012-06-24 08:31:52 PDT
bienvenu, what precisely was the impact(s) of not having this patch?
Comment 7 User image David :Bienvenu 2012-06-24 10:42:01 PDT
(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.

Note You need to log in before you can comment on or make changes to this bug.