772238 - NS_ERROR_FILE_TARGET_DOES_NOT_EXIST @ XPIProvider.jsm line 1932 when installing addons on B2G

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Jonathan Griffin (:jgriffin)]

We rely on the installation of addons in B2G for testing mochitest and reftest.  However, sometime since June 26, this has stopped working.  The B2G emulator was broken during that time for unrelated reasons, so I cannot identify a narrower regression range.

Our test harnesses copy some extensions to the B2G profile directory, then restart B2G.  When B2G restarts, one of two things happens:

1 - this exception occurs:

E/GeckoConsole(  400): [JavaScript Error: "ERROR addons.manager: Exception calling provider startup: [Exception... "Component returned failure code: 0x80520006 (NS_ERROR_FILE_TARGET_DOES_NOT_EXIST) [nsILocalFile.isDirectory]"  nsresult: "0x80520006 (NS_ERROR_FILE_TARGET_DOES_NOT_EXIST)"  location: "JS frame :: resource://gre/modules/XPIProvider.jsm :: <TOP_LEVEL> :: line 1932"  data: no]" {file: "resource://gre/modules/XPIProvider.jsm" line: 1932}]

The "extensions/staged" directory of the profile is populated with addons before this happens, and is empty afterwards.

2 - No exception occurs, but we get warnings like:

E/GeckoConsole(  392): [JavaScript Warning: "WARN addons.xpi: Ignoring directory whose name is not a valid add-on ID: /data/local/tests/profile/extensions/staged/[e-mail]"]

After this, all the extensions in "extensions" folder have been removed.

I'm cc'ing people on the B2G team in case this is due to some change in B2G.  This blocks us from running mochitests/reftests on B2G.

Comment 1

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Hmm, that's odd - what that code is going is iterating over the contents of a directory, and checking if anything there is a directory. 

Last time we hit something like this, it was due to OSX resource forks on a FAT32 filesystem (bug 733436) - where we'd iterate over the contents of a directory, delete/move a normal file, which made it's resource fork (which appeared as a normal file, with a modified filename) disappear, and then we'd try to check if that was a directory - which would throw with the same exception as what you're getting here. 

In bug 733436 we thought this shouldn't be a problem for the code handling the staging directory - maybe it was wrong to assume that.

I haven't played with the B2G emulator yet - now its working again, I'll try reproducing this. 

Does B2G (or the emulator) do anything weird like resource forks/shadow files? Or does it use a separate filesystem (or a host filesystem) for the profile?

Comment 2

[Jonathan Griffin (:jgriffin)]

To reproduce the problem:

to go objdir-gecko/_tests/testing/mochitest, and run the command:

python runtestsb2g.py --emulator arm --b2gpath /path/to/B2G/repo/ --xre-path /path/to/directory/containing/desktop/xpcshell

runtestsb2g.py copies a test profile to the device and then restarts B2G; you can see this error in adb logcat after the restart takes place.

Comment 3

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Just a quick update here: I'm stuck on not being able to get a working build.
https://github.com/mozilla-b2g/B2G/issues/56

Until I get that working, anyone have any idea whats going on with the filesystem?

Comment 4

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Ok, finally got a working build, and eventually got the tests running too - and I'm seeing the described error.

Comment 5

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Oh boy... so, AFAICT, we enumerate the staging directory, and when we get to what should be the last file, the directory enumerator gives us that last file again. So I believe this is bug 749661.

However, even if bug 749661 gets fixed today, I think processPendingFileChanges should generally be more resilient - since I believe resource forks on OSX can potentially cause a problem here too.

Comment 6

[[:fabrice] Fabrice Desré]

Without the yaffs2 patch indexedDB should also be broken in some use cases (this is how we found and fixed this fs bug on device). I guess the emulator is not patched.

Comment 7

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Would that explain the need to run "adb shell rm -rf /data/local/indexedDB" after starting up the emulator normally, to get things working?

Comment 8

[[:fabrice] Fabrice Desré]

Probably, yes - at least this is another clue in this direction.

Comment 9

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1

This basically carries on from bug 733436, applying the same principles (get a snapshot of dir entries, and detect non-existant file error) to processPendingFileChanges. So this not only works around bug 749661, but also fixes the OSX resource forks issue for that function. Contrary to bug 733436 comment 9, I think that was a possible issue here because the call to installAddon() uses move, not copy.

The other changes are just refactoring code to use a new convenience function, getDirectoryEntries().

Comment 10

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to Jonathan Griffin (:jgriffin) from comment #0)
> E/GeckoConsole(  392): [JavaScript Warning: "WARN addons.xpi: Ignoring
> directory whose name is not a valid add-on ID:
> /data/local/tests/profile/extensions/staged/[e-mail]"]

Jonathan: I haven't seen this occur yet, and I can't see how it could occur. Is it still an issue? If so, could you file a separate bug on it?

Comment 11

[Jonathan Griffin (:jgriffin)]

(In reply to Blair McBride (:Unfocused) from comment #10)
> (In reply to Jonathan Griffin (:jgriffin) from comment #0)
> > E/GeckoConsole(  392): [JavaScript Warning: "WARN addons.xpi: Ignoring
> > directory whose name is not a valid add-on ID:
> > /data/local/tests/profile/extensions/staged/[e-mail]"]
> 
> Jonathan: I haven't seen this occur yet, and I can't see how it could occur.
> Is it still an issue? If so, could you file a separate bug on it?

I haven't seen this in a while; if I see it again I'll file a new bug for it.

Comment 12

[Dave Townsend [:mossop]]

Comment on attachment 647906 [details] [diff] [review]
Patch v1

Review of attachment 647906 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/mozapps/extensions/XPIProvider.jsm
@@ +1930,5 @@
>        if (!stagingDir || !stagingDir.exists() || !stagingDir.isDirectory())
>          return;
>  
>        let seenFiles = [];
> +      let stagingDirEntries = getDirectoryEntries(stagingDir);

You have to sort the result of this to ensure the real file is first in the array, followed by any potential resource fork. Maybe it might be better to have getDirectoryEntries do the sorting too, though there is one place in this patch where sorting is unnecessary.

@@ +1941,5 @@
> +        }
> +        catch (e if e.result == Cr.NS_ERROR_FILE_TARGET_DOES_NOT_EXIST) {
> +          // If the file has already gone away then don't worry about it, this
> +          // can happen on OSX where the resource fork is automatically moved
> +          // with the data fork for the file. See bug 733436.

Probably should update the comment to reference this bug

Comment 13

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v2

Comment 14

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Review ping?

Comment 15

[Dave Townsend [:mossop]]

Comment on attachment 648615 [details] [diff] [review]
Patch v2

Review of attachment 648615 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/mozapps/extensions/XPIProvider.jsm
@@ +1942,5 @@
> +        }
> +        catch (e if e.result == Cr.NS_ERROR_FILE_TARGET_DOES_NOT_EXIST) {
> +          // If the file has already gone away then don't worry about it, this
> +          // can happen on OSX where the resource fork is automatically moved
> +          // with the data fork for the file. See bug 772238.

Can you also mention the yaffs problem here.

Comment 16

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Landed with 62% better comments:
https://hg.mozilla.org/integration/fx-team/rev/f360d7bf33dc

Comment 17

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/f360d7bf33dc