Compaction with outdated .msf => mail loss

VERIFIED FIXED

Status

MailNews Core
Backend
VERIFIED FIXED
17 years ago
10 years ago

People

(Reporter: Aleksey Nogin, Assigned: Navin Gupta)

Tracking

({dataloss})

Trunk
x86
Linux
dataloss

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

1.28 KB, patch
Details | Diff | Splinter Review
14.35 KB, patch
Details | Diff | Splinter Review
5.65 KB, patch
Bienvenu
: review+
(not reading, please use seth@sspitzer.org instead)
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

17 years ago
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

17 years ago
Compact -> Navin. But reporter, you should really download a more recent mozilla
build since several bugs in this area have been fixed.
Assignee: bienvenu → naving
Component: Mail Database → Mail Back End
(Reporter)

Updated

17 years ago
Keywords: dataloss
(Assignee)

Comment 2

17 years ago
Created attachment 52615 [details] [diff] [review]
proposed fix
(Assignee)

Comment 3

17 years ago
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. 

Status: NEW → ASSIGNED

Comment 4

17 years ago
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.
(Assignee)

Comment 5

17 years ago
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

17 years ago
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.
(Assignee)

Comment 7

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

Comment 8

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

Comment 9

17 years ago
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.
(Assignee)

Comment 10

17 years ago
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

17 years ago
is your tree up-to-date? Seth backed himself out, for a problem that could cause
exactly that.
(Assignee)

Comment 12

17 years ago
Created attachment 52766 [details] [diff] [review]
patch
(Assignee)

Comment 13

17 years ago
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

17 years ago
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?
(Assignee)

Comment 15

17 years ago
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

17 years ago
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.
(Assignee)

Comment 17

17 years ago
Created attachment 52999 [details] [diff] [review]
proposed fix, v3
(Assignee)

Comment 18

17 years ago
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

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

looks good, r=bienvenu
Attachment #52999 - Flags: review+
Comment on attachment 52999 [details] [diff] [review]
proposed fix, v3

sr=sspitzer
Attachment #52999 - Flags: superreview+
(Assignee)

Comment 21

17 years ago
fix checked in. 
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Updated

17 years ago
QA Contact: esther → sheelar

Comment 22

17 years ago
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. 
Status: RESOLVED → VERIFIED
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.