467747 - Add-On manager is broken, can't see add-ons

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Paul Black]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.1b3pre) Gecko/20081203 Shiretoko/3.1b3pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.1b3pre) Gecko/20081203 Shiretoko/3.1b3pre

Sometimes, the add-on manager shows no add-ons and searching doesn't work. The add-ons are still present and running because they have their various icons in the status bar.

Reproducible: Sometimes

Steps to Reproduce:
1. Start Firefox
2. Go to Tools->Add-ons

Actual Results:  
The window that opens doesn't show add-ons. It also has two extra icons across the top (languages and updates).

Expected Results:  
Normal add-ons Window with list of add-ons under extensions.

These two messages appear in the error console:

Error: missing ; before statement
Source File: file:///usr/local/firefox/modules/PluralForm.jsm
Line: 70
Source Code:
let gFunctions = [


Error: uncaught exception: [Exception... "Component returned failure code: 0x80004003 (NS_ERROR_INVALID_POINTER) [nsIPrefBranch2.addObserver]"  nsresult: "0x80004003 (NS_ERROR_INVALID_POINTER)"  location: "JS frame :: chrome://mozapps/content/extensions/extensions.js :: Startup :: line 989"  data: no]

Comment 1

[Paul Black]

Attachment: Screendump of strange add-ons Window.

Comment 2

[Paul Black]

Assuming it's more than just a coincidence, this error has started appearing when trying to download files:
Error: missing ; before statement
Source File: file:///usr/local/firefox/components/nsHelperAppDlg.js
Line: 109, Column: 6
Source Code:
      let prefs = Components.classes["@mozilla.org/preferences-service;1"]

Comment 3

[Dave Townsend [:mossop]]

This kind of reads like the JS engine is using the wrong JS version for parsing the scripts.

Is this an official build of Firefox? What extensions do you have installed?

Comment 4

[Dave Townsend [:mossop]]

Have you set javascript.options.jit.chrome to true?

Comment 5

[Paul Black]

These are standard Linux i686 nightly builds from ftp://ftp.mozilla.org/pub/firefox/nightly/*-mozilla-1.9.1
Extensions are:
noscript
foxmarks
better gmail 2
better greader
passwordmaker
new tab button
and nightly tester tools (to enable some of the above)

javascript.options.jit.chrome is false

I can have a stab tomorrow without extensions to see if the problem shows up.

I can't be 100% sure but I think this occurred with the 3.1b2 builds as well.

Comment 7

[Dave Townsend [:mossop]]

I'm going to push this over to the JS team, it doesn't seem restricted to the add-ons manager and they may have better ideas over why this might be happening.

Comment 8

[Jay Rossiter]

In my case jit.chrome _is_ true.  May or may not make a difference.

The only extensions I ahve in common with comment #5 are Nightly Tester Tools and Foxmarks.

Comment 9

[Jay Rossiter]

This also effects the RSS previewer.  Makes sense, since that's all Javascript.

Comment 10

[Brendan Eich [:brendan]]

Sounds like this is confirmed.

/be

Comment 11

[Jay Rossiter]

I thought that this had already been mentioned, but that was in my bug, not this one:

Along with RSS and Addons, the Check for Updates dialog is also busted when this occurs.

Another thing mentioned only in my bug was that launching in Safe Mode, and doing an updates check (for addons or builds) and then relaunching FF normally causes the problem to disappear for a period of time.  It generally only shows up again after an update (addon or build) is applied, but not /only/ then.

Comment 12

[Brendan Eich [:brendan]]

Could you try a tracemonkey hourly build, say the next one (if it passes automated tests)? Look for a 12/05 build soon under

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/

/be

Comment 13

[Jay Rossiter]

So, something from latest-tracemonkey (after the new build is posted)?  I've never used anything more granular than nightlies.

Comment 14

[Andreas Gal :gal]

If you can confirm with the nightly that will be produced tonight that would be a good start.

Comment 15

[Brendan Eich [:brendan]]

Jay: yeah, latest-tracemonkey -- I meant to include that in the link in comment 12, sorry.

/be

Comment 16

[Jay Rossiter]

I didn't see the issue while running the 2008-12-05-16-tracemonkey build, but it's not possible to force reproduction.  It happens sporadically.  However, _immediately_ upon applying the nightly build (Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b3pre) Gecko/20081206 Shiretoko/3.1b3pre), it was present.

It looks like the nightly reverted me to the 1.9.1 branch.  Not sure if that helps or not.

Comment 19

[Ben]

I have the same bug when using an existing profile with Firefox 3.1 Beta 2. If I create a new profile, by launching Firefox 3.1 Beta 2 with the -profilemanager, I do not have this issue. There appears to be something related to settings in older profiles.

Comment 20

[Ben]

