155281 - Moving msgs to local folders when disk space is full results in loss of msgs

Status: VERIFIED FIXED

(MailNews Core :: Backend, defect)

Description

[Sheela Ravindran]

commercial branch and trunk build: 2002-07-01 on winNT

Found this bug while verifying for another disk space bug(151699).
We don't check for the disk space when we are moving messages and we loose that 
message from the source folder. If you check the destination folder you would 
see the message in the thread pane and the contents of the message is lost 
permanently

Steps to repro:
Move message from pop inbox to a local folder without any disk space left.

Actual: You loose the message from source. The message is seen on the thread 
pane in the destination folder but the contents are lost.

Expected: Should detect the disk space and then move the message if not abort 
the move.

Comment 1

[Sheela Ravindran]

Sorry correcting the summary.
Nominating this for RTM since it is a data loss.

Comment 2

[Navin Gupta]

working on a fix. 

Comment 3

[Navin Gupta]

Attachment: proposed fix

The fix is to check how many bytes have actually been written. If it doesn't
match
the number of bytes we wanted to write throw write error alert and abort the
operation. The error needs to be propogated so that the file transport request
can be cancelled. I had to add a boolean m_writeFailed so that we know the
write has
failed in EndCopy and EndMove code.

Comment 4

[Navin Gupta]

David, Cavin, Can I get reviews? thx.

I have tested this by shrinking my partition and this fix works. 

Comment 5

[David :Bienvenu]

did you try this on Linux and mac?

Comment 6

[Navin Gupta]

How will it make any difference. We have other code that checks for write
failure in the same way, like when we are downloading pop3 msgs. 

Comment 7

[David :Bienvenu]

I just want to make sure it doesn't break anything on linux or mac, since file
system stuff tends to be rather platform dependent (things like line endings,
file io buffering, etc). It would also be nice to know if this fixes the problem
on linux and/or mac as well.

Comment 8

[David :Bienvenu]

Comment on attachment 89993 [details] [diff] [review]
proposed fix

sr=bienvenu, but I really think this should be at least run on linux and mac.

Comment 9

[Navin Gupta]

Our nsOutputFileStream::Write is platform independent. I don't think anything
should change but I can run the fix on other platforms. BTW, this is for trunk,
so could I use release builds after I have checked in. 

Comment 10

[Cavin Song]

Comment on attachment 89993 [details] [diff] [review]
proposed fix

r=cavin.

Comment 11

[David :Bienvenu]

yes, a release build is OK - you should, however, get the error message text
reviewed by robinf. Would we be putting up that same error message in the case
of a move?

Comment 12

[Navin Gupta]

yes, I ran the fix on mac and linux and it doesn't break anything, though I have
not tested the fix. yes, we would be putting the same error msg for move also.
It is the copy that is failing so I think it is ok. 

Comment 13

[David :Bienvenu]

whoa, wouldn't a move fail in exactly the same way? Internally, a move is simply
a copy followed by a delete.

Comment 14

[David :Bienvenu]

and, to elaborate, the delete doesn't save any disk space because you'd need to
compact the local folder before any of the disk space would be reclaimed.

Comment 15

[Navin Gupta]

I am saying move and copy will show the same alert because it is 
actual copying of msg that fails, so the alert wording will be same for both cases. 

Comment 16

[David :Bienvenu]

ah, sorry. I'm saying that the user may be doing a move, and thus to see a
message saying that the copy failed might be a bit confusing/wrong.

Comment 17

[robinf]

+copyMsgWriteFailed=...

Suggested text: "The messages could not be moved or copied to folder '%S'
because writing to the folder failed. From the File menu, choose Compact
Folders, and then try again."

Comment 18

[Navin Gupta]

I can change "copied" or "moved/copied" if robin also agrees.

Comment 19

[Navin Gupta]

robin, Compact will not work. So we need to alert the user that there is no disk
space (this is important).

Comment 20

[David :Bienvenu]

unfortunately (an understatement!), compact folders might not work in true low
disk situations, because to compact the folder, we must first copy all the
non-deleted messages into a new temporary folder, before we can reclaim space
from the original folder. I know we like to offer a remedy to problems in our
error messages - in this case, recommending emptying the trash and then maybe
compacting folders might be better advice.

Comment 21

[Navin Gupta]

I meant Compact *may* not work. why can't we ask user to create free disk space
and try again, something like we have for other similar write failure alerts
(like the compact one...)

"Verify that you have enough disk space, and that you have write privileges to
the file system, then try again."

Comment 22

[David :Bienvenu]

I think we *should* ask them to free up disk space, and emptying the trash is
the best way to free up disk space, at least from our app.

Comment 23

[robinf]

"The messages could not be moved or copied to folder '%S' because there is not
enough disk space. To gain disk space, from the File menu, first choose Empty
Trash, and then choose Compact Folders, and then try again."

Comment 24

[Navin Gupta]

fixed on trunk.

Comment 25

[David :Bienvenu]

if we're proposing this for the release, note that there's a ui resource change
required.

Comment 26

[scottputterman]

I'm not sure we need this for this release.  We need verification and l10n
approval before it could be considered.

Comment 27

[scottputterman]

we're adding adt1.0.1-.  Let's get this in the next release.

Comment 28

[Sheela Ravindran]

I was able verify this on both WinNT and Linux rh6.2. I still have to check this
on Mac machine. 
We do not move the message if we run out of disk space. And I did see the dlg
come up when trying to move a message when the disk space was full.  But the
wording is a little bit different from what has been suggested in comment # 23.
I will break a seperate bug for that.  But we don't loose the message anymore.  

Commercial trunk build:  07-03-08 on win98, linux rh6.2 fixed. Will try to
verify this on the mac as well[looking for a machine to test] Any means to tweak
easily on Mac to fill the disk space?  I have a dual boot OS X and Mac 9.1 with
21.55GB available space. 

Comment 29

[esther]

Using branch build 20030610 on macosx this is fixed. verfied. 
I tried to move a message that exceeded my disk space size and received and
error that there was not enough disk space to move the message.  The message or
message header did not get moved and was still intact after the failure. Verified