294584 - Spotlight/Norton Antivirus contention with bookmarks.html causing multiple backups (boomarks-n.html created)

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect)

Description

[Drew D. Saur]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.8) Gecko/20050511 Firefox/1.0.4
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.8) Gecko/20050511 Firefox/1.0.4

Despite proper file permissions and apparent similarity to many other "multiple
bookmarks.html" (e.g. bookmarks-1.html, bookmarks-2.html, etc) reports, I have
noticed a preponderance of such backup files in 1.0.4 since moving to Mac OS X
10.4 Tiger. Disabling Spotlight's indexing of the Firefox profile folder put
that to rest.

It seems like Spotlight is causing contention with Firefox upon the close of the
browser as bookmarks are being written, and it takes ownership of the file
before Firefox is done with it, causing a backup copy to be written.

Reproducible: Sometimes

Steps to Reproduce:
1. Use Firefox; do something that causes bookmarks to change, even if it's a
"favicon" resource change.
2. Quit
3. Check your profile folder for 'bookmarks-x.html" You'll get one eventually.
4. Disable Spotlight indexing for your Profile folder (using the Privacy tab in
the Spotlight preference pane), and it won't happen again.

Comment 1

[ozjason_bugmail]

Nice workaround.. I hadn't even considered Spotlight might be the culprit.

I can confirm I'm also seeing this problem on OS X 10.4.1 with FF 1.0.4. It's
potentially very serious.. I've been running OS X 10.4 for only 2 weeks and
found 60 copies of my bookmarks.html cluttering the profile directory. My
bookmarks file is close to 1MB in size.. if I hadn't discovered it, it could've
potentially chewed up a lot of disk space.

Comment 2

[Drew D. Saur]

Is anybody working on Firefox ever going to take notice of this bug?

Drew

Comment 3

[John B. Brown]

This problem has occured with my Windows-98 SE installation. I accumulated close
to a gigabyte of bookmarks-x.html files in my personal Profile directory before
my disc started to complain; a two gig partition. I did a wipe from the Linux
side of this box but the phemonenon is still occuring. I'll look for this
Spotlight thing, whatever that is; I have no prior experience with such a
preference.

Comment 4

[Drew D. Saur]

