Build ID: Mozilla Thunderbird 0.5 (20040208) This was encountered in Thunderbird 0.5, but since there doesn't seem to be an appropriate component for it there, I'm assuming this is code that's common to both Thunderbird and Mozilla MailNews. If I'm wrong, feel free to move it back. I ran out of disk space on my iBook while Thunderbird was running. Thunderbird politely stopped checking mail since it didn't have anywhere to put the mail when it downloaded it (or so it appeared anyway). However, when I cleared up the disk space issue, Thunderbird started downloading mail again. The only problem is it apparently tossed the UIDL information about the POP3 mailbox (I have it set to leave mail on the server for 7 days), and it proceeded to redownload the last 7 days worth of mail (3000 or so pieces), giving me duplicates of a lot of things and resurrecting several dozen messages I had locally deleted. I've actually had this happen to me twice today (I've been fighting with the disk space until I get a chance to move some things to another computer), so it's reproducible. (I have over 200 MB free as I type this, but OS X likes to allocate huge chunks at once for swap, and that's what's killing me).

The Bayes filter training may or may not have been lost at the same time, fwiw... Almost all of the duplicated spam that got downloaded during the above wound up in my inbox again instead of getting flagged as junk, even though they were all pieces that were specifically used for training previously.

Yes, the file in question is popstate.dat in the server folder of your mail directory. net_pop3_write_state() creates the whole file from scratch after we're done (or interrupted). David, write to a temp and then only delete the old and rename the new one if we're succeeded writing? Would be save for the old UIDL, but UIDL of in this session already downloaded messages would be lost too. But I guess with this file format we can't to more intelligent writing. BTW, what happens if a statement like outFileStream << host->user; runs into a full disk?

as far as downloading messages goes, since the pop3 server does tell you how big the messages are first before you start downloading, it would make sense to make sure there's that much disk space free before beginning to download them. Perhaps making sure there is ((room for new messages about to be downloaded) + (2 * sizeof(popstate.dat))) before begining a download would prevent this in all but "something else is rapidly filling my drive" situations. And in those situations, ensuring that we don't overwrite the existing popstate.dat if there's no space to do it would be better than nothing. If we wind up downloading the messages just from the last download session again, that's way better than re-downloading everything on the server for someone who has "leave on server" set to any number of days.

we do preflight the pop3 download by the total of the message size and abort if there's not enough disk space available. But, we don't do it for the uidl, as Christian points out. Yeah, we could write to a temp file first and then copy it over the original if successful. Usually, streaming calls won't fail because everything is buffered - often, it's only the final flush/close that fails because we're out of disk space. We could check the disk space available before writing the uidl file...

> we do preflight the pop3 download by the total of the message size and abort > if there's not enough disk space available. Oh, indeed. I didn't bargain for this. But why did Dave lose his UIDL's? If the check shows there isn't enough space left, we leave GetMsg(), go to POP3_ERROR_DONE and then write out the UIDL list. Because the list hasn't changed in this case, the "new" list should fit in the available space. The only explanation would the "something else is rapidly filling the drive" situation while we wrote out the list. I still vote for some check or the copy-solution, but as far as I can see, this bug shouln't have happened with the current code either.

I don't think we account for the size of the uidl file in the calculation, and there is a point at which we might have two copies of it on disk (while we're rewriting the file ? ) - and as you say, if the disk is rapidly filling up, we'll have problems. We don't reserve the space on disk for the pop3 download, iirc - we just check before we start writing.

Yes, the space isn't reserverd. But I don't think we've two copies on disk while rewrite. We open popstate.dat with PR_TRUNCATE and write it from scratch. Overwriting a 1.5 kB popstate.dat with a 1.5 kB popstate.dat should succeed in any case (except the very rare and unlikely rapidly filling). Dave, how was the situation you had? Wasn't there any byte free after Mozilla gave up? And did Mozilla even start downloading and then stopped after a few mails? Or did it recognzie there isn't enough space before downloading the first one?

