151699 - moving folders containing more mail than free disk space DELETES that mail

Status: VERIFIED FIXED

(MailNews Core :: Backend, defect)

Description

[Ralf Muhlberger]

This applies to mail stored locally such as when using POP.

To replicate: Fill your hard disk up and then drag a folder with mail to a
different location. If that folder contains more mail than you have available
hard disk space, data will be lost. This is irrecoverable as far as I can tell
(and I did a LOT of looking around to recover my mail..) 

I've tried this only on my Win2K machine, and have reproduced it both under
version 1.0 and 0.9. This may be the cause for the following bug report that
hasn't been replicated:
  http://bugzilla.mozilla.org/show_bug.cgi?id=138104
I initially thought that it had to do with filters, but I suspect that it's just
plain folder moving.

Friendly regards,

Ralf

PS I'm not sure if this is a 'Mail Database' component, but that seemed the most
likely choice.

Comment 1

[David :Bienvenu]

the mail database is for the header summary info, not the local mailbox folders.
I would think that we should just be using the file system rename function to
move the folder(s) and allow the OS to handle this - if we're not doing that,
then we have to worry about failure cases in the folder moving code.

-> Cavin

Comment 2

[Navin Gupta]

I have worked on drag and drop of folders. taking. 

Comment 3

[Navin Gupta]

Attachment: proposed fix

The fix is to return if the copying the folder or its summary file fails.
Cavin, David, Can I get reviews ? thx.

Comment 4

[Navin Gupta]

Attachment: proposed fix (right patch)

The fix is to return if the copying the folder or its summary file fails.
Cavin, David, Can I get reviews ? thx.

Comment 5

[David :Bienvenu]

Comment on attachment 88999 [details] [diff] [review]
proposed fix (right patch)

sr=bienvenu - did you try it to see what happens?

Comment 6

[Navin Gupta]

yes, I simulated in the debugger and we get out of copying/moving folder if the
code fails there, as expected. 

Comment 7

[Cavin Song]

Comment on attachment 88999 [details] [diff] [review]
proposed fix (right patch)

r=cavin. Good one.

Comment 8

[Navin Gupta]

fixed on trunk. We can take this for branch because it is a dataloss bug and the
fix is fairly low risk.

Comment 9

[Ralf Muhlberger]

From what I observed, the folder structure gets constructed first and
the messages are subsequently moved across. When you propose:

"The fix is to return if the copying the folder or its summary file fails."

does that mean that you do an initial test but not subsequently for
the later messages, i.e. if there is not enough space for the
folders it will stop, but not testing if there is not enough space 
for the messages? Or does that comment refer to the folder, summary
file _and content of folder_, in which case it returns rather than
'cleaning up' the older data. Is there an indicator of why it would
have failed, and would it clean up the folders that have been moved
so far already? In other words, is the folder move operation atomic?

Ralf

Comment 10

[Navin Gupta]

We don't copy msg by msg we just copy the folder and it's db(summary file).
No we don't perform any clean-up. you can file a separate bug for that.

Comment 11

[scottputterman]

adding adt1.0.1+.  Please get drivers approval before checking in.

Comment 12

[Judson Valeski]

please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.

Comment 13

[Navin Gupta]

fixed on branch

Comment 14

[Sheela Ravindran]

Tested this bug on WinNT machine in one of the labs. Marking this verified since 
I verified this bug on trunk. Also changing the keyword from fixed1.0.1.
I also saw that we don't clean up the messages when we know we have run out of 
disk space. But we do not loose the folder and messages from the source where we 
were trying to move it from.