User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20100101 Firefox/15.0.1
Build ID: 20120905151427
Steps to reproduce:
Moving a ~12 MiB message into a folder sized 4 GiB.
Actual results:
A pop-up windows appears instructing about the issue ("the folder cannot contain more messages, either compact or delete..."). The message cannot be moved to that folder.
Expected results:
I think nothing, given the new limits for the folder size should have been fixed. I'm running Thunderbird 15 on Debian Lenny (64-bits) and the file system where Thunderbird profile and messages are stored is formatted with ReiserFS. The folder was manually created (i.e., no special folder such Sent, Inbox, etc...) and this happens for a POP3 account (thus messages are stored under Local Folders). Compacting (which I usually run for the whole profile) did not help.

From Comment #1, should I take it as this warning is expected to be "normal"?
I thought the 4 GiB folder size limit was already something of the past (at least that's what can be read from the Limits page). In the end, is not the message what matters but the folder size limitation itself, should still exists.

Because bug 462665 was changed to FIXED after enhancement by bug 402392(internal offset value was perhaps changed from 32bits integer to signed 64bits integer),
> Bug 462665 - Remove the 4GB local mailbox file size limit
even if "removal of old size limit at any place" is still "work in progress", Tb should correctly support "2**63 bytes" or "file size limit by OS or File System including NFS or memory card" as "new file size limit" at any place.
As for "warning about 4GB(Win) or 2GB(Linux,Mac)", why it's mandatory is;
Once mail folder file size exceeds max value of 32bits unsigned/signed integer,
Tb can't process mail data above 4GB/2GB, and there is no way to recover from
mail folder file larger than 4GB/2GB.
After 64bits integer use as offset by Tb, "max file size supported by Tb" is far larger than "max file size by OS/File System". So, as far as Repair Folder(rebuild-index)/Compact/Move mail etc. works well, there is no need of "warning about exceeding file size".
Required new behaviour and new warning is;
If append of mail data fails due to file size limit of file system,
undo append(truncate at previous file size),
and warn user about copy/move/archive failure, download failure etc. due to
file size limit.
I believe this bug can't be INVALID.

@WADA
While I agree that Thunderbird should provide support for larger folder sizes (>4 GiB) this is a feature which has not been implemented yet thus this bug is, of course, invalid (please, note that despite the bug subject, the "warning" message is not the main issue here).
Feel free to open a new bug report if you think there's a different problem you estimate that needs to be reviewed or solved.

I just have read the forum posts, thanks for the pointer. It looks like more people is experiencing this behaviour.
By reading from Mozilla bug reports, I finally found this feature is not implemented and local "mbox" folders are still limited to 4 GiB of size, is this right? What's the current status about this? Can you please clarify?
Anyway, I will send a snapshot of the warning as soon as I move the messages back to the same folder, no problem, just let me some time.

(In reply to Camaleon from comment #7)
> By reading from Mozilla bug reports, I finally found this feature is not
> implemented and local "mbox" folders are still limited to 4 GiB of size, is
> this right? What's the current status about this? Can you please clarify?
I am not convinced yet that 4GB isn't correctly implemented. 3-4 reports is great, but isn't compelling, given we have millions of users. In short, we need to dig deeper.

(In reply to Wayne Mery (:wsmwk) from comment #8)
> I am not convinced yet that 4GB isn't correctly implemented. 3-4 reports is
> great, but isn't compelling, given we have millions of users.
I'd say more than three or four:
https://getsatisfaction.com/mozilla_messaging/tags/bug_462665?sort=recently_created&style=topics> In short, we need to dig deeper.
Well, after reading bug report #462665, I concluded this is not solved (as I understand, the limitation will be removed as soon as new mail storage alternatives are in play). I can be wrong, of course, that's why this point needs to be firmly confirmed by you, guys.

(In reply to Camaleon from comment #9)
> (In reply to Wayne Mery (:wsmwk) from comment #8)
>
> > I am not convinced yet that 4GB isn't correctly implemented. 3-4 reports is
> > great, but isn't compelling, given we have millions of users.
>
> I'd say more than three or four:
>
> https://getsatisfaction.com/mozilla_messaging/tags/
> bug_462665?sort=recently_created&style=topics
>
> > In short, we need to dig deeper.
>
> Well, after reading bug report #462665, I concluded this is not solved (as I
> understand, the limitation will be removed as soon as new mail storage
> alternatives are in play). I can be wrong, of course, that's why this point
> needs to be firmly confirmed by you, guys.
4GB support wasn't fully complete until TB12, which came out in April 2012. Although there were bits fixed in the year preceding it. But all the topics in https://getsatisfaction.com/mozilla_messaging/tags/bug_462665 are a year or more old.

(In reply to Wayne Mery (:wsmwk) from comment #10)
> 4GB support wasn't fully complete until TB12, which came out in April 2012.
> Although there were bits fixed in the year preceding it.
Can you please ellaborate that point? From the mentioned bug report #462665 I can't read the problem was fixed. According to Comment #57, the issue is still there.

(In reply to WADA from comment #12)
> FYI.
> Bug 462665 was changed to FIXED because enhancement by bug 402392 was made.
Can you please point to the exact patch that solved this? Bug 402392 is about discussing a new mailbox storage options.

Created attachment 661216[details]
Warning when I try to move a message on a folder that reached 4 GiB
This is what I get when I try to move a ~500 MiB message into a folder that curretnly is sized at 3,7 GiB:
-rw------- 1 sm01 sm01 3,7G sep 14 15:54 2012
-rw-r--r-- 1 sm01 sm01 1,1M sep 14 16:02 2012.msf
The warning is in Spanish but is the usual message: "The folder 2012 is full, and can't hold anymore messages. To make room for more messages, delete any old or unwanted mail and compact folder."

(In reply to Camaleon from comment #13)
> (In reply to WADA from comment #12)
> > FYI.
> > Bug 462665 was changed to FIXED because enhancement by bug 402392 was made.
> Can you please point to the exact patch that solved this? Bug 402392 is
> about discussing a new mailbox storage options.
You haven't read bug 462665 comment #57 by who created patch for bug 402392?
By the patch, in change of /mailnews/local/src/nsMovemailService.cpp, "PRUint64 messageOffset;" was introduced.
I don't understand reason why following is seen in the patch for Imap and Nntp module.
> - PRUint64 messageOffset;
> + PRInt64 messageOffset;

(In reply to Wayne Mery (:wsmwk) from comment #16)
> Is the message merely warning, and the copy or move actually works? I.e.
> you can access the messages added after the folder exceeds 4GB?
As mentioned at Comment #0, messages cannot be moved.

I can confirm the problem with both Thunderbird 15.0.1 and 12.0.1. I used the same profile with each version. I tested using a drafts folder for a POP account with a global inbox, with Windows 7/NTFS. I verified using a text editor that the almost empty mbox file was good and then saved drafts of plain text messages with large attachments rather than moving or copying messages.
I got the mbox file up to "3.99 GB (4,290,211,840 bytes)" according to "Size on disk" in the files properties in windows explorer. After that it started to become a problem finding files small enough to attach that wouldn't cause the error. When the error occurs nothing is saved to the mbox file.

Eric, thanks for testing this.
I don't know whether the previously "fixed" bugs were verified fixed, but it might be worth a look. Also, perhaps the tests to ensure correct functionality (I would guess/hope those fixes had tests) aren't working, or don't exist.

Hmm..I'm pretty sure the fixes applied did not take into account the file system in use. I still have a few system set up for Fat32, and according to Wicap.. 4GB isa design restraint for Fat32
"The maximum possible size for a file on a FAT32 volume is 4 GB minus 1 byte or 4,294,967,295 (232−1) bytes. This limit is a consequence of the file length entry in the directory table and would also affect huge FAT16 partitions with a sufficient sector size.[1] Video applications, large databases, and some other software easily exceed this limit."

(In reply to WADA from comment #21)
> Patch was proposed at bug 402392.
It was proposed and accepted?
> Have you looked into the bug where patch was proposed?
It's a 99-comments bug report with 3 attachments. The attachments filenames don't suggest a fix for this. Sorry, I don't want to sound rude, it's simply that can't locate the corresponding fix.

FYI.
Warning about "exceeding 4GB limit" on local mail folder was done by bug 387502 and bug 598104. This warning is required because "Tb's supported offset by 32bits integer" << "supported file size by file system".
"Removal of 4GB limit of IMAP offline-store file" was done by bug 532323.
Even if "Tb's supported offset by 64bits integer" >> "supported file size by file system", similar warning about "exceeding supported offset by 64bits integer" will be required in case that OS will support file size larger than max of 64bits integer, if Unix Mbox format will be continuously used without enhancement like "multiple Unix Mbox format files per a local mail folder".
Appropriate warning or error message about "exceeding file size limit of file system"(2GB if nfs like SMB) is needed if "multiple mails in Unix Mbox format file" will be continuously used.
"Pluggable mail store system by bug 402392" is a prerequisite of enhancement to "one file per a mail like qmail or maildir" or "SQLDB as mail store" which is needed to get rid of many restrictions/issues due to "multiple mails in a Unix Mbox format file".
Note:
Apple used "multiple mails in a Unix Mbox file" by Apple Mail Ver 1. But Apple already changed to "emlx==single mail in a Unix Mbox format file==one mail per a file" by Apple Mail Ver 2.

(In reply to WADA from comment #27)
> FYI.
>
> Warning about "exceeding 4GB limit" on local mail folder was done by bug
> 387502 and bug 598104. This warning is required because "Tb's supported
> offset by 32bits integer" << "supported file size by file system".
> "Removal of 4GB limit of IMAP offline-store file" was done by bug 532323.
Thanks for the note. The warning is not what worries me but the fact of a real limitation when a folder has reached > 4 GiB file size which seems to be the key issue here.

(In reply to Kent James (:rkent) from comment #29)
You are right.
4GB(2GB) limit was removed in back-end, but file size larger than 0xFFC00000 == 4GB - 4MB is still prohibited on local mail folder file(not IMAP offline-store) in code added by bug 387502 and bug 598104.
If Bug 462665 were for "removing size limit in back-end", this bug would be merely a followup request. But that bug is request of "removing file size limit in Tb" and has status of FIXED, this bug is apparently for fault in fixing Bug 462665 :-)

In doing some unrelated work, I came across important changes that were landed in Mozilla 17:
Bug 215450 - uploading files that are larger the 2GB fails
This extends the available function in nsIInputStream from 32 bit to 64 bit, and may have been needed to support 4 GB files.

With win7 x64 and Thunderbird 16.02, I was trying to move all my 2012 mail into one single folder (sent & received) and the size of these mails are 5GB, Inbox 3.5 and sent 1.5. Once I moved the inbox, I can't anymore move the Sent folder messages into that folder, Thunderbird claims folder is full.
So I guess I hit the 4gb local folders mbox file limit? Is this fixed now in TB 17?

To quote David Bienvenu who did most of the work on these bugs, in an email regarding Compaction he said the following
"Other than the extra disk space used, and the potential privacy issues of thinking deleted messages are gone from disk, the other problem that compaction fixes is the fact that we used to only allow folders of less than 4GB on disk, and couldn't add new messages to folders > 4GB. In TB 12, we got rid of the 4GB limit, so the main issues are wasted disk space and privacy."
Based on that and the fact he closed the original bug, there is nop doubt in my mind that he felt the issue was resolved and closed.

(In reply to Vincent (caméléon) from comment #37)
> I was thinking that Thunderbird now splits the mbox file in two if its size
> goes over 4gb. Am I wrong???
yes, you are wrong :)
regarding "the folder cannot contain more messages, either compact or delete...", why is this different from other warning like "The folder Inbox is full..."? Is it occurring in different places in the code?

(In reply to Domel from comment #42)
> In the newest TB 17, as well as all previous versions, it's still not
> possible to feed a regular mbox folder above 4 GB limit.
I *think* I hit the same bug and got curious and searched,found this entry.
I am using POP3 if this is important for this bug.
I was downloading a large number of e-mails for an account, each e-mail tends to have 100KB-3MB attachements (PDF for proofreading material). Since they came in rapid succession,
the folders grew rapidly.
The messages were filed into different folders based on the sender and some strings in the subjects.
During the course of downloading, TB complained that as in comment 1: "A pop-up windows appears instructing about the issue ("the folder cannot contain more messages, either compact or delete..."). The message cannot be moved to that folder."
[sorry I didn't record the exact wording myself, but I believe the message is the same.]
There are two things which surprised me when this happened.
1. I checked the folder size shown in the folder pane.
The particular folder to which the e-mail message was supposed to go into showed
a very small size of mere "254KB" (!?)
I think there is a bug in the printing code of the size.
(Printing only the lower 32bits of 64 bits offset maybe. This is a separate bug.)
2. So I quit TB and checked the file size of the folder in question.
ls -l showed:
-rw------- 1 ishikawa users 4295227305 2013-01-10 09:44 000assistant
-rw-r--r-- 1 ishikawa users 4147344 2013-01-10 10:00 000assistant.msf
There were other folders which were more than 3gb already.
I was pleasantly surprised to see that the size of 2GB of folder is now supported comfortably
on Linux. (After reading the comments above in this bugzilla entry, now I am not so sure, though).
Note the size. I used "bc" under linux to calculate some numbers.
$ bc
bc 1.06.95
Copyright 1991-1994, 1997, 1998, 2000, 2004, 2006 Free Software Foundation, Inc.
This is free software with ABSOLUTELY NO WARRANTY.
For details type `warranty'.
2^32
4294967296
f=4295227305
f - 2^32
260009
(f-2^32)/1024
253
quit
$
So I conclude that somehow the problematic folder became LARGER than 2^32 and
only then TB complained after the fact.
I thought we had put in some advance check for
2GB limit BEFORE the folder is modified (during TB2->TB3 transition period)
that would have warned us BEFORE the folder reaches 2GB so that removing/moving
from the folder still works. I wonder if the same is not done for handling 4GB limit.
is there no advance check for the size limit before the limit is reached?
One other thing is that the file size is larger than 2^32 by 253 KB and this matches my earlier
observation that maybe the printing of the size is flawed.
I was thrilled when I saw this error, but somehow after compacting, I could continue downloading.
*HOWEVER*, the message(s) that would have been put into the problematic folder were not
found there. I got panicky thinking that the e-mails were forever lost.
After searching, I found that they were sitting in the Inbox. It seems that TB first
downloads the message into Inbox safely, and then runs the filter and tries to move
the message as appropriately. Since the move fails, the original messages were still in Inbox.
Until I realized this, I was a little worried about losing the messages.
This is a usablity and HMI issue. But I am not sure how to tell the user where to expect "possibly unmoved messages may be still in Inbox".
This is because I am not entirely sure what happens *if Inbox overflows first.
Would I lose the e-mails or is TB clever enough to stop the deletion of message from POP3 account. I hope the latter is the case.
In any case, I am reporting
- the warning does get issued for 4GB limit.
- the printing of size is flawed.
- TB should stop moving files BEFORE the limit is reached.
We should allow some room (some arbitrary fixed size) before TB refuses downloading
a message into folder (See comment 30) even for POP3 and other situations.
(OK, make it downloading or moving/copying instead of simple "downloading".)
TIA
PS: This is the latest TB 17.0.2 on linux (TB was upgraded to this just before
the above mentioned , and it happened
under 32bit Debian GNU/Linux.
Sorry I forgot to check which file system (ext3, or ext4) it uses, but I can find out
if necessary.
PPS: I just realized that I posted two years ago a similar but slight different
experience (where TB kept on downloading although a folder reached a size limit)
Bug 634544
During download, when a mail folder reaches size limit, there is no way to stop download, and the folder can't compacted...
But since my experience yesterday showed TB stopped downloading somehow (or more exactly speaking moving the articles to the overflowing folder) and yesterday's experience is closely related to this 4GB limit, I will follow up with my checking of the code in this bugzilla entry.
Writing this comment helps me realize the cause of the slightly different manifestation of the bugs: there are different code paths for downloading and moving/copying the articles. to enter a new article into a folder: both of these need to have the advance check of size limit to avoid the type of problem mentioned here (not being able to delete/move any more after the limit is hit).
TIA

Have same issue. Generally I manually (or through filters) ensure my folders never exceed 2GB. But unfortunately some of them might still go that way (e.g. the All Mail folder of an IMAP GMail account).
Anyhow, the "bug" runs deeper than simply a warning and stopping from adding to the folder. If such adding has already occured and the folder exceeds the 4GB limit, there are at least 2 extremely worrying symptoms:
(1) The folder size is listed as the overflow of trying to indicate a > 4GB size in a unsigned 32bit integer. I.e. the actual size minus 4096GB (2^32).
(2) *MOST WORRYING OF ALL* The message count is also incorrect. It seems as if TB reads only up to the 4GB limit, then stops. So the client effectively only displays the messages up to that limit - the rest just disappears.

For space check and space shortage while downloading.
IIRC, bug report for next issue exists.
When Berkley Mbox size exceeds 4GB limit while new mail downloading,
warning of this bug is issued and download of mail fails,
and the error due to 4GB check occurs on any mail after first error,
and there is no way to stop the attempts of downloading mails.
So, as far as proper disk space check is effective, "data loss due to disk space issue while downloading" won't occur.
If 4GB size check in nsMsgBrkMBoxStore::HasSpaceAvailable will be removed without logic correction of nsMsgMaildirStore::HasSpaceAvailable, probability of "data loss due to disk full while downloading" will increase.

(In reply to :aceman from comment #41)
> There are still places in the code where it looks like the folder size is
> used as uint32_t, e.g.: (snip)
FYI. Bug 793865, Bug 794303 are example of problem due to it.

So what is the plan here?
I think we should leave the 4GB warning in place for now and first fix the hasAvailableSpace functions as WADA found and fix the handling of folder size as int32 (GetSizeOnDisk). That may fix the problems of wrong display size of folders.
I wonder if we can create a GetInt64Property as I do not see any yet.
It seems there is still test in http://mxr.mozilla.org/comm-central/source/mailnews/local/test/unit/test_over4GBMailboxes.js to check if we get the 4GB warning. It also includes ways to check the free disk space so that can probably be used to fix the hasAvailableSpace functions.

Created attachment 725187[details][diff][review]
make GetSizeOnDisk 64 bit
This could help the display of the folder size.
In my tests I could not go over 4GB, but I've only tried to copy messages. Maybe filters or POP3 download do not consult hasSpaceAvailable properly.
So I need anybody who has a folder larger than 4GB to try out this patch. Please backup the folder and also a repair may be necessary to see the folder size correctly. Chiaki and WADA, can you try it? Also, do you use the Extra folder columns extension so that you see the folder size? I will try to make a folder bigger than 4GB from outside TB but I also need somebody who managed to do it inside TB.

(In reply to :aceman from comment #50)
> Created attachment 725187[details][diff][review]
> make GetSizeOnDisk 64 bit
>
> This could help the display of the folder size.
> In my tests I could not go over 4GB, but I've only tried to copy messages.
> Maybe filters or POP3 download do not consult hasSpaceAvailable properly.
> So I need anybody who has a folder larger than 4GB to try out this patch.
> Please backup the folder and also a repair may be necessary to see the
> folder size correctly. Chiaki and WADA, can you try it? Also, do you use the
> Extra folder columns extension so that you see the folder size? I will try
> to make a folder bigger than 4GB from outside TB but I also need somebody
> who managed to do it inside TB.
I will try this over the weekend. I have been able to
use a local POP3 server to feed e-mails and so I can re-create the
condition rather easily (I HOPE(.
TIA

(In reply to :aceman from comment #50)
sary to see the
> Chiaki and WADA, can you try it?
If win32.zip tryserver build will be provided, I can test with "Extra Folder Column". I have Rexx script to generate Mbox file filled by many crafted mails of any size, of very long thread, althought it'll eat up my valuable 10GB free space of my HDD and CPU 100% for 10-to-15 minutes to generate 4GB Mbox... :-)

I have created a sub 4GB mail folder (Inbox) using a local POP3 server (downloading the e-mails from this server after sending a series of e-mails with attachment many times).
env LANG=C ls -l /COMM-CENTRAL/TEST-MAIL-DIR/Mail/127.0.0.1
total 4190676
-rw-rwx--- 1 mtest2 ishikawa 4282409531 Mar 17 07:15 Inbox
-rw-rwxr-- 1 mtest2 ishikawa 3567433 Mar 17 09:47 Inbox.msf
-rw-rwx--- 1 mtest2 ishikawa 5247690 Mar 17 01:59 Sent
-rw-rwxr-- 1 mtest2 ishikawa 3170 Mar 17 02:07 Sent.msf
-rw-rwx--- 1 mtest2 ishikawa 0 Mar 17 01:38 Trash
-rw-rwxr-- 1 mtest2 ishikawa 1320 Mar 17 02:00 Trash.msf
-rw-rwxr-- 1 mtest2 ishikawa 25 Mar 17 01:38 msgFilterRules.dat
-rw-rwx--- 1 mtest2 ishikawa 61 Mar 17 09:49 popstate.dat
The file system on which this Mail directory resides is ext4 under 32-bit Debian GNU/Linux.
(I am testing 32 bit version.)
Because my system-wide mail spool in this development linux image is not that large, I had to send many small e-mails at a time and download it so as not to
overflow the mail spool. It took me overnight to create this.
(One run adds about 103592600 bytes to the mail folder. [This consists of 200 e-mails.]. I had to add "sleep 2" between each e-mail, and "sleep 60" between
one batch run so that TB has chance to download the e-mails from POP3 spool area
before the incoming test e-mails would fill up system root partition (/var/mail, and /var/spool/pop3. Hmm, testing TB is not easy is it?)
I am saving this mail folder and .msf files for later experiments.
So far, no discernable problems. (So 2G+ mail folder is certainly working more or less.)
BTW, I do use Extra Column extension so that I can see the folder size.
With the file size shown above, the extension shows 4084 MB.
(BTW, we need to check three passes in TB where the folder size (precisely
speaking mail filder implemented as single file) may cause problems.
POP3 -> Inbox [I am testing THIS.]
IMAP -> Inbox
Inbox -> other folders (filtering or manual)
If I am not mistaken, I think the sending of e-mail and saving the sent
mail into Sent may also be a different path.
Tough luck. )
Stay tuned.

Here is the continuation of test, still work-in-progress.
(Without your patch)
This is the dialog I got when the mail folder size goes over 4GB.
The folder Inbox is full, and can't hold any more messages. To make
room for more messages, delete any old or unwanted mail and compact
the folder.
Then the extra column extension shows Inbox as having 8298 e-mails
(8284 unread) and size is "382KB" while actually it has the size shown
in the following listing.
total 4203388
-rw-rwx--- 1 mtest2 ishikawa 4295358606 Mar 17 10:23 Inbox
-rw-rwxr-- 1 mtest2 ishikawa 3634842 Mar 17 10:26 Inbox.msf
-rw-rwx--- 1 mtest2 ishikawa 5247690 Mar 17 01:59 Sent
-rw-rwxr-- 1 mtest2 ishikawa 3170 Mar 17 02:07 Sent.msf
-rw-rwx--- 1 mtest2 ishikawa 0 Mar 17 01:38 Trash
-rw-rwxr-- 1 mtest2 ishikawa 1320 Mar 17 02:00 Trash.msf
-rw-rwxr-- 1 mtest2 ishikawa 25 Mar 17 01:38 msgFilterRules.dat
-rw-rwx--- 1 mtest2 ishikawa 61 Mar 17 10:23 popstate.dat
Funny, I thought there was this advance check for 10MB room, but
obviously it did not kick in. (Either the check is not there, or
put in an incorrect place, or is now removed altogether?.)
This was without your patch.
Now with your patch.
[I created the binary using TryServer. Thank you for the earlier tips
regarding TryServer. But due to my linux-centric development
environment, it seems that I can't create win binary, etc. My files
have LF as end of the line terminator and not CRLF and this seems
to interfere with Win build(?). Anyway, you can obtain a copy of linux binary from
> Results will be displayed on TBPL as they come in:
> https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=e06da28d79ad
>
> Once completed, builds and logs will be available at:
> http://ftp.mozilla.org/pub/mozilla.org/thunderbird/try-builds/ishikawa@yk.rim.or.jp-e06da28d79ad]
I reverted the Inbox and its .msf file to the
sub-4GB status and repeated
the final addition of [200 e-mails] to go across 4GB size boundary.
This is again the "before" state:
drwxrwxr-x 2 mtest2 ishikawa 4096 Mar 17 01:52 .
drwxrwx--- 4 mtest2 ishikawa 4096 Mar 17 01:38 ..
-rw-rwx--- 1 mtest2 ishikawa 4282409531 Mar 17 11:29 Inbox
-rw-rwxr-- 1 mtest2 ishikawa 3610876 Mar 17 11:29 Inbox.msf
-rw-rwx--- 1 mtest2 ishikawa 5247690 Mar 17 11:29 Sent
-rw-rwxr-- 1 mtest2 ishikawa 3170 Mar 17 11:29 Sent.msf
-rw-rwx--- 1 mtest2 ishikawa 0 Mar 17 11:29 Trash
-rw-rwxr-- 1 mtest2 ishikawa 1320 Mar 17 11:29 Trash.msf
-rw-rwxr-- 1 mtest2 ishikawa 25 Mar 17 11:29 msgFilterRules.dat
-rw-rwx--- 1 mtest2 ishikawa 61 Mar 17 11:29 popstate.dat
I am using linux 32 build with your patch:
For reasons I don't quite understand (maybe because of the copying
from the backup, Inbox had a newer timestamp than Inbox.msf?), the new
TB tried to re-create .msf and it took a long time for 4GB mail
folder.
Once it is done, it shows again 8259, 8273, 4084MB.
TEST:
I injected e-mails to this account from a different window.
After downloading some e-mails, TB showed 4098MB
as Inbox size (so the display seems to be correct now.)
However, I didn't see the automatic downloading of the next minute and
got anxious when I see the following dialog.
"The folder Inbox is full, and can't hold any more messages. To make
room for more messages, delete any old or unwanted mail and compact
the folder." [OK]
(Well, I though ext4 supports file system size> 4GB,
but maybe the individual file can't go over this size?)
At this stage, the file size has become this way.:
/COMM-CENTRAL/TEST-MAIL-DIR/Mail/127.0.0.1:
合計 4204872
drwxrwxr-x 2 mtest2 ishikawa 4096 Mar 17 11:33 .
drwxrwx--- 4 mtest2 ishikawa 4096 Mar 17 01:38 ..
-rw-rwx--- 1 mtest2 ishikawa 4296912495 Mar 17 11:41 Inbox
-rw-r--r-- 1 mtest2 mtest2 3594866 Mar 17 11:45 Inbox.msf
-rw-rwx--- 1 mtest2 ishikawa 5247690 Mar 17 11:29 Sent
-rw-rwxr-- 1 mtest2 ishikawa 3170 Mar 17 11:29 Sent.msf
-rw-rwx--- 1 mtest2 ishikawa 0 Mar 17 11:29
According the caluculation of bc
m=2^32
4296912495 - m
1945199
So the Inbox folder file size is larger than 2^32 by 1945199 bytes.
(So ext4 does seem to support > 4GB size. Or does the kernel and driver need to be compiled with special flag?
The following URL suggests that ext4 can support a file larger than 2Ti !
https://ext4.wiki.kernel.org/index.php/Frequently_Asked_Questions#History_of_ext2.2C_ext3.2C_and_ext4
)
Anyway, with this Inbox I did some tests:
Test 1: I tried to copy one e-mail from another folder
(Sent).
I get the same warning. Good.
"The folder Inbox is full, and can't hold any more messages. To make
room for more messages, delete any old or unwanted mail and compact
the folder." [OK]
So I cannot copy something to this full Inbox.
(At least, I will not go into more problems.)
Test 2: I tried to delete one e-mail from Inbox to Trash.
(I chose the last one)
*SOME FISHY behavior* : Instead of the last e-mail, I am afraid
an e-mail article near the beginning of the mail folder (at least the
subject line suggests so) is now moved to Trash(!?)
I think the article was somewhere near the 20th position of the Inbox.
(I injected some smallish test e-mails when I started testing, so
I can not co-relate the mail article position easily with the position of the
deleted last article. The later
articles have exactly the same size [modulo the difference of system
generated header lines, etc.] .)
Test 2-a: I tried to delete the now shown last e-mail article from
Inbox to Trash.
This time, the deleted article seems to correctly show up in Trash.
Test 2-b: I tried to delete the now shown last e-mail article from
Inbox to Trash one more time. This time again, the deleted article seems to
correctly show up in Trash.
So now trash has three articles. One from near the beginning of Inbox
and two from the end... OK, where did the last one (I deleted first) go ?
I used some numbers (sometimes repeated for each batch) in the subject
line for easy identification. Currently the subject list shows 3521,
3522, 3533 in the inbox pane at the end. Trash showed "test attachment", 3524,
3525 (Instead of "test attachment" article from near the beginning of
Inbox, I thought I would have 3526 in the Trash when I deleted the
"last" one for the first time. Now I will compactify Inbox and see what
happens. I suspect article with subject 3526 may show up again Inbox !)
But I hit a snug.
I forgot that I don't have that much space left in the file system and
I got:
"The folder 'Inbox' could not be compacted because writing to
folder failed. Verify that you have enough disk space, and that you
have write privileges to the file system, then try again.compact "
Well, getting this warning is good. [Without proper checking I lost
some mails, 5 or 6 years ago. :-( ]
Here is where I am now. I will create more disk space by attaching
virtual disk image (I am testing using a vm image) and
I will check what happens after compaction, and then plan to test
another scenario:
Each e-mail with attachment is abut 600KB, and so I will delete the last
10 e-mails from Inbox and compactified to see what happens.
I may be able to finish this later today, but may not finish it over
the weekend after all.
TIA

I am concerned that the deletion of the wrong message behaviour is because the message keys used to identify messages are 32-bit and for local folders are the actual 32-bit position of the message within the mbox file. By comparison I believe the keys for IMAP are simply the server's UIDs although I am not certain as to how the offline store is maintained.

Thanks for your experiments guys!
For reference, 4GB is 4096MB but the check on mbox size should only allow to grow up to 4092MB.
Chiaki, it seems you have low disk space in the VM, could you please also test the new feature of the "folder is full" dialog coming up when the Inbox fills your disk even before the Inbox size is 4GB large. The "inbox full" should show if 1) the new message would grow the inbox above 4GB-4M 2) or if the free disk space after the new message would become less than 1MB (we can discuss if this is not too low for modern OSes). I have checked this to work on Linux, but it would be good if you doublechecked it on Windows too.
(In reply to ISHIKAWA, chiaki from comment #58)
> For reasons I don't quite understand (maybe because of the copying
> from the backup, Inbox had a newer timestamp than Inbox.msf?), the new
> TB tried to re-create .msf and it took a long time for 4GB mail
> folder.
Yes, you need to take both Inbox and Inbox.msg from backup so they are in the same state and timestamp.
> After downloading some e-mails, TB showed 4098MB
> as Inbox size (so the display seems to be correct now.)
Great!
> However, I didn't see the automatic downloading of the next minute and
> got anxious when I see the following dialog.
>
> "The folder Inbox is full, and can't hold any more messages. To make
> room for more messages, delete any old or unwanted mail and compact
> the folder." [OK]
>
> (Well, I though ext4 supports file system size> 4GB,
> but maybe the individual file can't go over this size?)
I intentionally did not remove the limit of 4GB-4MB for mbox. So it seems you still managed to grow above it when downloading new msgs (not copying). We need to find why this is happening. Probably the download code does not call hasSpaceAvailable.
> Anyway, with this Inbox I did some tests:
>
> Test 1: I tried to copy one e-mail from another folder
> (Sent).
>
> I get the same warning. Good.
>
> "The folder Inbox is full, and can't hold any more messages. To make
> room for more messages, delete any old or unwanted mail and compact
> the folder." [OK]
> So I cannot copy something to this full Inbox.
> (At least, I will not go into more problems.)
Good!
>
> Test 2: I tried to delete one e-mail from Inbox to Trash.
> (I chose the last one)
>
> *SOME FISHY behavior* : Instead of the last e-mail, I am afraid
> an e-mail article near the beginning of the mail folder (at least the
> subject line suggests so) is now moved to Trash(!?)
> I think the article was somewhere near the 20th position of the Inbox.
> (I injected some smallish test e-mails when I started testing, so
> I can not co-relate the mail article position easily with the position of the
> deleted last article. The later
> articles have exactly the same size [modulo the difference of system
> generated header lines, etc.] .)
May be bug 793865. That is why I do not lift the 4GB limit for now.
> Test 2-a: I tried to delete the now shown last e-mail article from
> Inbox to Trash.
>
> This time, the deleted article seems to correctly show up in Trash.
>
> Test 2-b: I tried to delete the now shown last e-mail article from
> Inbox to Trash one more time. This time again, the deleted article seems to
> correctly show up in Trash.
Good.
(In reply to WADA from comment #61)
> Aha!, msgStore was obtained by following code.
> > 3583 nsCOMPtr<nsIMsgPluggableStore> msgStore;
> > 3584 nsresult rv = GetMsgStore(getter_AddRefs(msgStore));
> nsMsgBrkMBoxStore::HasSpaceAvailable is perhaps called when Berkley Mbox
> after PluggableStore support.
I think you all use nsMsgBrkMBoxStore::HasSpaceAvailable. You haven't mentioned anywhere that you had manually migrated your account to the maildir-lite message store.
(In reply to neil@parkwaycc.co.uk from comment #62)
> I am concerned that the deletion of the wrong message behaviour is because
> the message keys used to identify messages are 32-bit and for local folders
> are the actual 32-bit position of the message within the mbox file.
May be bug 793865, so I do not lift the 4GB limit. But it seems we do not check it consistently and sometimes grow above it.

(In reply to ISHIKAWA, chiaki from comment #58)
> Funny, I thought there was this advance check for 10MB room, but
> obviously it did not kick in. (Either the check is not there, or
> put in an incorrect place, or is now removed altogether?.)
Seems this can be bug 640371 so we could move there to not clutter the bug here any more.

FYI. Size check by POP3 or Copy/Move is done via WarnIfLocalFileTooBig.
(i) Upon both POP3 download and Copy/Move to Berkley Mbox,
nsMsgLocalMailFolder::WarnIfLocalFileTooBig is called,
and WarnIfLocalFileTooBig requests HasSpaceAvailable.
> msgStore->HasSpaceAvailable(this, 0xFFFFF, &spaceAvailable);
> (FFFFF=1024**2=1MB)
(ii) nsMsgBrkMBoxStore::HasSpaceAvailable does following file size check only, without checking actual free space of HDD.
> *aResult = ((fileSize + aSpaceRequested) < 0xFFC00000);
> Checks ( Current file size + 1MB ) < (4GB -4MB)
(iii-A) If current file size < 4GB - 4MB - 1MB, true is returned.
So, if Disk Full condision occurs in following Copy/Move,
data loss may occur if POP3.
(iii-B) If current file size >= 4GB - 4MB - 1MB, false is returned,
then "The folder %S is full" is shown.
In this case, "mail dala loss in mail store due to disk full"
won't occur, unless Compact will corrupt mail store.
> 81 mailboxTooLarge=The folder %S is full, and can't hold any more messages. To make room for more messages, delete any old or unwanted mail and compact the folder.
This in Berkley Mbox will be resolved by aceman's patch followed by removel of "4GB-4MB" check.
When non-Berkley(maildir), file size check is not needed, because 'one mail per a file' and mail size is limited by 32bits integer(2GB or 4GB).
However, even if non Berkley, check of 'free disk space size' is mandatory. nsMsgMaildirStore::HasSpaceAvailable in nsMsgMaildirStore.cpp should be re-coded correctly too.

(In reply to :aceman from comment #63)
> I intentionally did not remove the limit of 4GB-4MB for mbox. So it seems
> you still managed to grow above it when downloading new msgs (not copying).
> We need to find why this is happening. Probably the download code does not
> call hasSpaceAvailable.
If current file size < 4GB - 4MB - 1MB, hasSpaceAvailable returns true, then POP3 download code appends mail data to Berkley mail store, thus file size will exceed 4GB if downloaded mail size is larger than 5MB.
However, offset(MsgKey) is kept within 32bits integer by "4GB - 4MB" check in hasSpaceAvailable, and, because any data can not be appended to mail store any more by "4GB - 4MB" check, critical problem like data loss won't occur. This is the reason why the "4GB - 4MB" check was introduced.
Even after the "4GB - 4MB" check, problem of "Sent larger than 4GB" occured, because "Copy to Sent after mail send" is different from Copy/Move and POP3. This problem was perhaps resolved by calling hasSpaceAvailable. So, "sent mail larger than 5MB" may produce "file size larger than 4GB" too.

(In reply to neil@parkwaycc.co.uk from comment #62)
> I am concerned that the deletion of the wrong message behaviour is because
> the message keys used to identify messages are 32-bit and for local folders
> are the actual 32-bit position of the message within the mbox file.
> By comparison I believe the keys for IMAP are simply the server's UIDs although
> I am not certain as to how the offline store is maintained.
By change for Pluggable message store support, Offset in message tore==Berkley Mbox(64bits integer) and MsgKey(unique identifier of message, also 64bits integer) is cleanly separated.
This is similar to IMAP offline-store. In IMAP, MsgKey is UID of mail, and Offset is 64bits integer of offset of mail data in offline-store file.
In almost all places, MsgKey==Offset in Berkley Mbox file. However, MsgKey!=Offset occurs in followig situation.
When multiple mails are consecutively moved to a FolderX by message
filer upon mail download,
moved mail #1 : Offset #1 = File Size before append mail data
MsgKey #1 = File Size before append mail data
moved mail #2 : Offset #2 = Offset #1 + Mail size #1,
MsgKey #2 = MsgKey #1 + 1
|
moved mail #N : Offset #N = Offset #(N-1) + Mail size #(N-1),
MsgKey #N = MsgKey #(N-1) + 1
"Move to local" only phenomenon. If Filter Copy, MsgKey==Offset.
Quarantine option may be relevant.
Note-1:
Uniqueness of MsgKey is always kept, because Offset is always unieque.
In ordinal Copy/Move, download to Inbox without filter move to local mail folder, MsgKey==Offset is always kept.
Even when filter move upon download, if number of moved mail is one, MsgKey==Offset is kept.
When MsgKey!=Offset, it is change to MsgKey==Offset by Compact, Copy/Move.
Note-2:
Even when MsgKey!=Offset, message is accessed via the unieque MsgKey, and read/write of mail data is done via MsgKey=>Offset conversion, so, "wrong mail delete" can't occur, as far as correctness of MsgKey=>Offset conversion is kept and correctness of message size is kept, unless someone copies Offset value to 32bits integer and uses it when offset of a mail is greater than 4GB.

Currently, following problems are known(set in Blocks: of this bug).
Bug 634544 : Unable to stop POP3 download when reached at "4GB-4MB"
Bug 640371 : File size can exceed 4Gb by POP3 mail download,
even when "4GB - 4MB - 1MB" warning is effective
Bug 794303 : Compact fails, even when file size is less than 4GB
These may be simple "32bit integer" problem, but different problem may be involved. And, if POP3 download is relevant, required disk space calculation, filter move, quarantine option, etc. may be relevant. Further, care for "Sent mail copy" is also needed.
To remove "4GB - 4MB" check in HasSpaceAvailable by this bug, resolving of all above problems is needed.

Yes, good summary WADA.
What the patches here do for now is this:
- In addition to 4GB-4MB check, also check the free disk space in hasSpaceSvailable to prevent dataloss (saving incomplete message or something). The disk space check is also done for maildir.
- If it happens that folder grows above 4GB show the size properly (via Extra folder columns) and folder -> properties. To avoid user panic that the whole folder is lost as the size shrunk to some small size (size minus 4GB). This work is also needed for future removal of the 4GB limit.
Yes, none of the patches/bugs intend to remove the 4GB (32bit) limit on single message size. But that seems fine for the foreseeable future.

If the changes here are deemed OK, I can also convert expungedBytes to 64bit as somebody may need to clear whole >4GB folder and it may fail with 32bit expungedBytes variable.
WADA, nsMsgKey is still uint32_t (unsigned long) at http://mxr.mozilla.org/comm-central/source/mailnews/base/public/MailNewsTypes2.idl#6 . Where do you see it is 64bit?
It is just not used as the message position in the mbox file if the value is nearing 4GB.

(In reply to WADA from comment #70)
> By change for Pluggable message store support, Offset in message
> tore==Berkley Mbox(64bits integer) and MsgKey(unique identifier of message,
> also 64bits integer) is cleanly separated.
I was referring to this comment of yours.
(In reply to WADA from comment #79)
> Following may be a practical mid range solution for removing 4GB-4MB-1MB
> size limit, because Mork looks to ordially assign highest+1.
> > 1134 nsMsgKey key = m_startOfNewMsg > 0xFFFFFF00 ==> 0x8FFFFFFF
This could be good. Because what if there are only small messages that fill the mbox until 4GB - 256 file size. The next message added gets message key of 4GB - 256 + 1. That means there is only space for new 256 messages when that happens. And that is bad.

(In reply to :aceman from comment #80)
> I was referring to this comment of yours.
Sorry, my misunderstood, mistake.
How about next?
> 1134 nsMsgKey key = m_startOfNewMsg >= 0x00000000
Because messageKey and messageOffset is already cleanly separated, I think there is no need to use Offset value as messageKey even in Berkley Mbox file.

(In reply to WADA from comment #81)
> > 1134 nsMsgKey key = m_startOfNewMsg >= 0x00000000
> Because messageKey and messageOffset is already cleanly separated, I think
> there is no need to use Offset value as messageKey even in Berkley Mbox file.
So making message key just be an increasing counter even for low offsets?
That looks nice, but I am not sure that is possible to do (or for existing mboxes).

FYI.
As known by Process Monitor log of Compact in bug 794303 comment #15, current Compact is continuous "read into 8KB buffer" + "write from 4KB buffer". If bigger Berkley Mbox file than current will be officially supported by resolvig this bug, issue of bug 558528 should be resolved.

Created attachment 732999[details][diff][review]
make GetSizeOnDisk 64 bit v3
Fixed nits. So I propose to change OnFolderSizePropertyChanged to 64bit and use OnItemLongPropertyChanged (create it or convert OnItemIntPropertyChanged to 64bit) in next patch. Something similar is also attempted in bug 813459.
But we actually do not enable 32bit+ values to flow across these functions (hasSpaceAvailable observes 4GB limit) so we could apply this patch as is.

Comment on attachment 732996[details][diff][review]
fix hasSpaceAvailable v4
Review of attachment 732996[details][diff][review]:
-----------------------------------------------------------------
::: mailnews/local/src/nsMsgBrkMBoxStore.cpp
@@ +198,5 @@
> +
> + // TODO: We are working on allowing mboxes > 4GB, but it is not ready yet.
> + // So allow the mbox to only reach 0xFFC00000 = 4 GiB - 4 MiB for now.
> + *aResult = // ((fileSize + aSpaceRequested) < 0xFFC00000) &&
> + DiskSpaceAvailableInStore(pathFile, aSpaceRequested);
Did you mean to leave the <4GB test commented out in this patch? If so, I'd prefer that you duplicate the whole statement and leave one version commented out, rather than having the comment start in the middle of the statement. That is,
// TODO: blah blah
// *aResult = test 1 &&
// test 2;
*aResult = test 2;
and make sure that some bug (possibly this one) tracks the TODO so that it will be cleaned up.
Otherwise, remove both the TODO: comment and the commented out text.

Comment on attachment 732999[details][diff][review]
make GetSizeOnDisk 64 bit v3
Review of attachment 732999[details][diff][review]:
-----------------------------------------------------------------
Looks fine aside from the NotifyIntPropertyChanged issue. Not sure what we want to do about that. Standard8? Should we just change that to "long long"? I'd hate to have to audit all the code that uses folder int property listeners to register specifically the folder size under a different listener.
::: mailnews/imap/src/nsImapMailFolder.cpp
@@ +4820,1 @@
> NotifyIntPropertyChanged(kFolderSizeAtom, oldFolderSize, mFolderSize);
NotifyIntPropertyChanged is still defined in IDL as having 'long' (== int32_t) parameters for the before and after integers.

(In reply to :aceman from comment #94)
> Oh, that would be bad, we need at least uint32 for the current 4GB-4MB.
The problem may be if some folder properties need negative values. Then we can't change to uint32 but would need to go to int64. As we discuss in bug 813459. However, it is still not sure if we really have such a property.

Sorry for disturbing your work but I've noticed the title change of this bug report and now looks like since Thunderbird relase 12.0 there's support for folders over 4 GiB which I guess is not still the case (that's the reason why the original report was opened).
Can someone explain -in plain words- what's the current status of the original bug report? Thanks.

We believe over 4GB folders are only supported if you manually switch to the new maildir-lite message store, not the default mbox.
However, I am not sure even then the folder size would be reported correctly.

(In reply to :aceman from comment #98)
Thanks for your reply.
> We believe over 4GB folders are only supported if you manually switch to the
> new maildir-lite message store, not the default mbox.
Mmm... and how can that be done? I mean, is that new "maildir-lite" storage option available right now? If so, how users can choose it for new accounts or switch to it from a current profile? Where is it documented? This is completely new to me, I mean, I know there's some work on the way but still not stable/well tested enough to be used by now.
> However, I am not sure even then the folder size would be reported correctly.
May I ask what are you now trying to fix in this bug report? I'm completely lost :-)

(In reply to Camaleon from comment #99)
> Mmm... and how can that be done? I mean, is that new "maildir-lite" storage
> option available right now? If so, how users can choose it for new accounts
> or switch to it from a current profile? Where is it documented? This is
> completely new to me, I mean, I know there's some work on the way but still
> not stable/well tested enough to be used by now.
See bug 58308 on how to enable it. But backup your profile as there seem to be some bugs, see bug 845952.
> > However, I am not sure even then the folder size would be reported correctly.
>
> May I ask what are you now trying to fix in this bug report? I'm completely
> lost :-)
Exactly this. The folder size is not reported properly if a folder is over 4GB (regardless of mbox or maildir).
Also, we found some problems why mbox folder can't be allowed to grow above 4GB so we are fixing them here and in the dependent bugs.
So over 4GB mbox folders will only be allowed as the last patch in this bug.

(In reply to :aceman from comment #100)
> See bug 58308 on how to enable it. But backup your profile as there seem to
> be some bugs, see bug 845952.
So I guess this is not yet an option for plain users and the 4 GiB limit is still in place. Okay.
> Exactly this. The folder size is not reported properly if a folder is over
> 4GB (regardless of mbox or maildir).
But the warning is still useful so people can be noticed about the folder size limit. Why remove it?
> Also, we found some problems why mbox folder can't be allowed to grow above
> 4GB so we are fixing them here and in the dependent bugs.
Mmm, so are you guys following two different paths to solve this problem so users can decide which one to take?
1/ Allow >4 GiB folders over mbox
2/ Allow >4 GiB folders by using the new maildir-lite storage
That would be simply great :-)
> So over 4GB mbox folders will only be allowed as the last patch in this bug.
Which is not still added to the realease branch. Okay, and thanks for the info.

(In reply to Camaleon from comment #101)
> > Exactly this. The folder size is not reported properly if a folder is over
> > 4GB (regardless of mbox or maildir).
>
> But the warning is still useful so people can be noticed about the folder
> size limit. Why remove it?
We do not remove it, we actually make it more strict (the 'hasSpaceAvailable' patch) for now.
> > Also, we found some problems why mbox folder can't be allowed to grow above
> > 4GB so we are fixing them here and in the dependent bugs.
>
> Mmm, so are you guys following two different paths to solve this problem so
> users can decide which one to take?
>
> 1/ Allow >4 GiB folders over mbox
> 2/ Allow >4 GiB folders by using the new maildir-lite storage
>
> That would be simply great :-)
Yes.
> > So over 4GB mbox folders will only be allowed as the last patch in this bug.
>
> Which is not still added to the realease branch. Okay, and thanks for the
> info.
This patch does not even exist yet. We first need to fix the dependent bugs.

(In reply to ISHIKAWA, chiaki from comment #104)
> As it turns out MS C++ compiler does not have atoll() and so the compilation
> fails for WIN32 on TB TryServer.
> We could either
> - reverse atoll() to atol() assuming that no sane download contains more
> than 32-bit amount of data (as reported as the sum by POP3 protocol), or
I am not sure we can assume this. If you have a >4GB gmail account and your local copy is lost, you may want to redownload it again.
I did the change purposefully so that any size we can store in local folder we must be able to download too.
Yeah, it seems atoll is part of C++11 so may not be in all compilers. I'll try to find another function.
Could you check if strtoll exists in MS C++ ?
Can anybody comment if conditional compilation is allowed in the tree?

There is an implementation of atoll in nsCRT.{h,cpp} that is used several places in the tree; you can find this sort of thing by searching in the Mozilla source cross-reference at mxr.mozilla.org/comm-central/. There is also a newer fancier implementation for mozilla-central at dxr.mozilla.org, but this index does not currently include the Thunderbird comm-central sources.
http://dxr.mozilla.org/search?tree=mozilla-central&q=atoll&redirect=false
In general, yes, conditional compilation is allowed but we try to limit it as much as possible; in the case of missing library functions like atoll we try to keep the conditional compilation in a compatibility header file rather than spreading all through the code base.

(In reply to :Irving Reid from comment #106)
> There is an implementation of atoll in nsCRT.{h,cpp} that is used several
> places in the tree; you can find this sort of thing by searching in the
> Mozilla source cross-reference at mxr.mozilla.org/comm-central/. There is
> also a newer fancier implementation for mozilla-central at dxr.mozilla.org,
> but this index does not currently include the Thunderbird comm-central
> sources.
>
> http://dxr.mozilla.org/search?tree=mozilla-central&q=atoll&redirect=false
>
> In general, yes, conditional compilation is allowed but we try to limit it
> as much as possible; in the case of missing library functions like atoll we
> try to keep the conditional compilation in a compatibility header file
> rather than spreading all through the code base.
It is great to learn strtoll() is in mozilla tree.
So we can simply change atoll() to strtoll() and all is fine. (Oh, we should
include nsCRT.h?)
MS C++ compiler does not still support it natively as of last summer.
http://connect.microsoft.com/VisualStudio/feedback/details/758053/missing-strtold-strtoll-strtoull-functions-from-stdlib-h

(In reply to :aceman from comment #108)
> I haven't seen strtoll, but I'll use nsCRT:atoll() .
> Yes, it needs to include nsCRT.h . I'll attach an updated patch if you wish.
Sorry, I got a little confused. atoll() is it.
strtoll() is used in a few places in mozilla, but I found that they are
inside the #if/#else/#endif type constructs to handle the portability issues with
MS C++ compiler.
If you can create an updated patch, I will appreciate it. I have been compiling locally and submitting thunderbird tryserver jobs with your patches to make sure
everything compiles OK. (I have noticed a few GC-related errors, etc. on top of
the usability bugs I found and am trying to figure out if I can dig the cause of
the problems.)
TIA

(In reply to :aceman from comment #110)
> Created attachment 738666[details][diff][review]
> make GetSizeOnDisk 64 bit v4
>
> OK, here is the updated patch, also extended to cover news folders now (as
> they just now got folder size display:))
Thank you I will replace the old patch with this one and test it out.
(Now news folder also has size display? How interesting. Compared with e-mail,
I don't think news folder god that big. Oh wait, for old-fashined alt.sources, etc., it can become quite large, I suppose.)

Comment on attachment 738666[details][diff][review]
make GetSizeOnDisk 64 bit v4
Review of attachment 738666[details][diff][review]:
-----------------------------------------------------------------
I could compile the binary for win32 API (MS VS C+ compiler) with the modification of atoll() to use nsCRT version.
Sorry, it took a while since I had a disk issue in the middle of last week, and lost the HG MQ series locally, and took a while to
figure out how to recover. (Not sure if recovered completely or not. I messed up badly. But TryServer compilation and test for WIN32 and linux 64 worked without an error, and linux worked both locally and TryServer. Thank you.
[I have not used the test program you posted in bugzilla. Not sure where to add it. Maybe in the mozmill test suite directory and
mention the name in mozmilltestlist or some such file?]

(In reply to :Irving Reid (On vacation, responding sporadically) from comment #93)
> Comment on attachment 732999[details][diff][review]
> -----------------------------------------------------------------
>
> Looks fine aside from the NotifyIntPropertyChanged issue. Not sure what we
> want to do about that. Standard8? Should we just change that to "long long"?
> I'd hate to have to audit all the code that uses folder int property
> listeners to register specifically the folder size under a different
> listener.
Standard8, can we get your opinion here so we can move forward?

(In reply to :aceman from comment #117)
> (In reply to :Irving Reid (On vacation, responding sporadically) from
> comment #93)
> > Comment on attachment 732999[details][diff][review]
> > -----------------------------------------------------------------
> >
> > Looks fine aside from the NotifyIntPropertyChanged issue. Not sure what we
> > want to do about that. Standard8? Should we just change that to "long long"?
> > I'd hate to have to audit all the code that uses folder int property
> > listeners to register specifically the folder size under a different
> > listener.
I think it would be good to whip up a separate patch that changes that listener to long long. It shouldn't affect javascript uses, as they'll just absorb it, the c++ uses would be the ones I'm worried about.
If we didn't change it to long long, then I'd say we'd need to audit everything anyway to check the effects of the 64 -> 32 bit notification change.

(In reply to :aceman from comment #119)
> Would you be OK with keeping the name as NotifyIntPropertyChanged while
> changing to long long so as not to rewrite all callers?
Yes, I think that's probably fine, although there's not that many callers. Note that we're going to need to change nsIFolderListener as well - but that name can stay the same as well (as long as we change the iid), as javascript doesn't really care.

In this day and age to have any Folder content limit is ridicules. Use Mysql if you can't figure it out.
Just write the darn record and store a key for the index. GMAIL and Yahoo and every other provider has virtually unlimited space. The client app. should do the same. These developers are a Joke, if they can't figure out how to eliminate this "in box full" condition.
The fix for this is a Trainee level programing task. (this comment comes from a Software Developer
with over 30 years experience) Just FYI.

(In reply to stevedonato from comment #122)
> In this day and age to have any Folder content limit is ridicules. Use Mysql
> if you can't figure it out.
>
> Just write the darn record and store a key for the index. GMAIL and Yahoo
> and every other provider has virtually unlimited space. The client app.
> should do the same. These developers are a Joke, if they can't figure out
> how to eliminate this "in box full" condition.
>
> The fix for this is a Trainee level programing task. (this comment comes
> from a Software Developer
> with over 30 years experience) Just FYI.
I look forward to seeing your patch for the issue Steve.

Created attachment 8516876[details][diff][review]
make GetSizeOnDisk 64 bit v5
This refreshes the GetSizeOnDisk patch, makes the mFolderSize int64_t per rkent's suggestion (in bug 813459) and makes -1 be the special value meaning size needs to be determined yet (instead of 0 which is a valid folder size). It also updates the 4GB test to see sizeOnDisk can now return >2^32 values up to JS callers.

Comment on attachment 8516876[details][diff][review]
make GetSizeOnDisk 64 bit v5
Review of attachment 8516876[details][diff][review]:
-----------------------------------------------------------------
Overall this is fine, but there are enough little issues that I should see it again. This patch, as you said, was applied after the patch in bug 813459.
Do you understand why the test fails in Windows? I enabled it, bypassed the failing setSparse, but then the test hangs in POP3pump.
::: mailnews/base/util/nsMsgDBFolder.cpp
@@ +136,5 @@
> mInitializedFromCache(false),
> mSemaphoreHolder(nullptr),
> mNumPendingUnreadMessages(0),
> mNumPendingTotalMessages(0),
> + mFolderSize(-1),
Although I agree that this is the correct thing to do, this also needs supporting in the Extra Folder Columns extension. Is there a bug to incorporate that? If so, we should make a note there.
In general, we should avoid magic numbers that are used in multiple places and not clearly defined. So you need a line like:
static const int64_t kSizeUnknown = -1;
or
#define SIZE_UNKNOWN -1
and use those in place of -1.
Because those definitions may span multiple files, they should probably be in a public file like MailNewsTypes.h
@@ +4564,3 @@
> {
> NS_ENSURE_ARG_POINTER(size);
> + // TODO: should this be 0 or -1 (uninitialized)?
This should be -1 (kSizeUnknown)
::: mailnews/base/util/nsMsgUtils.cpp
@@ +475,1 @@
> {
Because you are making size == -1 be a flag for unknown, you need to capture that here so that there is no risk that unknown gets displayed as -1. It is sufficient to simply have:
float unitSize = size < 0 ? 0.0 : size;
::: mailnews/local/src/nsLocalMailFolder.cpp
@@ +202,5 @@
> +
> + // TODO: This should be pushed into RebuildIndex so that the size is
> + // recalculated as part of parsing the folder data (important for maildir),
> + // once GetSizeOnDisk is pushed down to the msgStores (bug 1032360).
> + return RefreshSizeOnDisk();
I don't understand why you want to add this. RebuildIndex is always async in spite of the comment above, so the failure is just a failure to start the async process. Why would you want to refresh the disk size on failure to index?
@@ +1108,5 @@
>
> NS_IMETHODIMP nsMsgLocalMailFolder::RefreshSizeOnDisk()
> {
> + int64_t oldFolderSize = mFolderSize;
> + mFolderSize = -1; // we set this to -1 to force it to get recalculated from disk
This should be = kSizeUnknown
@@ +1119,4 @@
> {
> NS_ENSURE_ARG_POINTER(aSize);
> nsresult rv = NS_OK;
> + if (mFolderSize == -1)
again kSizeUnknown
::: mailnews/news/src/nsNewsFolder.cpp
@@ +72,5 @@
> #define NEWSRC_FILE_BUFFER_SIZE 1024
>
> #define kNewsSortOffset 9000
>
> +#define kSizeUnknown -1
You'll want to use whatever you use in the common file. This definition is fine, you could use it it all of the files if you like.

(In reply to Kent James (:rkent) from comment #125)
> Comment on attachment 8516876[details][diff][review]
> make GetSizeOnDisk 64 bit v5
>
> Review of attachment 8516876[details][diff][review]:
> -----------------------------------------------------------------
>
> Overall this is fine, but there are enough little issues that I should see
> it again. This patch, as you said, was applied after the patch in bug
> 813459.
>
> Do you understand why the test fails in Windows? I enabled it, bypassed the
> failing setSparse, but then the test hangs in POP3pump.
No I do not. If you just disable the sparsing of the file and POP3Pump still hangs, then I don't know. I suspect there is some lock held (unclosed file or so) so POP3pump can't proceed. Windows FS is stricter in this regard (like you can't delete an open file). We track this in bug 872357.
>
> ::: mailnews/base/util/nsMsgDBFolder.cpp
> @@ +136,5 @@
> > mInitializedFromCache(false),
> > mSemaphoreHolder(nullptr),
> > mNumPendingUnreadMessages(0),
> > mNumPendingTotalMessages(0),
> > + mFolderSize(-1),
>
> Although I agree that this is the correct thing to do, this also needs
> supporting in the Extra Folder Columns extension. Is there a bug to
> incorporate that? If so, we should make a note there.
I am not sure an external caller can ever see the value of -1 (but maybe on an account type that does not override GetSizeOnDisk to return a proper value). The extension knows that it can get a value of -1 for number of messages and is prepared for that. I can check folder size too and add the fix to my already improved local copy of the extension that I plan to upload for bug 464973.
> In general, we should avoid magic numbers that are used in multiple places
> and not clearly defined. So you need a line like:
>
> static const int64_t kSizeUnknown = -1;
> or
> #define SIZE_UNKNOWN -1
>
> and use those in place of -1.
>
> Because those definitions may span multiple files, they should probably be
> in a public file like MailNewsTypes.h
Of course I'd love to do a named constant, I just didn't know where I would be allowed to put it :)
Thanks for the hint, I'm on it.
@@ +4564,3 @@
> > {
> > NS_ENSURE_ARG_POINTER(size);
> > + // TODO: should this be 0 or -1 (uninitialized)?
>
> This should be -1 (kSizeUnknown)
>
> ::: mailnews/base/util/nsMsgUtils.cpp
> @@ +475,1 @@
> > {
>
> Because you are making size == -1 be a flag for unknown, you need to capture
> that here so that there is no risk that unknown gets displayed as -1. It is
> sufficient to simply have:
>
> float unitSize = size < 0 ? 0.0 : size;
>
> ::: mailnews/local/src/nsLocalMailFolder.cpp
> @@ +202,5 @@
> > +
> > + // TODO: This should be pushed into RebuildIndex so that the size is
> > + // recalculated as part of parsing the folder data (important for maildir),
> > + // once GetSizeOnDisk is pushed down to the msgStores (bug 1032360).
> > + return RefreshSizeOnDisk();
>
> I don't understand why you want to add this. RebuildIndex is always async in
> spite of the comment above, so the failure is just a failure to start the
> async process. Why would you want to refresh the disk size on failure to
> index?
I only catched this because the test didn't work. When execution got to ParseListener_growOver4GiB() (which I hoped would be after the parsing is done), the .sizeOnDisk still did not contain the new (above 4GB) value. So I had to add this refresh here. Maybe there is a better place for it? Or there is a bug that the size is not properly updated insize RebuildIndex?
Or is OnStopRunningUrl in ParseListener_growOver4GiB not really run after everything is done?

Created attachment 8524993[details][diff][review]
make GetSizeOnDisk 64 bit v6
This just adds tests whether the updated OnItemIntPropertyChanged from bug 813459 can return 64bit values now. I could not test it in that bug before there actually was a first 64bit property.
The only change in C++ code is the addition of "UpdateSummaryTotals(true);" in nsMsgLocalMailFolder::OnStopRunningUrl to the RefreshSizeOnDisk I already had to add before. Otherwise number of messages were not updated immediately when folder re-parsing finished.
I see the number of do_check_eq() calls gets quite big, but I want to be sure we do not miss anything in the 32/64bit internal conversions.

Comment on attachment 8524993[details][diff][review]
make GetSizeOnDisk 64 bit v6
Review of attachment 8524993[details][diff][review]:
-----------------------------------------------------------------
As a general comment, it is time to quit tweaking this patch and just get it landed. I have already spent a lot of time looking at this patch and followup bugs. Some of the issues that you are tweaking will end up getting changed significantly when we fix some of this for maildir, so this is not time well spent by you or me. I was satisfied with the previous version, the only required change was moving some changes here from bug 813459 (which was also unnecessary make-work IMHO), and these delays are slowing down the maildir work.

Yes, that status is correct. The "feature" is not done yet until all the blocking bugs are fixed. The last patch to attach here is a simple one to remove the 4GB limit (like https://bug793865.bugzilla.mozilla.org/attachment.cgi?id=751871).
Unless we decide to close this with the existing check-ins and many comments and create a new meta bug.

(In reply to Kent James (:rkent) from comment #140)
> It's a pity that we never finished the few things remaining to do this.
Actually I do not remember anything that would be remaining here (the last was the incremental generation of keys).
I will now create some tests.
As I still think there may be unexpected surprises in the code yet, would you support the plan that in the first stage we make a pref (off by default) that would turn on the support of 4GB+ folders? I think the pref only needs to be checked in nsMsg*Store::HasSpaceAvailable() which decides if new msgs can be accepted into the message store. In that way adventurous nightly users could turn it on.

I think that the correct path is to add the pref, but default it to enable support for >4GB in nightlies and betas. We need testing, and a default off pref is not going to help.
Yes this is critical code, but reaching the 4GB limit is also a critical issue that needs fixing. I would love to see us leave support enabled for TB 52.

Created attachment 8774067[details][diff][review]
allow crossing 4GB limit by default
Maildir already doesn't have the 4GB limit (I think it was removed due to spurious errors). So this removes it for mbox, based on a pref. I read the pref once per folder into a static variable. That is similar to e.g. gTimeStampLeeway but I didn't want to do the gGotGlobalPrefs dance (not sure why we can't read the leeway pref too in the constructor).

Pinning the value of the pref is not that great. I'd like to toggle it on the fly in tests. That would allow to just add a few tests into test_over4GBMailboxes.js. I'm not sure we want to duplicate that file completely (it takes its time to run).
Would it be to expensive to check the pref at each adding of a new msg?

Comment on attachment 8774067[details][diff][review]
allow crossing 4GB limit by default
Review of attachment 8774067[details][diff][review]:
-----------------------------------------------------------------
It looks to me like the reading of a pref is done using a C++ hashtable, and therefore should not be considered expensive to do for each message.
The other change I know I would like with a brief look at this is the naming of mailnews.folders.4GBlimit First, there is nothing else in the branch mailnews.folders and no plans to add anything, so I don't see why we need that extra level. Second, I cannot tell the polarity of "4GBlimit" without searching the code.
So I suggest instead mailnews.limitMboxTo4GB for the current definition. But that is a negative definition, which makes one think hard when it is false. So I really prefer a positive definition like mailnews.allowMboxOver4GB
r- only because this is going to change, but overall I like what I see!

Created attachment 8775726[details][diff][review]
allow crossing 4GB limit by default v2
I think creating the pref service is more expensive. I'm not sure why there is no mozilla::services::GetPrefService() shortcut for it yet.