102751 - Compaction with outdated .msf => mail loss

Status: VERIFIED FIXED

(MailNews Core :: Backend, defect)

Description

[Aleksey Nogin]

If I compact a folder that was modified outside of Mozilla, Mozilla will create
a new folder with size 0, losing all the mail

Reproducible: not sure. I lost 3 folders this way, but haven't tried actively
reproducing it.
Steps to reproduce:
1) Delete some mail from a folder (to make sure there is something to compact).
2) Quit Mozilla.
3) Add more messages to the folder using an external program (e.g. pine).
4) Start Mozilla.
5) "Compact all folders" (*without* selecting the folder in question).

Expected behaviour: Mozilla should notice that .msf is outdated and either
rebuild it before the compaction or refuse to perform the compaction.
Actual: After compaction, the folder file has zero size and all the mail is lost.

P.S. I had this happen to me while using Mozilla 0.9.2.1 on RedHat Linux 7.1

Comment 1

[David :Bienvenu]

Compact -> Navin. But reporter, you should really download a more recent mozilla
build since several bugs in this area have been fixed.

Comment 2

[Navin Gupta]

Attachment: proposed fix

Comment 3

[Navin Gupta]

This change may affect lots of things but this is the right way to catch if 
db is not intialized i.e outdated so do not start compacting. 

Comment 4

[David :Bienvenu]

You'd have to test all the places that cal this routine to make sure no
regressions are introduced.

The way this should work is that we should run a url to reparse the folder and
create the database, and if that url succeeds, compact the folder.

Comment 5

[Navin Gupta]

The problem is we get expunged bytes from pancea.dat when we have not selected
the folder and for the example that aleksey has given the db would list all the 
old msgkeys only, so we will have to make sure that db is properly opened
before we can start compacting. I agree i will have to test this for possible
regression. 

Comment 6

[David :Bienvenu]

I don't think panacea.dat has anything directly to do with the problem. I agree
that we have to make sure we've opened the db successfully before compaction. 
What I'm saying is if that fails because the summary is out of date, or
missing(which is an explicit error code), we should reparse the folder and then
compact it.

Comment 7

[Navin Gupta]

I wouldn't worry about compacting if db fails to open correctly, it can get 
compacted next time around. 

Comment 8

[David :Bienvenu]

No, it will never be compacted unless the user opens it and lets it reparse.

Comment 9

[David :Bienvenu]

Navin is right, the db open will fail but it will kick off a reparse of the
folder, so the next compact all will work.

Comment 10

[Navin Gupta]

found other problems while working on this one. compact crashing randomly, have
a fix for that. now trying to find how are we passing null mWindow from 
nsMsgFolderDataSource, i mean it was working 10 days back, what could have 
caused that problem.

Comment 11

[David :Bienvenu]

is your tree up-to-date? Seth backed himself out, for a problem that could cause
exactly that.

Comment 12

[Navin Gupta]

Attachment: patch

Comment 13

[Navin Gupta]

ok, I have accounted for all the places GetMsgDatabase() is used and restored 
their behavior so that they are unaffected by this change. Unfortunately if 
you return failure code to js, it throws exception and msgDatabase is null so
sorting view may not get transferred from oldDB to newDB. everything else 
should work as before, other than compact.

Comment 14

[David :Bienvenu]

I really don't like dropping all those return codes in all those places. Can't
we just change one place and leave the other code the way it is?

Comment 15

[Navin Gupta]

That is not possible because in GetMsgDatabase() instead of returning NS_OK, we 
are now returning rv, which can be anything, because it gets that from 
GetDatabase(). 

Comment 16

[David :Bienvenu]

GetDatabase is not a random number error code generator. I don't really
understand why it's not possible so I'm going to look at this for a while and
see if I can come up with a fix where we pay attention to error codes.

Comment 17

[Navin Gupta]

Attachment: proposed fix, v3

Comment 18

[Navin Gupta]

The fix is to parse the folder and then start compact if parsing was successful.
removed redeclaration of rv in GetDatabaseWOReparse(), was never allowing rv
to be propagated, bad! 

david, need review. 

Comment 19

[David :Bienvenu]

Comment on attachment 52999 [details] [diff] [review]
proposed fix, v3

looks good, r=bienvenu

Comment 20

[(not reading, please use [e-mail] instead)]

Comment on attachment 52999 [details] [diff] [review]
proposed fix, v3

sr=sspitzer

Comment 21

[Navin Gupta]

fix checked in. 

Comment 22

[Sheela Ravindran]

Verified on win98 and linux( 2001-12-07-06 build). I followed the steps given by 
the reporter. I deleted messages from one of the local folder. Then restarted 
the application.  Copied few more messages from the menu item to that folder 
and did not select that folder. Then I compact folders from the menu item. Later 
checked to make sure the messages that were copied and the remaining were not 
lost in the folder.