289615 - bookmark dataloss in abnormal windows shutdown, especially FAT32 systems

Status: RESOLVED WORKSFORME

(Firefox :: File Handling, defect)

Description

[Asa Dotzler [:asa]]

I've been trying to reproduce the bookmark and other dataloss problems on
Windows (both winXP and Win2K on NTFS) with no success for quite a while (bug
193749). After setting up a Win2K environment with FAT32 I can reliably
reproduce a variety of dataloss bugs during abnormal windows shutdowns (hard
reboots). 

I haven't figured out exactly what causes the specific dataloss issues, but I
can lose bookmarks, other profile data like cookies, or the full profile.
Bookmarks and the full profile lossage seems easiest to reproduce. 

Steps to reproduce:
1. Set up a Windows install on FAT32
2. Install Firefox (I've tested 1.0 and 1.0.2 and both are affected)
3. Open Firefox and do various browsing activities. 
4. Either with Firefox running or shortly after Firefox shutdown, power down the
machine.
5. Restart the machine and allow windows to perform the checkdisk function.

Results:
a. open \Application Data\Mozilla and see it's empty - launching Firefox will
offer the Migration wizard. This seems to happen if you power down Windows
sometime soon after your initial Firefox install, but not limited to first
Firefox run.
b. open the \Application Data\Mozilla\Firefox directory and see nothing but
these files: pluginreg.dat, Profiles (not a folder, a file,) and profiles.ini
(and registry.dat in the parent dir.) The user will either get a Profile Wizard
that claims the default profile is in use, or a new profile will be created. In
some cases the user will get a "Profile couldn't be created. Probably the chosen
folder isn't writable." dialog.
c. open profile folder and see that bookmarks.html is gone but bookmarks.bak is
fine. User experiences no problem.
d. open profile and find that bookmarks.html and bookmarks.bak are both gone
(I've also seen prefs.js and localstore disappear.) User starts up with an empty
bookmarks folder and personal toolbar.

Expected Results:
We should find and use, if possible, some Windows API to flush the FAT32 cache
on browser exit, perhaps more regularly like say every 5 minutes.

Please do not dupe this bug against any other bugs, especially bug 193749. If
and when we come up with a solution to the the lameness of FAT32 here, then we
can see what's left at 193749 and create new bugs for those specific issues.

Comment 1

[Adam Doppelt]

The impact on users can be quite severe. On my girlfriend's laptop (FAT32)
closing firefox frequently crashes win32, which almost always results in lost
bookmarks and/or profile problems. The overall effect is that around 60% of the
time closing firefox results in lost bookmarks. She has gradually learned to
never close the browser. Maybe that's not so bad after all. :)

I have no information on the shutdown crash, since it takes down win32.

Comment 2

[Asa Dotzler [:asa]]

Please do not comment here unless you are involved in actually fixing the
problem or have specific knowledge about the FAT32 system or Windows APIs that
could be used to help solve this problem. Thanks.

Comment 3

[Stuart Parmenter]

Attachment: Flush before closing nsSafeFileOutputStreams

Part one of fixing this involves being sure we call Flush before closing in
order to ensure that the file gets written to disk.  NSPR seems to do the right
thing on each platform when you call PR_Sync.

What this patch doesn't address is the temp file/rename/etc stuff here.  On
Windows NT based OSes at least we could change the use of MoveFile to
MoveFileEx with MOVEFILE_WRITE_THROUGH.  This would solve the rename not
getting flushed to disk problem, but we can't use MoveFileEx on 95/98/ME. 
There doesn't seem to be a good general solution for ensuring that a file is
flushed to disk after it is renamed.  According to the windows docs, you can't
flush a file that was opened readonly, so just reopening/flushing/closing the
renamed file won't solve the problem here either.

I'm not sure what the right thing to do here is, but I think doing a direct
write to the file followed by a flush (or even with flushing in the middle) is
currently after than what we've got now with the delete/rename scheme.

Comment 4

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to comment #3)
> I'm not sure what the right thing to do here is, but I think doing a direct
> write to the file followed by a flush (or even with flushing in the middle) is
> currently after than what we've got now with the delete/rename scheme.

I'm not sure; this is what we had before, and it causes a different set of
problems -- dataloss whenever a write fails, due to things like disk failure,
network hiccup, whatever.  The flush-and-move *should* be our best bet, but it
fails badly on older systems.  Perhaps what we really want is to GetProcAddress
MoveFileEx, and decide based on whether it's available or not?

Comment 5

[Jo Hermans]

(In reply to comment #3)
> What this patch doesn't address is the temp file/rename/etc stuff here.  On
> Windows NT based OSes at least we could change the use of MoveFile to
> MoveFileEx with MOVEFILE_WRITE_THROUGH.  This would solve the rename not
> getting flushed to disk problem, but we can't use MoveFileEx on 95/98/ME. 
> There doesn't seem to be a good general solution for ensuring that a file is
> flushed to disk after it is renamed.  According to the windows docs, you can't
> flush a file that was opened readonly, so just reopening/flushing/closing the
> renamed file won't solve the problem here either.

see also bug 179641 : use ReplaceFile instead of a delete + Movefile. Would that
be safer to use when the OS crashes in the middle of the operation ?

Comment 6

[Ekhart]

I and most other technically interested users who end up here are not involved
in actually fixing the problem, and only some of us can provide details about
actual data loss situations that can be used to help solve this problem. 

Nevertheless, our interests and concerns should be addressed here because we can
explain to technically clueless users (who now make up the majority of FF users)
what is being done to address this serious issue and can prevent FF from
*quickly* getting a very bad name *if* the following two suggestions are quickly
implemented:

1) Since the Bookmark Backup extension provides a realistic workaround to
prevent major catastrophe, this should be immediately included in the official
download version until this major design flaw has been corrected.

2) Please give more details on how FF handles bookmarks. E.g.

Could someone please answer the questions in comment 117 on bug 193749 ?

As explained in 
http://forums.mozillazine.org/viewtopic.php?p=1212567#1212567
my comment 106 in bug 193749 was of course wrong in guessing at the cause (as i
already supposed then).

But since then, comment 117 in that bug has confirmed my previously posted
suspicion about FF keeping bookmarks.html continuously open instead of only
opening it when the user adds or deletes a bookmark. As far as i understand the
experts' comments about flushing and MoveFile and ReplaceFile, these don't seem
to contradict my naïve guess.

Also, is it true that FF's current automatic backup does the same nonsense MS
did with system.dat and da0, i.e. rewrites bookmarks.bak with a backup of the
currently open version? Would't it be easy to implement a solution where FF
instead makes a new bookmarksX+1.bak and leaves the previous one untouched with
the name bookmarksX.bak?

Now, when bookmarks.html is destroyed, bookmarks.bak is a copy of that useless
file. Implementing a solution with up to bookmarksX+9 could probably be done
easily and rapidly. Probably 90% of users would never notice or at least never
complain if this bug would at worst destroy only bookmarks added since the last
FF restart.

Comment 7

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

There are a number of issues that compound this problem, one of which is that
due to the structure of the bookmarks file (or lack thereof), it's impossible to
tell whether a bookmark file is really corrupted or not, and automatically try
the backup.  There are potential stopgap measures that can be taken, but the
right thing to do is to find a fix for 1.1, and discussion about how to do that
is currently in the works.

> But since then, comment 117 in that bug has confirmed my previously posted
> suspicion about FF keeping bookmarks.html continuously open instead of only
> opening it when the user adds or deletes a bookmark. As far as i understand the
> experts' comments about flushing and MoveFile and ReplaceFile, these don't seem
> to contradict my naïve guess.

Firefox does not keep bookmarks.html open; it writes the file en-masse every N
seconds if there are any changes that need to be written.  (I think N = 15 right
now, though I'd have to check.)

> Also, is it true that FF's current automatic backup does the same nonsense MS
> did with system.dat and da0, i.e. rewrites bookmarks.bak with a backup of the
> currently open version? Would't it be easy to implement a solution where FF
> instead makes a new bookmarksX+1.bak and leaves the previous one untouched with
> the name bookmarksX.bak?

I'm not sure what you mean here; when bookmarks.html is about to be written, the
old bookmarks.html is moved to bookmarks.bak.  The .bak stuff was implemented as
a half-measure for this problem, because noone could find a reproducible case of
it before 1.0 shipped.  We now have a reproducable case and the problem is
better understood.

> Now, when bookmarks.html is destroyed, bookmarks.bak is a copy of that useless
> file. Implementing a solution with up to bookmarksX+9 could probably be done
> easily and rapidly. Probably 90% of users would never notice or at least never
> complain if this bug would at worst destroy only bookmarks added since the last
> FF restart.

Sure, except as I said, you have no way of knowing when to go back to a backup
file.  Having to do this manualy /is/ better than losing the file altogether,
but only marginally so.

Comment 8

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

One possible band-aid is to change SafeOutputStream to only write a backup if
one hasn't been written yet today (check date on the .bak file, and only write
if it's one day older), and to keep the last N (default to 5?) backup-days'
worth of files around.  Then we can check for file size, and if it changed
drastically, we can do something intelligent.

We may be able to write some sort of sentinel at the end of bookmarks.html, and
warn if it's not found, but is found at the end of some .bak file.

Helpwanted on both these things though; I doubt I'll have time to do either
before 1.1.

Comment 9

[Frank Wein [:mcsmurf]]

(In reply to comment #8)
> We may be able to write some sort of sentinel at the end of bookmarks.html, and
> warn if it's not found, but is found at the end of some .bak file.

I think this alone would not help here (at least not on FAT32, on NTFS we could
give it a try, maybe just checking on <!DOCTYPE NETSCAPE-Bookmark-file-1> in
first line since normally the whole file is corrupted). Comment 0 says that one
case is bookmarks.html gone and bookmarks.bak fine, this is already handled
fine. The other case would be bookmarks.html and bookmarks.bak gone, so here we
would need to combine this with making multiple backups of bookmarks.html then...

Comment 10

[Chris Beard]

according to vlad, pav's patch won't work for anything else than NT4.0 (or
Win2k?).  we'd really like to see this fixed, but it is at risk.  it's tough. 
fix likely needs to be done in SafeOutputStream as we have the same potential
problem with all file writing, e.g. bookmarks, cookies, saved passwords, etc.  

dwitte, any chance you would be able to take a look?

/cb

Comment 11

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Sorry, I was thinking of a different patch.  This patch will work, but it
probably won't help much -- we tried this patch with Asa's machine, and it was
still having the same lossage.  Flush seems to flush the temp file to disk, but
when we rename we can still lose the file.  We should probably check in this
patch anyway, though.

The patch I was thinking of was pav talking about opening these files with some
WRITE_THROUGH flag, so that data is immediately written to disk instead of
getting stuck in buffer cache.  But that flag is only available on win2k+.  He
should probably respond since he's been looking at this.

Comment 12

[Jaime Mitchell (use [e-mail] for email)]

(In reply to comment #11)
>Flush seems to flush the temp file to disk, but
> when we rename we can still lose the file. 

Is that possibly related to bug 298943 ?

Comment 13

[Mike Connor [:mconnor]]

filed Bug 305004 on a workaround for this, will fix that for beta.

Comment 14

[José Jeria]

*** Bug 318014 has been marked as a duplicate of this bug. ***

Comment 15

[VanillaMozilla]

Reports of bookmark loss continues in versions 1.5+.  This bug could be fixed with a lock file, which would simply record whether "bookmarks.html" was properly closed.

Should this be marked critical, since data loss is still possible even with a daily backup?  Should I file a separate bug with the proposed solution?

Comment 16

[Bogdan Enache]

(In reply to comment #0)
> Please do not dupe this bug against any other bugs, especially bug 193749. If
> and when we come up with a solution to the the lameness of FAT32 here, then we
> can see what's left at 193749 and create new bugs for those specific issues.

This bug (and obviously 193749) affected me on Windows XP with NTFS partitions a few times. So I guess it doesn't have to do with FAT32, rather with improper application design.

And I didn't lost only bookmarks, but also search engines and toolbar customization, which now appear to be locked (I can't add anything to search engines for example).

Comment 17

[Bogdan Enache]

(In reply to comment #15)
> Should this be marked critical, since data loss is still possible even with a
> daily backup?
> 

Not only that you can lose data, but some things you can't add back either (for example adding back search engines and toolbar customization didn't work anymore for me after power failure; search engines were simply not added from https://addons.mozilla.org/search-engines.php when clicked, but without any error or warning). I had to uninstall and clean everything manually, and then reinstall.

So yes, this should be a critical bug.

Comment 18

[Mona]

I think this bug (or rather group of bugs) should be considered from diferent point of view. 
1) it would be beter to work over method unrelated with any specific OS, I  mean files handling operations specific in this system . 
2) everytime after change bookmark file (and/or other considered) it (them) sould be on HD in state ready-to-load-again-like-at-FF-start, ready to use. 
So: after any change file sould be updated and "closed" on HD. And during this operation a copy (copies?) of previus file should be maintained, just in case of eg. powerfailure etc. in this moment. HD operations seems to me reasonable cost versus manual restoring data.
3) Usually new bookmark will be larger so ff can compare at start size of files and if file-is-to-be-loaded is smaler then last backup ff can ask user if: 1-go on, 2-load last backup 3-import bookmarks from every saved backups 4-import bookmarks from backups saved whitin entered time period. 
4) There are backup files on disk but there is none easy way to import from them  anything, unless you are more experinced user. Perhaps it would be reasonable to implenment "load bookmarks form backup" option? As suggested in bug 193749 Comment #46 From Christian Flatscher  2003-11-26 06:49 PDT

5) This problems,  lost bookmarks etc. , I have observed with FF1.5.0.1 when closed Win2k with IBM laptot specific button which close system but it is not "hard switch of". System just close. I eveluate them as critical cause it makes any serious work imposible, I have lost essential data for my client and if I would not have backup I would have serious financial and low consequences. This is like unreliable HD. Exclude this good soft  from anything besides porno sites and games.

