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

RESOLVED FIXED in mozilla1.8.1beta2

Status

Core Graveyard
File Handling
--
major
RESOLVED FIXED
12 years ago
10 months ago

People

(Reporter: Drew D. Saur, Assigned: Mark Mentovai)

Tracking

({fixed1.8.1})

1.8 Branch
mozilla1.8.1beta2
PowerPC
Mac OS X
fixed1.8.1
Bug Flags:
blocking1.8b5 -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [no l10n impact])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

12 years ago
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.
(Reporter)

Updated

12 years ago
Summary: Spotlight contention wiht bookmarks.html causing multiple backups? → Spotlight contention with bookmarks.html causing multiple backups?
(Reporter)

Updated

12 years ago
Version: unspecified → 1.0 Branch
(Reporter)

Updated

12 years ago
Severity: normal → major

Comment 1

12 years ago
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.
(Reporter)

Comment 2

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

Drew

Comment 3

12 years ago
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.
(Reporter)

Comment 4

12 years ago
(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

12 years ago
My bet is that Firefox tries to delete the temporary backup file, but can't
because it's still being indexed.
Assignee: nobody → joshmoz

Comment 6

12 years ago
(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?
Flags: blocking1.8b4?
Flags: blocking-aviary1.1?
Whiteboard: [no l10n impact]

Updated

12 years ago
Flags: blocking1.8b4?
Flags: blocking1.8b4+
Flags: blocking-aviary1.1?

Comment 7

12 years ago
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

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

Comment 9

12 years ago
Created attachment 189979 [details]
filtered fs_usage output

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

Comment 10

12 years ago
Using .noindex makes sense.

Comment 11

12 years ago
This appears to be a bug in Apple's HTML spotlight importer. I filed a bug with
Apple, radar ID 4188208.
(Josh, users should have access to this file, hiding it in the finder isn't a
good plan.)

Comment 13

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

Comment 14

12 years ago
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).
(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

12 years ago
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

12 years ago
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

Updated

12 years ago
Flags: blocking1.8b4+ → blocking1.8b4-

Comment 18

12 years ago
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.
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.
*** Bug 311829 has been marked as a duplicate of this bug. ***

Comment 21

12 years ago
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.
*** Bug 288139 has been marked as a duplicate of this bug. ***

Comment 23

12 years ago
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

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

Updated

12 years ago
Summary: Spotlight contention with bookmarks.html causing multiple backups? → Spotlight contention with bookmarks.html causing multiple backups (boomarks-n.html created)
Version: 1.0 Branch → Trunk

Comment 25

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

Comment 26

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

Comment 27

11 years ago
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

11 years ago
(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

11 years ago
(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;=

Updated

11 years ago
Summary: Spotlight contention with bookmarks.html causing multiple backups (boomarks-n.html created) → Spotlight/Norton Antivirus contention with bookmarks.html causing multiple backups (boomarks-n.html created)
(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?)
(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

11 years ago
*** Bug 321566 has been marked as a duplicate of this bug. ***
*** Bug 343201 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 34

11 years ago
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.
*** Bug 346313 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 36

11 years ago
Yup, that's what's going on.
Assignee: joshmoz → mark
Target Milestone: --- → Firefox 2 beta2
(Assignee)

Comment 37

11 years ago
Created attachment 231441 [details] [diff] [review]
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.)
Attachment #231441 - Flags: review?(joshmoz)

Updated

11 years ago
Attachment #231441 - Flags: review?(joshmoz) → review+

Comment 38

11 years ago
I highly recommend putting the fix for this bug on the 1.8.1 branch.
(Assignee)

Updated

11 years ago
Attachment #231441 - Flags: superreview?(darin)
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.)
Attachment #231441 - Flags: approval1.8.1?

Updated

11 years ago
Attachment #231441 - Flags: superreview?(darin) → superreview+
(Assignee)

Comment 40

11 years ago
Created attachment 231536 [details] [diff] [review]
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.
Attachment #231441 - Attachment is obsolete: true
Attachment #231536 - Flags: approval1.8.1?
Attachment #231441 - Flags: approval1.8.1?
(Assignee)

Updated

11 years ago
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Component: Bookmarks → File Handling
Flags: review+
Product: Firefox → Core
Resolution: --- → FIXED
Target Milestone: Firefox 2 beta2 → mozilla1.8.1beta2
Version: Trunk → 1.8 Branch

Comment 41

11 years ago
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?
Are there any issues mentioned in bug 118203 or the bugs it refers to that are worth retesting with this patch?
(Assignee)

Comment 43

11 years ago
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.
(Assignee)

Comment 44

11 years ago
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

11 years ago
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 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
Attachment #231536 - Flags: approval1.8.1? → approval1.8.1+
(Assignee)

Comment 47

11 years ago
Checked in on MOZILLA_1_8_BRANCH for 1.8.1b2.  Darin, we can come up with some unit tests separately.
Keywords: fixed1.8.1
(Assignee)

Updated

11 years ago
Depends on: 347778

Comment 48

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

Comment 49

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

Comment 50

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

Updated

10 years ago
Duplicate of this bug: 337812
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.