(In reply to comment #7) > Dave, how was the situation you had? Wasn't there any byte free after Mozilla > gave up? > And did Mozilla even start downloading and then stopped after a few mails? Or > did it recognzie there isn't enough space before downloading the first one? I honestly don't know. I had walked away from the computer at the point when it was out of space, and discovered everything stopped up when I got back. At the point when I got there there were 0 bytes free on the drive according to 'df', although I only had the "your startup disk is almost full" dialog in the Finder and not the "your startup disk is full, please choose an application to force-quit" dialog.

*** Bug 281671 has been marked as a duplicate of this bug. ***

This problem is also true for NNTP. I subscribed to several hierarchies on 2 newsgroups servers in my thunderbird and at some point, I closed thunderbird while the disk was out of space. Then, when I started it again, on one of my 2 NNTP accounts, all the groups I had subscribed had disappeared. The files corresponding to these groups are still there in my profile account but it seems that thunderbird has no idea of them anymore. If I try to subscribe again to these groups, It seems that thunderbird don't retreive the old informations it had on them, like read articles.

Nominating this for tb3, possibly not a huge amount of work.

A fine idea, but not a blocker.

Has this been fixed recently? I've been trying to reproduce with a recent build, but I haven't been able to. As expected, Shredder informs me that the disk is out of space and refuses to download anything until I free up some space. When I do, only new messages are downloaded. Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1b4pre) Gecko/20090412 Shredder/3.0b3pre