My expectation:
1) Method of handle, update, manage user data file(s) should be safe form crash (total or partial) of system or ff anytime during sesion. Acceplable is lost data older then 1 - 2 minutes berfore crash (if we want to have reliable soft for serious user) 
2) Easier way to import data from backups of bookmarks, settings, etc. would be good help or temporary solution.

Comment 19

[Bogdan Enache]

I think a better idea is to use SQLite for most user data, excepting browser cache, of course. SQLite fully supports transactions (ACID), so data loss is impossible.

Comment 20

[Jo Hermans]

(In reply to comment #19)
> I think a better idea is to use SQLite for most user data, excepting browser
> cache, of course. SQLite fully supports transactions (ACID), so data loss is
> impossible.
> 

Firefox 3.0 (not 2.0, which is now in beta) already uses SQLlite.

Comment 21

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

sqlite doesn't really help use here -- it's ACID only as long as the underlying OS fs can give it certain guarantees.  FAT32 apparently cannot; I believe a sqlite-backed store would suffer from similar corruption issues.

Comment 22

[Mark Grant]

(In reply to comment #21)
> FAT32 apparently cannot; I believe a
> sqlite-backed store would suffer from similar corruption issues.

Possibly. But I think it's telling that before I reformatted my system disk to NTFS from FAT32 the only thing I ever lost in a system crash or power dropout was Mozilla bookmarks, and I lost them pretty much every time I had such a crash. The video editing program I was using back then would crash the PC regularly, and almost every time I rebooted afterwards the bookmarks were corrupt.

I haven't lost them a single time so far with NTFS, though I've rarely had a crash or power failure since.

Comment 23

[Wayne Mery (:wsmwk)]

cbeard in comment #10:
> according to vlad, pav's patch won't work for anything else than NT4.0 (or
> Win2k?).  we'd really like to see this fixed, but it is at risk.  it's tough. 
> fix likely needs to be done in SafeOutputStream as we have the same potential
> problem with all file writing, e.g. bookmarks, cookies, saved passwords, etc.  
> 
> dwitte, any chance you would be able to take a look?


dwitte's results?

Comment 24

[Jason Barnabe (np)]

One of the top support issues, requesting blocking1.9.

Comment 25

[Wayne Mery (:wsmwk)]

workaround implemented by Bug 305004 http://kb.mozillazine.org/Lost_bookmarks#Restoring_bookmarks_from_backup

Comment 26

[Benjamin Smedberg]

I believe that this is fixed, but it's going to be hard to test until widespread betas: the places backend is sqlite which is much more fault-tolerant than the bookmarks.html backend, and we took several patches that shut down Firefox properly when the OS is shut down.

Please renominate if this can be reproduced in places-trunk.

Comment 27

[Wayne Mery (:wsmwk)]

with FF3 out for almost a year, is this bug ready to be retirement? Or, is the patch potentially useful for some more general purpose?

Comment 28

[Andrei Purice]

Hey Asa,
Can you still reproduce this issue or can we consider it as fixed?