313360 - profile locking doesn't work if the profile is located on a FAT partition

Status: RESOLVED FIXED

(Core Graveyard :: Profile: BackEnd, defect)

Description

[Wolfgang Rosenauer [:wolfiR]]

I got a report that profile locking stuff doesn't work as expected if the
profile is located on a FAT partition (this is the Linux version).

According to the report it works with plain Firefox 1.0.x but it doesn't work
with 1.5b2 (and it doesn't work with the SUSE version of 1.0.x (because the
profile locking change is backported there)

This is most likely a regression from bug #151188

Comment 1

[Wolfgang Rosenauer [:wolfiR]]

the profile is always reported as being in use

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I'm pretty sure that FF1.5 won't block on this... I should be able to fix this
though

Comment 3

[Asa Dotzler [:asa]]

ROC, what would be involved in fixing this and what kind of risk do you think that would introduce?

Comment 4

[Asa Dotzler [:asa]]

Too late in the game for something this edge case to block our release (code complete tonight.)

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

The problem is that you can't create a symlink in a FAT partition. Symlink creation fails and then we bail out of profile locking with an error. This worked before the fix to bug 151188 because if symlink creation failed with other than EEXIST, we'd fall back to fcntl locking, which DOES work in FAT. Bug 151188 made fcntl happen first and also made symlink failure fatal, which was unintentional.

This patch reverts to something like the previous behaviour. We allow execution to proceed if symlink creation fails for any reason other than the symlink already exists --- the net effect is as before bug 151188 was fixed, we will hold an fcntl lock and not the symlink lock. This patch is thus as safe as the old locking code ever was, and will continue to lock out FF1.0 builds (unless some transient non-EEXIST fault occurs, which would have doomed the FF1.0 locking code anyway).

Comment 6

[Brendan Eich [:brendan]]

Comment on attachment 200783 [details] [diff] [review]
fix

This is safe for 1.8, because it suppresses an error returned by LockWithSymlink only if the symlink did not exist.  In which case, we should suppress the error and get on with new-style locking, because there's no symlink and we couldn't make one.

/be

Comment 7

[Brendan Eich [:brendan]]

The failure on Linux FAT partitions is caused by assuming symlink failure other than EEXIST should lead to failure to set the profile directory.  Roc's patch fixes this by making any error other than EEXIST turn into NS_OK and get on with using the profile.  This is after having taken an fcntl (new-style) lock, so the profile is locked against access by other processes.

There is zero risk to Mac and Windows because the patch affects #elif defined(XP_UNIX) code.  The file is small enough, and static is used, that code review plus testing should suffice for getting this into 1.8/fx1.5.  I think we should take it.

/be

Comment 8

[Brendan Eich [:brendan]]

(In reply to comment #6)
> (From update of attachment 200783 [details] [diff] [review] [edit])
> This is safe for 1.8, because it suppresses an error returned by
> LockWithSymlink only if the symlink did not exist.  In which case, we should
> suppress the error and get on with 

using the already-acquired [I meant to write; or something simpler]

> new-style locking, because there's no
> symlink and we couldn't make one.

Again, this is Linux only, and the new-style lock as been successfully acquired so we are only suppressing failure to get an old (Firefox 1.0.x) style symlink lock, and only in the case where the symlink doesn't exist (so there can't be another downrev Firefox process running).

/be

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked into trunk

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

IMHO this is not a stop-ship fix. However, if a window opens for patches to be landed, I'd like to land this one.

Comment 11

[Brendan Eich [:brendan]]

Agreed, it isn't stop ship.

Is the FAT32-based profile for dual booting?  Most profile data is OS-independent, and byte order is either canonical or same on the same machine when dual-booting Windows and Linux.

This is a followup fix to a notable fix to bug 151188 for Firefox 1.5.  While we have time to polish, my quality sense says to do so with provably safe patches. Proof is just a convincing argument; code review may be less convincing than a day or two baking in the trunk.

/be

Comment 12

[Asa Dotzler [:asa]]

not going to lose a day with this but could become a ride-along if we're forced to respin by some other bug.

Comment 13

[Wolfgang Rosenauer [:wolfiR]]

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

Comment 14

[Sébastien Auger]

indeed, this is nothing that will cause another bug. Or if you want, include it before a RC2.

Comment 15

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Could this fix be causing bug 314180?

Comment 16

[Brendan Eich [:brendan]]

(In reply to comment #15)
> Could this fix be causing bug 314180?

No, that report is not against the trunk (see the rv: and Gecko/build-date in bug 314180 comment 0).  It's a dup of this bug.

/be

Comment 17

[Brendan Eich [:brendan]]

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

Comment 18

[Magnus Melin [:mkmelin]]

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

Comment 19

[Asa Dotzler [:asa]]

moving out to the rc2 ride-along list in case we spin an rc2 we can evaluate this for that release.

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

fixed on branch. Thanks.

Comment 21

[Scott MacGregor]

flag clean up, nothing to see here. 

Comment 22

[Simonas Leleiva]

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

Comment 23

[Sean Brannon]

This same problem also effects Firefox and SMB-mounted network home dirs with:

OS: Mac OS x v10.3.x and v10.4.x
Browser: v1.0.7+ (possibly earlier)

This is also reported in bug #309323. This bug is marked as fixed, and yet I am still having this problem.

Versions of Firefox up to and including v1.5 do not work with profile's stored on SMB-mounted network home dirs under Mac OS X. They are either failing to find them, or incorrectly determining that they are in use.

1. Launch Firefox.

	Launch with firefox -P, else in v1.5 we immediately get the error message below, and have to chance to monkey with the profile placement.

2. Select profile.
	Use default or create a new profile (in FF's default location), doesn't matter.
	
3. Get following message:

<= v1.0.7

	"Profile in use Firefox cannot use the profile "default" because it is in use.

	To continue, close the running instance of Firefox or choose a different profile."
	
= v1.5

	"Close Firefox
	
	Firefox is already running, but is not responding. To open a new window, you must close the existing Firefox process or restart your system."

I have verified that the appropriate files are being created in ~/Library/Application Support/Firefox when Firefox is launched (else there wouldn't be a 'default' profile to select). This of course means the user has privleges to write in this location.

However, if instead of choosing a dir in ~, you select a local dir (non-SMB mounted), all is well.


Ideas?

Comment 24

[Sean Brannon]

Forgot to state that I tested this issue and it still occurs on this build:

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.1) Gecko/20060131 Firefox/1.5.0.1

If it's 'FIXED'/'RESOLVED', why is it still happening in this build, or am I missing something?

Comment 25

[Sean Brannon]

Attachment: Text output of running ktrace on Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.1) Gecko/20060131 Firefox/1.5.0.1

Here is a ktrace of running "Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.1) Gecko/20060131 Firefox/1.5.0.1" and it giving me the profile in use message.

This is REALLY driving me nuts. It's holding up our diskless Mac deployment, and my boss is screaming. :-((

Comment 26

[Brendan Eich [:brendan]]

(In reply to comment #24)
> If it's 'FIXED'/'RESOLVED', why is it still happening in this build, or am I
> missing something?

You are reporting a problem when the profile directory is on an SMB-mounted remote filesystem.  That's not what this bug is about -- a FAT partition is a local disk filesystem.  This bug is fixed.  You want bug 309323.

/be