(In reply to comment #19) > Has this been fixed recently? I've been trying to reproduce with a recent > build, but I haven't been able to. As expected, Shredder informs me that the > disk is out of space and refuses to download anything until I free up some > space. When I do, only new messages are downloaded. > > Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1b4pre) Gecko/20090412 > Shredder/3.0b3pre Thanks for testing.

This happens to me with thunderbird 2.0.0.22. I have not tried with Shredder yet. Would be nice if it was really fixed.

I've experienced this bug three times in the last few weeks, as my laptop is constantly running low on HD space. This bug has been with me as long as I can remember using Thunderbird. It'd be great if it could get fixed. How can I assist? I'm using version 2.0.0.23 (20090812) for Linux. P.S.: The Remove Duplicate Messages add-on makes it easier to clean up, but it's still very annoying, especially with messages that I manually sorted into folders.

Just confirmed this bug on Win7 Home Premium x64 SP1. See attached screenshot where Thunderbird is downloading visibly duplicate messages after I resolved a full disk error. This has happened twice and is easily reproducible and really annoying to fix.

Hi guy, I am facing the same problem running Thunderbird 17.0.2 in Ubuntu Linux 12.10, 64Bit. When the system runs out of hard drive memory Thunderbird starts to download all mails again. I am using a POP server.

reopening. It's not clear (to me) whether comment 19 was a sufficient test.

I can confirm this bug on Linux running Thunderbird version 17.0.2 (64bit). This only happens with POP-Accounts.

I think this is not fixed. The checks for free disk space before pop3 download are not sufficient. We may help this in bug 640371 (do not even start pop3 download if disk space is not enough for all the messages to be downloaded) and bug 789679, where we actually check free disk space on some more operations in TB. In all the checks, we only allow the operation if there still remains at least 4MB free disk space after the operation. So 4MB should be enough to rewrite the popstate.dat file. However we still should check for free space before popstate.dat is rewritten.

Adding some searchwords for better retrievability.

(In reply to :aceman from comment #28) > However we still should check for free space before popstate.dat is > rewritten. Any proposals on what to check? Current size of popstate.dat*2 + 4MB of spare? Anything more clever? What sizes of popstate.dat have you guys seen in the wild?

The largest I find recorded in my emails and bug reports is 1.8MB. But bienvenu would have great historical knowledge here. how about popstate.dat*3 + X MB, where X is what we are using elsewhere as "buffer against disaster size"?

17.0.5 (64bit), ~50.000 POP emails, this bug is annoying. I use this addon to get rid of the havoc: https://addons.mozilla.org/en-us/thunderbird/addon/remove-duplicate-messages-alte/ But junk mail, manually removed before, can remain. With the DiskSpaceAvailableInStore(file, spaceRequested) function proposed in https://bugzilla.mozilla.org/attachment.cgi?id=735319&action=diff the proposed criterion could be implemented. I think writing the new content of popstate.dat to a temporary file and then moving it over would be saner, because then you can never enter a corrupt state. This bug may be more closely related to Bug 263142 than Bug 789679 (new dep?), and pehaps can be resolved together.

This patch should prevent the bug, because move does not fail when the disk is full.

Comment on attachment 756011 [details] [diff] [review] Write updated popstate in tempfile first, then copy over You need to request reviews for patches. Setting it for you.

Cool, thanks for the patch. The patch you referenced (https://bugzilla.mozilla.org/attachment.cgi?id=735319&action=diff) was already checked in. Also bug 640371 should help this case as it refuses to download any new messages if there is less than several MB of free disk space. Even if we rewrote popstate.dat in that case (but unchanged) there should be enough space to do it. But your patch may cover some more cases (e.g. if the disk is filled by other program and some messages are only marked for deletion on server). I just wonder why net_pop3_write_state can't report any failure to higher levels.

Can't reproduce disk full bug using this patch. Can reproduce disk full bug without this patch. Move does not fail when the disk is full. For testing, the disk must be filled up between connecting to the server and closing the connection, because Thunderbird does not try to fetch new messages anymore if the disk is full. I just now saw Bug 246675 is the right approach. Will make an update

So when you reference bug 246675, why do you not use the nsISafeFileOutputStream ? Is it not applicable to the file here?

As in Bug 246675, using NS_NewSafeLocalFileOutputStream now, which is an existing implementation that writes to a temp file first. I verified that this patch works using strace and filling the disk. Writing to the temp file fails, and rename is not done, preserving the original file. I think MsgNewSafeBufferedFileOutputStream should now be used in a number of other places. Please review.

Comment on attachment 756486 [details] [diff] [review] Write updated popstate in tempfile first, then move over Review of attachment 756486 [details] [diff] [review]: ----------------------------------------------------------------- Thanks so much for the patch; sorry it took so long for me to review it. I'm OK with either landing this patch as is, or expanding it to refactor out the other places in mailnews where we create safe, buffered output streams. ::: mailnews/base/util/nsMsgUtils.cpp @@ +1438,4 @@ > return rv; > } > > +nsresult MsgNewSafeBufferedFileOutputStream(nsIOutputStream **aResult, There are a couple of other places in mailnews where we're creating safe output streams; it would be great to update those to use this helper function instead of repeating the code, as part of this patch.

Johannes do you want to do what Irving is proposing ?

Hi Irving, thanks for the review. For some reason I did not get/read the notification email from Mozilla. I, too, noticed that there are other places which would deserve attention and use of a similar function. However, I would prefer this patch to go through first, and that we make another bug to (1) identify those code segments/files, (2) discuss the appropriate solution for each and, then, (3) write a patch, which I can help with. So I would ask for this patch to go through as is, since you indicated it is ok.

I have marked the bug so that the patch will be checked in. Johannes, please file the follow-up bug for the remaining work on the other similar places. Please CC me there. I hope you can work on that bug too.

The follow-up bug is at Bug 912465 (feel free to change the component, etc). Lets continue there. Thank you Irving, Aceman, and Ryan.

This bug did not fix at all. nsISafeOutputStream.finish has to be invoked to overwrite popstate.dat.

Thanks Hiro. I overlooked this. Patch looks good to me. We don't need to check for NS_FAILED(rv) again, because either way void is returned.

(In reply to Hiroyuki Ikezoe (:hiro) from comment #47) > Created attachment 800048 [details] [diff] [review] > followup_fix_for_bug239455.patch > > This bug did not fix at all. > > nsISafeOutputStream.finish has to be invoked to overwrite popstate.dat. see bug 913502 That seems to occur only if you set the pref leave messages on the server.

I can't speak to the original intent of this bug. But, with a local build including attachment 800048 [details] [diff] [review] the symptoms of bug 913502 are gone.

This might be worth slipping in to the closed tree, to make sure it shows up in the next successful nightly.

(In reply to :Irving Reid from comment #51) > This might be worth slipping in to the closed tree, to make sure it shows up > in the next successful nightly. I advocate for this. I believe "leave messages on the server" is a commonly used setting.

Now the tree is at least building partially, I've landed this: https://hg.mozilla.org/comm-central/rev/a5e88cd9f942