Last Comment Bug 721925 - compaction of local folders > 4GB can cause problems
: compaction of local folders > 4GB can cause problems
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Backend (show other bugs)
: Trunk
: x86_64 Windows 7
: -- normal (vote)
: Thunderbird 13.0
Assigned To: David :Bienvenu
:
:
Mentors:
Depends on:
Blocks:
  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:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


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

Description David :Bienvenu 2012-01-27 17:11:01 PST
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 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 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 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 Mark Banner (:standard8, afk until Dec) 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 David :Bienvenu 2012-02-21 10:24:53 PST
fixed on trunk, with comments addressed - http://hg.mozilla.org/comm-central/rev/93cd3473e73f
Comment 6 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 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.