(In reply to comment #3)
> This problem has occured with my Windows-98 SE installation. I accumulated close
> to a gigabyte of bookmarks-x.html files in my personal Profile directory before
> my disc started to complain; a two gig partition. I did a wipe from the Linux
> side of this box but the phemonenon is still occuring. I'll look for this
> Spotlight thing, whatever that is; I have no prior experience with such a
> preference.

This is a Mac OS X-specific bug. The error you are seeing is covered elsewhere.

Drew

Comment 5

[Simon Fraser [no longer active]]

My bet is that Firefox tries to delete the temporary backup file, but can't
because it's still being indexed.

Comment 6

[Full Decent]

(In reply to comment #5)
> My bet is that Firefox tries to delete the temporary backup file, but can't
> because it's still being indexed.

Solution: check for temporary backup files on startup and delete if found?

Comment 7

[Josh Aas]

Maybe we should prefix the backup files with a "." - spotlight won't index
hidden files, and we don't want the perf hit of it indexing them anyway. So we
solve the problem and get a small perf benefit.

Comment 8

[Josh Aas]

You can also put ".noindex" as the file extension, and the file will remain
visible but spotlight won't index it.

Comment 9

[Josh Aas]

Attachment: filtered fs_usage output

This output shows exactly what is going on. mdimport=spotlight

Comment 10

[Simon Fraser [no longer active]]

Using .noindex makes sense.

Comment 11

[Josh Aas]

This appears to be a bug in Apple's HTML spotlight importer. I filed a bug with
Apple, radar ID 4188208.

Comment 12

[Asaf Romano (gone)]

(Josh, users should have access to this file, hiding it in the finder isn't a
good plan.)

Comment 13

[Josh Aas]

If we can do this we'll use .noindex, and that doesn't hide files from users.
See comments 8 and 10.

Comment 14

[Josh Aas]

We can't really change the extension because the new file name appears to be a
result of a failure to open the existing file with nsFileOutputStream (we give
it some a create-if-necessary flag).

Maybe we should try cleaning up the files during startup or shutdown (e.g.
taking the highest numbered one, deleting all the rest, and renaming it to
bookmarks.html).

Comment 15

[Asaf Romano (gone)]

(In reply to comment #14)
> We can't really change the extension because the new file name appears to be a
> result of a failure to open the existing file with nsFileOutputStream (we give
> it some a create-if-necessary flag).

ah? The filename (on both sides) is defined in nsBookmarksService.

Comment 16

[Gary Elshaw]

The creation of extra bookmarks-X.html files is certainly not exclusive to 10.4,
this was happening in 10.3/Panther as well.

Comment 17

[Josh Aas]

In response to comment #15...

josh: we don't define the fallback name
josh: we can't rename bookmarks.html, and we wouldn't want to anyway
josh: we want bookmarks to get indexed
Mano: agreed, i was under the impression that the probem is with bookmarks.bak

Comment 18

[Floyd Miller]

This problem of multiple bookmark files being created, one each time Firefox is
quit, also occurs with Mac OSX Panther (10.3.9).  It is currently happening with
Firefox 1.0.7 but I am sure it was happening with 1.0.6 and 1.0.5 based on the
age of some of the older bookmark-n.html files.

There's no spotlight in Panther, but there is search indexing.  However the
indexing is not enable on this computer.  I have tried making the orignal
bookmarks.html file and the folder in which it is stored writable by all but
that has not solved the problem.

Comment 19

[Mike Pinkerton (not reading bugmail)]

we shouldn't care if the bookmarks file gets indexed because there's nothing the
importer can do with a file of multiple bookmarks. it can associate it with
firefox, but spotlight can't do anything with that information beyond opening
the default browser to load the html file.

i'd say we should make the file noindex.

Comment 20

[Asaf Romano (gone)]

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

Comment 21

[Quiroga]

For me it's work and the problem isn't happen again.

It's a spotlight (for 10.4,Tiger system) how's make a index and cause the
confusion on Firefox bookmarks. 

You need to ask spotlight preference to don't index the folder
("user"/Library/Application
Support/Firefox/Profiles.

Go to system preferences, spotlight, Privacy and add the folder.


Thank's all you.

Comment 22

[Samuel Sidler (old account; do not CC)]

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

Comment 23

[Tom Hessman]

I've had the boomarks-n.html problem for a very long time...I was up to bookmarks-1286.html and about 357MB!  Firefox created a new one every single time I quit, regardless of whether I altered my bookmarks or not.  Setting Spotlight not to index my profile fixed it!  I'm relieved to see it finally fixed, having been confused since the main bug that resembles that issue has to do with the file being read-only and hasn't had any significant activity.

Perhaps this should be listed in the release notes?

Comment 24

[Floyd Miller]

This problem still occurs with Firefox 1.5 on Mac OSX 10.3.9
(Panther not Tiger).

Comment 25

[Frankie]

See bug 157152 for bookmarks-999.html problems that are not caused by Spotlight.

Comment 26

[Francesco Albano]

I've fixed this bug putting my Firefox profile directory in non-indexed list in Spootlight's preferences.

Comment 27

[Tom Hessman]

I wonder if other programs that monitor file access and creation, such as Norton Antivirus, can cause this too?  I disabled indexing in Spotlight of my Firefox profile, which seemed to work for a while, but last week when I was making a quick backup of my bookmarks before sending my computer off for repair I noticed that there were multiple numbered copies again!  I hadn't changed Spotlight settings, but I had semi-recently re-enabled Norton's Auto-Protect feature...

Comment 28

[Floyd Miller]

(In reply to comment #27)

I tested this on my sysotem running Panther, no Spotlight.
With Norton Antivirus Auto-protect turned off, the bookmarks-n.html
are not created - or at least not left in the folder.

When I re-enable auto-protect the problem returns.  Not the
first time I launch and quit Firefox, but starting with the
2nd time after re-enabling auto-protect.

Comment 29

[Tom Hessman]

(In reply to comment #28)
> (In reply to comment #27)

It sounds like this problems gets caused by an program which "locks" the file while scanning it, including Spotlight and Norton Antivirus.  Can anybody think of any other programs that cause this behavior?  Has anyone ver seen this happen with Norton on Windows?

The fact that NAV does it explains why I thought I remembered it happening before Tiger...it probably was!  Now I have another thing to add to my incredibly long list of what not to scan for viruses...(c;=

Comment 30

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

(In reply to comment #9)
> Created an attachment (id=189979) [edit]
> filtered fs_usage output
> 
> This output shows exactly what is going on. mdimport=spotlight

I'm not yet skilled at reading fs_usage output: where in there is Firefox trying to do something and failing, and where is mdimport establishing whatever condition it is that causes it to fail?

(Also: is there any update on the Radar report you entered for this?)

Comment 31

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

(In reply to comment #14)
> We can't really change the extension because the new file name appears to be a
> result of a failure to open the existing file with nsFileOutputStream (we give
> it some a create-if-necessary flag).

I'll confess that I'm having a heckuva time tracing that behaviour case here.  Specifically, why the rename-over-existing-file part is failing (what system calls are being made, and what error they're giving).  I've been reading ktraces of mdimport, and I can't see it acquiring any locks on the bookmarks file when it indexes it, or the containing directory.  It just opens the file read-only (a few times, but hey, who's counting?), makes some fcntl() calls to suppress some default caching behaviour and check if it's inside a "package", and then reads the data.  My understanding of OS X (and Unix) filesystem semantics are such that an open reader doesn't exclude a writer _or_ a rename of another file over the same name.

I haven't been able to catch Firefox in the act, even when running mdimport in a loop over bookmarks*.html, so I'm hoping someone can tell me better what's causing us to fail, and where, in the safe-output-stream path.

Comment 32

[Josh Aas]

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

Comment 33

[Phil Ringnalda (:philor)]

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

Comment 34

[Mark Mentovai]

This is quite possibly a result of implementing nsIFile::MoveTo with File Manager APIs (FSMoveObject) that are more respectful of locks instead of underlying POSIX ones (rename) that give the semantics we're expecting.

Comment 35

[Phil Ringnalda (:philor)]

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

Comment 36

[Mark Mentovai]

Yup, that's what's going on.

Comment 37

[Mark Mentovai]

Attachment: v1, use POSIX calls for rename/unlink/rmdir

This replaces a few File Manager calls with the underlying POSIX equivalents.  We lose File Manager semantics like "can't remove a file that someone else has opened through the File Manager," but we don't care.  It's what Gecko expects, and it's what we expect.  Note that the Spotlight/antivirus process reading the file won't be harmed by this, it'll continue to be able to work with the file until it closes it (typical POSIX semantics).

This should clear up the bookmarks-##.html files (and cookies-##.txt and everything else.)

Comment 38

[Josh Aas]

I highly recommend putting the fix for this bug on the 1.8.1 branch.

Comment 39

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 231441 [details] [diff] [review]
v1, use POSIX calls for rename/unlink/rmdir

I highly recommend requesting approval for it, in that case.  (Agreed, this would be a huge win.)

Comment 40

[Mark Mentovai]

Attachment: As checked in

I found a bug in the original patch:

+    if (NS_FAILED(rv) || leafName.IsEmpty())
+      return rv;

would return NS_OK if leafName was empty.  Looking at GetNativeLeafName, this really isn't a possibility, so I got rid of the leafName.IsEmpty() check.  (If, due to some OS bug, leafName really wound up empty, the rename would fail anyway.)

Also changed this:

+    if (status != 0)
+      return NSRESULT_FOR_ERRNO();

to set rv instead of return, since a return follows anyway.  And I turned the two path.get() calls in Remove into a single one that assigns a const char*, saving duplicated work.

Checked in on the trunk.

As above - this is a big win because it keeps the profile from becoming stuffed with unnecessary temp files.  We should get it on the 1.8 branch.

Comment 41

[Mike Schroepfer]

Mark - is there any chance the change is semantics for rename can have any negative impact elsewhere besides the bookmarks code?  Do we have any automated testcases to test around the edges here to understand how the code works with symlinks, crazy filenames, etc?

Comment 42

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Are there any issues mentioned in bug 118203 or the bugs it refers to that are worth retesting with this patch?

Comment 43

[Mark Mentovai]

We don't depend on File Manager semantics for Remove or Move anywhere as far as I'm aware.  There are actually subtle data-loss problems with using the File Manager impl: if your Move fails because the File Manager forbids you to get rid of the destination file, you'll wind up with an old version of bookmarks (or whatever).  And because Move wasn't implemented atmoically, it would be possible (although unlikely) to remove the old file at the destination and then crash, losing bookmarks or cookies or whatever.  Mozilla follows good practice in writing many of its files by writing a temp file and then renaming it over an existing file, but this practice is only good where renames are reasonable atomic ops.

I've tested this against a good deal of uses of Remove and Move, including app startup, bookmarks manipulation, and file download (to completion and cancelled).  I'm not changing any of the underlying structures, and I'm actually working one step closer to the underlying CFURLs that we store as the authoritative identifiers for file objects (the File Manager calls use FSRefs which we must derive from the CFURLs).  If there are any more specific tests you'd like me to run, please let me know and I'd be glad to run through them.

Comment 44

[Mark Mentovai]

P.S. I also tested paths with UTF-8 non-ASCII characters, and was able to successfully remove and rename things in a directory with a Japanese name.

Comment 45

[Darin Fisher]

Mark,

Is there a simple unit tests that can be constructed (in xpcom/tests) that
would help us ensure that we are only making the changes that we intend to
nsLocalFileOSX?

Comment 46

[Mike Connor [:mconnor]]

Comment on attachment 231536 [details] [diff] [review]
As checked in

a=mconnor on behalf of drivers for checkin to 1.8 branch for 1.8.1

Comment 47

[Mark Mentovai]

Checked in on MOZILLA_1_8_BRANCH for 1.8.1b2.  Darin, we can come up with some unit tests separately.

Comment 48

[Pam Greene]

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

Comment 49

[Mark Mentovai]

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

Comment 50

[Simon Fraser [no longer active]]

Apple now have a developer note about this issue:
http://developer.apple.com/qa/qa2006/qa1497.html