Sorry for posting my last comment while still testing. I just launced Firefox 3.1 Beta 2 with my previously existing profile in (Safe Mode) and now when I launch it regularly with my existing profile I am able to open my add-ons without issue.

Comment 22

[(no longer active)]

I saw this on a clean install of Beta 2.  Where this happens, it can make major parts of the browsing defunct, bumping severity to critical.

Comment 23

[David Mandelin [:dmandelin]]

Could the problem relate to this code in jsinterp.cpp?

    currentVersion = (JSVersion) script->version;
    originalVersion = (JSVersion) cx->version;
    if (currentVersion != originalVersion)
        js_SetVersion(cx, currentVersion);

This is at the start of js_Interpret and ensures that the current version is set to the script's desired version, in case something else changed it. I didn't find any obvious place in the jstracer.cpp where this was implemented for tracing.

Comment 24

[(no longer active)]

FWIW, I did what Ben mentioned in comment 20 and it fixed the issue.  Before this, I was able to reproduce this with 3.1b2 and the latest 3.1b3pre and 3.2a1pre nightlies.  This was indeed profile related, because the same installation worked fine in other profiles.  The original profile was migrated from 3.0.

Comment 25

[Paul Black]

The fix from comment 20 is not a permanent fix. I use that fix but on one profile I have (created by 3.1b2) it recurs everytime I upgrade the nightly version. Another profile I have (both on Linux) does not exhibit this problem at all.

Comment 26

[Robert Sayre]

Waldo, could you take a look at this? Looks like something that definitely needs to be fixed for b3.

Comment 27

[Jeff Walden [:Waldo]]

Sure.

Comment 28

[Jeff Walden [:Waldo]]

I can't reproduce this with a debug build on OS X from a TM build as of perhaps 02:00 this morning; are others still reproducing this?

Comment 29

[Paul Black]

I've still been seeing this with the mozilla-1.9.1 overnight builds but it only occurs on 1 out for 4 installations I have. I have two Window and two Linux installations and it's one of the Linux ones that fails for me. I could recover with the fix in comment 20 but each time it was updated with the latest nightly it would fail again. I've just created a new profile on the machine that fails and will see if it still occurs.

Comment 30

[Jay Rossiter]

I had it happen with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b3pre) Gecko/20090105 Shiretoko/3.1b3pre on two machines.  One was the same machine it's always been happening on, the other was a brand new install (copied profile, however).

Comment 31

[Jeff Walden [:Waldo]]

Can't reproduce on a Windows debug build from this morning, either, with a clean profile.  I also installed NTT and Foxmarks to see if they changed it, no dice.  I flipped jit.chrome as well, still no dice.  Not sure where to go from here, to be honest...

Comment 32

[Jeff Walden [:Waldo]]

Could anyone who's reproducing this on Windows possibly zip up a profile and attach it here to see if that's what's making it hard for me to reproduce this?

Comment 33

[(no longer active)]

I'm working on creating a minimal profile to reproduce this.  Can I mail the profile to you at your bugzilla address (due to possible private data and the size of the compressed file)?

Comment 34

[Jeff Walden [:Waldo]]

Sure, but send it to me @mozilla.com, because my normal account is running low enough on space that a zipped profile could conceivably overflow that space.  I need to figure something out about that soon.  :-\

Comment 35

[Jeff Walden [:Waldo]]

Got the profile, intermittently reproducible, but hopefully that's enough to figure out the problem; doing so now...

Comment 36

[Damon Sicore (:damons)]

Jeff, as soon as you have an estimate on what it's going to take to resolve this, can you update?  Just trying to get a realistic estimate on beta 3 wrap up.

Comment 37

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

Similar to bug 439713?

Comment 38

[(no longer active)]

