Compacting folders when getting lock failed

VERIFIED FIXED

Status

MailNews Core
Backend
VERIFIED FIXED
16 years ago
10 years ago

People

(Reporter: Navin Gupta, Assigned: Navin Gupta)

Tracking

({dataloss})

Trunk
x86
Windows 2000
dataloss

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ish1+][fixedish1])

Attachments

(1 attachment)

639 bytes, patch
Cavin Song
: review+
Bienvenu
: superreview+
chris hofmann
: approval+
Details | Diff | Splinter Review
(Assignee)

Description

16 years ago
I found a major hole in compacting folders. When we don't get the lock we still
continue to compact, issuing more than one compact etc. bad bug!!
(Assignee)

Comment 1

16 years ago
Created attachment 102245 [details] [diff] [review]
proposed fix

We should return not fall through after calling compactNextFolder. major
hole...
causing real bad bugs.
(Assignee)

Comment 2

16 years ago
cc cavin & bienvenu for reviews ? thx.
Status: NEW → ASSIGNED

Updated

16 years ago
QA Contact: gayatri → esther

Comment 3

16 years ago
do we try to compact the same folder again, or the next folder in the list? Why
does failing to get the lock on one folder affect the compaction of the next
folder? Or is there some other problem? Your change seems OK, but I'm curious
about the details.
(Assignee)

Comment 4

16 years ago
The details are self explanatory. We issue compact for next folder and then
comeback and compact the folder for which lock failed. 
(Assignee)

Comment 5

16 years ago
cc putterman. Scott, we need this for blackbird. 

Comment 6

16 years ago
well, it may be self-explanatory to you, but the way I imagine compact all
working is one folder after the other. So, I do folder A, then I do folder B,
then folder C. If folder A fails, then I should move on to folder B.

Comment 7

16 years ago
ah, so what's the affect of your change? Do we abort all the compacts, or just
the current compact, and go on to the next folder?
(Assignee)

Comment 8

16 years ago
So if folder A fails we move on to folder B but because we don't return after
calling CompactNextFolder() we were compacting A. So 2 compacts for A & B issued
at the same time and compact for A doesn't even have the lock. 
(Assignee)

Comment 9

16 years ago
With my current we just abort current compact. 
(Assignee)

Comment 10

16 years ago
s/current/change

Comment 11

16 years ago
Comment on attachment 102245 [details] [diff] [review]
proposed fix

well, I still can't tell for sure if we go on to compact the next folder or
not, but at this point, anything is better than what must be happening without
this patch. I think it would probably be better to continue on to the next
folders, and I'm guessing that's what happens.
Attachment #102245 - Flags: superreview+

Comment 12

16 years ago
Comment on attachment 102245 [details] [diff] [review]
proposed fix

r=cavin.
Attachment #102245 - Flags: review+
(Assignee)

Comment 13

16 years ago
I meant we skip the current folder and compact the next folder. 
(Assignee)

Comment 14

16 years ago
fix checked in. 
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
(Assignee)

Comment 15

16 years ago
*** Bug 170600 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 16

16 years ago
*** Bug 164624 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 17

16 years ago
*** Bug 144561 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 18

16 years ago
*** Bug 137139 has been marked as a duplicate of this bug. ***

Updated

16 years ago
Keywords: adt1.0.2

Comment 19

16 years ago
*** Bug 152101 has been marked as a duplicate of this bug. ***

Comment 20

16 years ago
Navin, do you have a reproducible test case for this?  I read through all the
dup and have tried to find a reproducible case myself without any luck.  I see a
currently reported reproducible case in bug 137139 for showing Inbox doubling of
messages during compacting, which I haven't seen yet.

Comment 21

16 years ago
It looks like the test case you put in 173399 is one I can use for this bug. 
Using that test case I lost all my msgs in my inbox.  Seems like that test case
could be the one for all the dups too reporting lost msgs during compacting. 
(Assignee)

Comment 22

16 years ago
yeah, that one is for sure.

Comment 23

16 years ago
The symptom of doubling the Inbox display, reported in Bug#137139, Comment#6,
still happened tonight on 2002-10-09 build.  Either clear the dupe link there or
re-open this one.

Comment 24

16 years ago
Bug 137139 has been reopened, this bug remains as resolved fixed.  I've tested
this bug in today's trunk 20021011 on winxp and can't reproduce the problem.  
Keywords: dataloss
(Assignee)

Comment 25

16 years ago
*** Bug 157018 has been marked as a duplicate of this bug. ***

Comment 26

16 years ago
Plussing for adt.  Need checkin by cob 10/16. Please make sure to get all
required  approvals (drivers) before checkin if this is not already done.
Keywords: adt1.0.2 → adt1.0.2+

Updated

16 years ago
Blocks: 168047

Comment 27

16 years ago
Comment on attachment 102245 [details] [diff] [review]
proposed fix

a=chofmann for 1.0.2
Attachment #102245 - Flags: approval+
(Assignee)

Comment 28

16 years ago
fixed on branch
Keywords: fixed1.0.2
(Assignee)

Comment 29

16 years ago
*** Bug 174787 has been marked as a duplicate of this bug. ***

Updated

16 years ago
Whiteboard: [ish1+]

Comment 30

16 years ago
Using branch build 20021016 on winxp I tested the scenario in 173399 per
comments 21 & 22.  I also tested various scenarions the include:  
1.) Clicking Compact Folders multiple times while compacting is in progress for
the Inbox = OK, I received the expected message: "The folder "xyz" cannot be
compacted because another operationis in progress.  Please try again later."  I
OK the error and compacting continues without losing msgs or corrupting the msf
file.  
2.) Clicking on Compact Folder multiple times while compacting of Inbox and 2
other  folders is in progress = OK, I received the expected message: "The folder
"xyz" cannot be compacted because another operationis in progress.  Please try
again later."  I OK the error and compaction continues.  I click Compact Folders
again while the compacting is still going on this time in another folder.  I get
the appropriate error msg.  I continue this until I don't get the error msg
anymore and then I check the folders that were compacted.  They are all OK.
3.) Clicking on OK for the automatic compact notice for POP account while POP
Inbox is getting messages.  I get the error notice, OK'ing it.  Result all
folders OK.
4.) Clicking on OK for the automatic compact notice for POP account while IMAP
Inbox is getting messages.  =  OK.

Still need to test on linux and mac before verifying.

Comment 31

16 years ago
Verified on Mac 9.2 and linux using branch build 20021017, changing keyword to
verified1.0.2
Keywords: fixed1.0.2 → verified1.0.2

Updated

16 years ago
Status: RESOLVED → VERIFIED

Comment 32

16 years ago
verifying as fixed since this was tested in the trunk and branch. Note: tested
on trunk build on 10-11 when testing bug 173399.

Comment 33

16 years ago
I want insist on a specific case of this bug, which I reported in a dup of this
bug (bug # 174787). I don't know if this circunstance makes the difference, but
I suffer the folder corruption at same time Mozilla crashed.
I had had some crashes while Mozilla compacts folders, but it never corrupted my
messages (only remaining temporal folders after restart Mozilla, easily deleted).
(Assignee)

Comment 34

16 years ago
*** Bug 175980 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

16 years ago
Whiteboard: [ish1+] → [ish1+][fixedish1]
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.