Closed Bug 294584 Opened 16 years ago Closed 14 years ago

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

Categories

(Core Graveyard :: File Handling, defect)

1.8 Branch
PowerPC
macOS
defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.8.1beta2

People

(Reporter: dsaur, Assigned: mark)

References

Details

(Keywords: fixed1.8.1, Whiteboard: [no l10n impact])

Attachments

(2 files, 1 obsolete file)

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.
Summary: Spotlight contention wiht bookmarks.html causing multiple backups? → Spotlight contention with bookmarks.html causing multiple backups?
Version: unspecified → 1.0 Branch
Severity: normal → major
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.
Is anybody working on Firefox ever going to take notice of this bug?

Drew
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.
(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
My bet is that Firefox tries to delete the temporary backup file, but can't
because it's still being indexed.
Assignee: nobody → joshmoz
(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]
Flags: blocking1.8b4?
Flags: blocking1.8b4+
Flags: blocking-aviary1.1?
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.
You can also put ".noindex" as the file extension, and the file will remain
visible but spotlight won't index it.
This output shows exactly what is going on. mdimport=spotlight
Using .noindex makes sense.
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.)
If we can do this we'll use .noindex, and that doesn't hide files from users.
See comments 8 and 10.
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.
The creation of extra bookmarks-X.html files is certainly not exclusive to 10.4,
this was happening in 10.3/Panther as well.
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
Flags: blocking1.8b4+ → blocking1.8b4-
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. ***
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. ***
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?
This problem still occurs with Firefox 1.5 on Mac OSX 10.3.9
(Panther not Tiger).
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
See bug 157152 for bookmarks-999.html problems that are not caused by Spotlight.
I've fixed this bug putting my Firefox profile directory in non-indexed list in Spootlight's preferences.
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...
(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.


(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;=
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.
*** Bug 321566 has been marked as a duplicate of this bug. ***
*** Bug 343201 has been marked as a duplicate of this bug. ***
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. ***
Yup, that's what's going on.
Assignee: joshmoz → mark
Target Milestone: --- → Firefox 2 beta2
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)
Attachment #231441 - Flags: review?(joshmoz) → review+
I highly recommend putting the fix for this bug on the 1.8.1 branch.
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?
Attachment #231441 - Flags: superreview?(darin) → superreview+
Attached patch As checked inSplinter Review
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?
Status: NEW → RESOLVED
Closed: 14 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
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?
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.
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.
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+
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
Depends on: 347778
*** Bug 347240 has been marked as a duplicate of this bug. ***
*** Bug 326111 has been marked as a duplicate of this bug. ***
Apple now have a developer note about this issue:
http://developer.apple.com/qa/qa2006/qa1497.html
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.