(In reply to comment #37)
> Similar to bug 439713?

I didn't follow the steps to reproduce for this bug personally, but it looks like this may be the same issue.

Comment 39

[Jeff Walden [:Waldo]]

To be clear, I can make no estimate of how long it'll take to resolve this right now.  I can make much greater progress in the other P1 I have right now, so I'm concentrating my efforts on that for the moment.

Comment 40

[Carsten Book [:Tomcat]]

so with the files i got from jeff, i was able to find a regression range:

Firefox
Version=3.1b2pre
BuildID=20081120120920
SourceRepository=http://hg.mozilla.org/mozilla-central
SourceStamp=f397d3057e84 works

and

Firefox
Version=3.1b2pre
BuildID=20081121034512
SourceRepository=http://hg.mozilla.org/mozilla-central
SourceStamp=62d26c7840a4 is broken

so we have there...
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f397d3057e84&tochange=62d26c7840a4

Comment 41

[Andreas Gal :gal]

Nothing in that range sticks out. Could you try to reduce this further?

Comment 42

[Jeff Walden [:Waldo]]

I'm betting it's bent's checkin, seems like the only thing that might munge with versions; if it's not that, I have no idea.

Comment 43

[Carsten Book [:Tomcat]]

(In reply to comment #42)
> I'm betting it's bent's checkin, seems like the only thing that might munge
> with versions; if it's not that, I have no idea.

will test this and will report back !

Comment 44

[Carsten Book [:Tomcat]]

(In reply to comment #42)
> I'm betting it's bent's checkin, seems like the only thing that might munge
> with versions; if it's not that, I have no idea.

yeah seems this is a regression of bent's patch. 

I have build testbuilds with <http://hg.mozilla.org/mozilla-central/rev/f93edfbd082e> and <http://hg.mozilla.org/mozilla-central/rev/13ebc220fad3>

f93edfbd082e is fine and 13ebc220fad3 fails

Comment 45

[Helmut Leininger]

I cannot prove it but it seems to happen more often when the FireProxy (2.8.11) Add-On is installed and activated. At least, for me it never occurred when I had deactivated Fireproxy.

Comment 46

[Jeff Walden [:Waldo]]

Comment 45 is accurate (modulo s/FireProxy/FoxyProxy/) from what I've deciphered so far in the last several hours using the profile that was sent to me -- finally hit on a reasonable way to debug this.

Specifically, I added a bunch of NS_ABORT_IF_FALSE (not NS_ASSERTION, natch) calls to various locations in mozJSComponentLoader.cpp where mContext was being used.  I then added tracepoints on all of them to print out the module being loaded and breakpoints on all the assertions to trigger when a bad version was detected (whatever code does aborts on Windows doesn't fall into the debugger, boo hiss).  From this I could figure out the usual sequence of module imports; then I set a conditional breakpoint to hit when loading one of the modules before the assertion fired, which got me within spitting distance of the step astray, so a watchpoint set on the location of the version wouldn't fire an insane number of times.  A few more continues past a series of XPCOMUtils.jsm imports got me to the actual version change that's probably the one not being reset correctly.

Anyway, regarding the actual problem...

The URL classifier downloads updates, which goes through a FoxyProxy proxy filter, which eventually notifies observers for the "foxyproxy-throb" topic.  The one observer for this topic is a function in a FoxyProxy overlay, and the JS version associated with that script is JSVERSION_DEFAULT, presumably because the overlay is loaded with that version.  Preparation to call the function sets the version on the JS module loader's context to not allow most recent JS language syntax features; this should be okay because the version should be reset when the function's execution completes (and any sub-function calls should themselves include their own version sets and unsets).

And that's where I am right now.  Definitely a whole lot further along than at any time in the past, tho, so this should be history Real Soon Now...

Comment 47

[Geoffrey G Wright]

I had FoxyProxy installed when I encountered this bug.

Comment 48

[Jeff Walden [:Waldo]]

Attachment: js_SetVersion must sync to cx->options

mozJSComponentLoader::mContext, shortly after creation, has its options set to 0x40 (JSOPTION_XML) and its version set to 0x10b4 (JSVERSION_LATEST).

We call back into a function defined in an overlay whose script->version is JSVERSION_DEFAULT; in preparation we effectively call js_SetVersion(cx, script->version) now.  If you look at that method, it sets cx->version to script->version and does nothing else beyond beyond effect-free debug assertions.

So now we have cx->version == 0x0 and cx->options == 0x40, and now we call down into a QueryInterface.

The JS_SetOptions(cx, JS_GetOptions(cx) | JSOPTION_DONT_REPORT_UNCAUGHT) modified in the fingered checkin sets cx->options == 0x140 (J_D_R_U == 0x100) -- but it also syncs the XML bit (0x40) in 0x140 over to the XML bit in cx->version, setting cx->version == 0x1000.

Now we call the function and return.

The reverting JS_SetOptions(cx, oldopts) has oldopts == 0x40, which sets cx->options == 0x40, its previous value.  Now it syncs the XML bit (0x40) over to the XML bit in cx->version -- but since that bit is 1, cx->version == 0x1000, which is a change from before the first JS_SetOptions call.

We go astray when the XML bits in options and in version become unsynced, which is when js_SetVersion is called.  If those bits aren't in sync, you start losing or gaining XML support.  The JS engine preserves versions until a version change (to the full cx->version value, not just to the JSVERSION_MASK part) is detected, and then it stops changing the version entirely -- the new version is "sticky", and next time an import of a not-just-default+XML version happens, syntax errors will happen.  The solution's simple once it's all laid out this way: js_SetVersion needs the sync-xml-bit logic found in JS_SetOptions.

I'm pretty sure this patch works, but it'll be an hour or two for my Windows build to finish; it's got this patch plus a thousand new revisions to build through.

Comment 49

[Brendan Eich [:brendan]]

Comment on attachment 361070 [details] [diff] [review]
js_SetVersion must sync to cx->options

Could you comment this mentioning SYNC_OPTIONS_TO_VERSION over in jsapi.cpp? Bonus would be to make that a helper function defined here in jscntxt.cpp, exported for use by jsapi.cpp's JS_SetOptions and JS_ToggleOptions. It doesn't need to be inlined. It should live next to js_SetVersion. js_SyncOptionsToVersion would be the transliterated name, but it's not symmetric with js_SetVersion. To have a SyncVersionToOptions peer (static inline this time), called by js_SetVersion, would balance things nicely.

/be

Comment 50

[Jay Rossiter]

++FoxyProxy;

I also have this installed and experience the problem frequently.

Comment 51

[Jeff Walden [:Waldo]]

Attachment: Just what the doctor ordered

The previous patch seemed to be good on the test profile I was provided; currently rebuilding and testing with this one on Windows just to be sure I didn't mess anything up, but we can race that test against this review as mistakes seem unlikely...

Comment 52

[Brendan Eich [:brendan]]

Comment on attachment 361122 [details] [diff] [review]
Just what the doctor ordered

>+/*
>+ * JSOPTION_XML and JSOPTION_ANONFUNFIX must be part of the JS version
>+ * associated with scripts, so in addition to storing them in cx->options we
>+ * duplicate them in cx->version (script->version, etc.) and ensure each bit
>+ * remains synchronized between the two through these two functions.
>+ */
>+

Nit: no need for blank line here even though the comment addresses both functions.

> /*
>+ * Ensures the JSOPTION_XML and JSOPTION_ANONFUNFIX bits of cx->options are
>+ * correspondingly set in cx->version, since each bit must travel with a
>+ * JSScript that has it set.

s/correspondingly set/reflected in/
s/JSScript/script/

r=me with nits picked, thanks.

/be

Comment 53

[Jeff Walden [:Waldo]]

In TM:

http://hg.mozilla.org/tracemonkey/rev/d2460761a5a4

Comment 54

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

Comment on attachment 361122 [details] [diff] [review]
Just what the doctor ordered

>+inline void
>+js_SyncVersionToOptions(JSContext* cx)
>+{
>+    if (cx->version & JSVERSION_HAS_XML)
>+        cx->options |= JSOPTION_XML;
>+    else
>+        cx->options &= ~JSOPTION_XML;
>+    if (cx->version & JSVERSION_ANONFUNFIX)
>+        cx->options |= JSOPTION_ANONFUNFIX;
>+    else
>+        cx->options &= ~JSOPTION_ANONFUNFIX;
>+}

Jeff, just a small question. Why is this function inline while js_SyncOptionsToVersion is not?

Comment 55

[Rob Campbell [:rc] (:robcee)]

+1 vote for this. I've seen this a couple of times over the weekend and it's taking down add-ons, download manager and bookmarks.

the "missing ; before statment" error message isn't exactly helpful. I was wondering if error parsing was broken when looking for missing semi-colons.

e.g., Error: missing ; before statement
Source File: file:///Users/rob/Applications/Shiretoko.app/Contents/MacOS/modules/PluralForm.jsm
Line: 70
Source Code:
let gFunctions = [

Comment 56

[Jeff Walden [:Waldo]]

(In reply to comment #54)
> Jeff, just a small question. Why is this function inline while
> js_SyncOptionsToVersion is not?

Well, the latter's callsites aren't in code that needs to be performant.  The former conceivably could be, although it probably isn't.  Realistically, tho, adding inline to a method declaration is just sprinkling magical pixie dust, because the compiler is free to ignore it if it wants.

This'll get picked up in an upcoming TM merge, I'm sure.

Comment 57

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/d2460761a5a4

Comment 58

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

When this will be checked into trunk? Do we need a bit of baking for the latest tracemonkey fixes?

Comment 59

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

Sorry, I meant "checked into 1.9.1 branch".

Comment 60

[Jay Rossiter]

It's unclear from the comments whether this has made it into 1.9.1 yet, or not.  If it has, it's certainly not fixed.  I've run into it four times today alone.

Comment 61

[Nochum Sossonko [:Natch]]

This has been fixed on trunk (latest development) but *not* on the 1.9.1 branch. When it is checked in to the 1.9.1 branch the keyword "fixed1.9.1" will be added.

Comment 62

[(no longer active)]

Jeff, should I just land this on 1.9.1, or will it be done in the next TM merge?

Comment 63

[Jeff Walden [:Waldo]]

Next merge.

Comment 64

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/5e69859829fe

Comment 65

[Bob Clary [:bc] (inactive)]

tomcat, is this something you can develop a reduced testcase for?