Closed Bug 494186 Opened 15 years ago Closed 15 years ago

Stockticker breaks Firefox since upvar2 patch has landed

Categories

(Core :: Preferences: Backend, defect)

defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 494946

People

(Reporter: micha.postbox, Unassigned)

Details

(Keywords: regression, testcase, Whiteboard: [explained in comment 31 and comment 35])

Attachments

(3 files, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090521 Shiretoko/3.5pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090521 Shiretoko/3.5pre

When Stockticker is installed you can't open or close tabs, addonmanager has the scrollbar on left side, scroll with mouse in addon manager doesnt work, options window is empty...

Reproducible: Always

Steps to Reproduce:
1. Create a clean profile.
2. Install Stockticker 1.04 or 1.05 and restart
3. go to js console - no error
4. go to addon manager and then you get the following messages (possible you need 2 or 3 trys before fx break)

Error: formatURLPref: Couldn't get pref: extensions.getMoreExtensionsURL
Source File: file:///C:/Programme/Mozilla_Firefox31/components/nsURLFormatter.js
Line: 68

Error: uncaught exception: [Exception... "Component returned failure code: 0xc1f30001 (NS_ERROR_NOT_INITIALIZED) [nsIPrefBranch2.getBoolPref]"  nsresult: "0xc1f30001 (NS_ERROR_NOT_INITIALIZED)"  location: "JS frame :: chrome://mozapps/content/extensions/extensions.js :: updateGlobalCommands :: line 2187"  data: no]

6. Addonmanager scrollbar on left side, no mouse scrolling

7. go to options window - its empty, messages:

Error: uncaught exception: [Exception... "Component returned failure code: 0xc1f30001 (NS_ERROR_NOT_INITIALIZED) [nsIPrefBranch.getBoolPref]"  nsresult: "0xc1f30001 (NS_ERROR_NOT_INITIALIZED)"  location: "JS frame :: chrome://global/content/bindings/preferences.xml ::  :: line 576"  data: no]

8. Impossible to close/open tabs.

9. several extensions doesnt work



When Stockticker is installed and Fx was compiled with changeset 24809:b2d7dbeb0f1b its ok, but compiled with changeset 24812:79606200f871 the extension breaks the entire firefox.
Just for clarity, b2d7dbeb0f1b is a changeset only present on the 1.9.1 branch.  The pushlog between those two changesets is:

http://hg.mozilla.org/releases/mozilla-1.9.1/pushloghtml?fromchange=b2d7dbeb0f1b&tochange=79606200f871

I'd be interested in the equivalent range from m-c, or better yet tracemonkey...  All that range tells me on branch is that we had a TM merge.

I'd also be interested in an actual link to the extension in question, just to make sure that the right thing is being tested.
Assignee: nobody → general
Component: General → JavaScript Engine
Product: Firefox → Core
QA Contact: general → general
Flags: blocking1.9.1?
Tested the addon StockTicker 1.04 from amo (https://addons.mozilla.org/en-US/firefox/downloads/latest/183/addon-183-latest.xpi) on i686 linux:

Testing mozilla-central:

good: Gecko/20090406 (http://hg.mozilla.org/mozilla-central/rev/b14428284d51)
bad:  Gecko/20090407 (http://hg.mozilla.org/mozilla-central/rev/f0507c4d0abb)

Testing tracemonkey:

good: http://hg.mozilla.org/tracemonkey/rev/7d681f116714
bad:  http://hg.mozilla.org/tracemonkey/rev/0f7ab38ff862

which does point at upvar2.

To test I installed the addon in a clean profile (reused with all builds, ignoring some places breakage, and with extensions.checkCompatibility off), then opened the error console and addon manager (in any order), then repeatedly closed and opened the addon manager (extensions tab). Opening less than half a dozen times triggers this on the error console on closing the addon manager:

Error: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIPrefBranch2.removeObserver]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://mozapps/content/extensions/extensions.js :: Shutdown :: line 1140"  data: no]

The next open triggers:

Error: formatURLPref: Couldn't get pref: extensions.getMoreExtensionsURL
Source File: file:///home/marienz/tmp/firefox-2009-04-07-03-tracemonkey/components/nsURLFormatter.js
Line: 68

Error: uncaught exception: [Exception... "Component returned failure code: 0xc1f30001 (NS_ERROR_NOT_INITIALIZED) [nsIPrefBranch2.getBoolPref]"  nsresult: "0xc1f30001 (NS_ERROR_NOT_INITIALIZED)"  location: "JS frame :: chrome://mozapps/content/extensions/extensions.js :: updateGlobalCommands :: line 2176"  data: no]

and after that the main firefox preferences window is blank. I did not check the other breakage mentioned in comment #0, just watched the error console and confirmed the pref window is blank.
ok, mzz can reproduce this on Linux, with all three of 1.9.1, m-c, t-m.

The ranges he found were:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b14428284d51&tochange=f0507c4d0abb
http://hg.mozilla.org/tracemonkey/pushloghtml?fromchange=7d681f116714&tochange=0f7ab38ff862

So looks like this is an upvar2 regression.
Blocks: 452598
Status: UNCONFIRMED → NEW
Ever confirmed: true
Yes, its the 1.9.1 Branch. The headline of changeset 79606200f871 is Brendan Eich — upvar2, aka the big one take 2 (452498, r=mrbkap).

http://mozmonkey.com/packages/stockticker/stockticker.xpi
Can a testcase (however reduced) be extracted from the add-on?

/be
Assignee: general → brendan
Status: NEW → ASSIGNED
OS: Windows XP → All
Priority: -- → P1
Hardware: x86 → All
Target Milestone: --- → mozilla1.9.1
Version: unspecified → Trunk
Flags: blocking1.9.1? → blocking1.9.1+
Attached file reduced testcase (obsolete) —
I can reproduce with this testcase in the latest 1.9.1 nightly and 3.0.10. Relevant line of code:

var prefBranch = Components.classes["@mozilla.org/preferences-service;1"].createInstance(Components.interfaces.nsIPrefBranch2);
What do I set via about:config to get the add-on to install? It doesn't like 3.6a1pre...

/be
Confirm testcase on 1.9.1.
(In reply to comment #7)
> What do I set via about:config to get the add-on to install? It doesn't like
> 3.6a1pre...

Brendan, you should install the nightly tester tools from dave. See https://addons.mozilla.org/en-US/firefox/addon/6543. That way you can override the compatibility. Otherwise setting extensions.checkCompatibility to false could help.

I'll have a look at it right now.
That's happening because the init function of this test extension calls createInstance instead of getService on the nsIPrefBranch2 interface. Changing it to getService everything is fine.

I'll check for check-ins on the given date.
Summary: Addon Stockticker breaks firefox since 20090418 → Extensions which are using createInstance instead of getService break Firefox
Oh, I missed that the regression window is already found. But the referenced bug on the dependency list was wrong. Updating it to the upvar2 bug.
Blocks: upvar2
No longer blocks: 452598
This is a more reduced testcase which needs chrome privileges to run. No need to install an extension anymore.

Steps:
1. Open and run the testcase
2. Reload the page

After step 2 you can see the same behavior and errors as described above.
Attachment #379319 - Attachment is obsolete: true
Updating the summary to reflect the latest investigations. I think that's all what I can do here for now.
Summary: Extensions which are using createInstance instead of getService break Firefox → Using createInstance to instantiate a XPCOM service breaks Firefox
What has this bug to do with upvar2, then? Is it possible the extension happened to switch to createInstance at the same time? Or did we finally outlaw calling of createInstance on a service at the same time?

I see no upvars at all in the reduced testcase, so removing the dependency.

/be
No longer blocks: upvar2
Stockticker was last packed at July 2008 and works since years. This bug came with bugfix 452498 and this was before 4 weeks. When you did outlaw calling of createInstance on a service, we have likely some more extensions which need an update. If say there is no problem with 452498 we have likely more extensions which breaks the fx...
This was an XPCOM change? Or XPConnect? Trying XPCOM first.

/be
Assignee: brendan → nobody
Component: JavaScript Engine → XPCOM
QA Contact: general → xpcom
so, at first I thought there was no way these could be upvar2 problems. But then I noticed that the error messages in the console were from sessionStore and other things trying to access the pref service. Maybe something isn't falling out of scope that should be?
Could be two bugs -- clone this bug if necessary.

What were those sesionStore error messages, and implicated code? Thanks,

/be
SessionStore: [Exception... "Component returned failure code: 0xc1f30001 (NS_ERROR_NOT_INITIALIZED) [nsIPrefBranch.getIntPref]"  nsresult: "0xc1f30001 (NS_ERROR_NOT_INITIALIZED)"  location: "JS frame :: file:///C:/Programme/Mozilla_Firefox31/components/nsSessionStore.js :: sss_serializeHistoryEntry :: line 1181"  data: no]

Error: formatURLPref: Couldn't get pref: extensions.getAddons.recommended.browseURL
Source file: file:///C:/Programme/Mozilla_Firefox31/components/nsURLFormatter.js
Line: 68

Error: uncaught exception: unknown (can't convert to string)

Error: Component returned failure code: 0xc1f30001 (NS_ERROR_NOT_INITIALIZED) [nsIPrefBranch.getIntPref]
Source file: file:///C:/Programme/Mozilla_Firefox31/components/nsSessionStore.js
Line: 2541

Error: Component returned failure code: 0x8007000e (NS_ERROR_OUT_OF_MEMORY) [nsIPrefBranch2.setIntPref]
Source file: file:///C:/Programme/Mozilla_Firefox31/components/nsUpdateService.js
Line: 2878

Error: Component returned failure code: 0x8007000e (NS_ERROR_OUT_OF_MEMORY) [nsIPrefBranch.setCharPref]
Source file: file:///D:/Backup/Mozilla/Firefox/Firefox31/extensions/%7B73a6fe31-595d-460b-a920-fcc0f8843232%7D/components/noscriptService.js
Line: 1825
This testcase reproduces the issue in the latest 1.9.1 but not in 3.0.10 (as opposed to the first testcase which reproduces it for both).
Attached file Simple testcase
This testcase reproduces the issue without having to do the add-on window stuff. It forces a garbage collection and then tries to read a pref. Fails in latest 1.9.1 and 3.0.10.
I don't that this is a regression at all. I can go back to Firefox 2.0.0.20 which is also affected and even crashes when opening the Addons Manager after the test file has been loaded.

I'll check with the mentioned Addon and the given time frame if there is a difference.
Brendan, the regression which is specific to the addon is really a separate issue. After cloning the tracemonkey branch and building own builds I can confirm the above regression range and that http://hg.mozilla.org/tracemonkey/rev/2cf0bbe3772a caused this problem.

How shall we proceed? Filing the above regression as a new bug since we morphed this one a lot? I would think so.
calling Components.classes['@mozilla.org/preferences-service;1'].createInstance(Components.interfaces.nsIPrefBranch) is *always wrong* (you should be using .getService instead). It has been wrong since the beginning of time. It's not clear how upvar2 made this worse, but I don't see how this is a blocker.
(In reply to comment #23)
> Brendan, the regression which is specific to the addon is really a separate
> issue. After cloning the tracemonkey branch and building own builds I can
> confirm the above regression range and that
> http://hg.mozilla.org/tracemonkey/rev/2cf0bbe3772a caused this problem.

Any hope of a reduced testcase? Or at least an HTML one?

> How shall we proceed? Filing the above regression as a new bug since we morphed
> this one a lot? I would think so.

Either that, or morph this back -- shouldn't have morphed it, in hindsight.

/be
This testcase demonstrates the "regression" caused by upvar2.

Good: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090406 Minefield/3.6a1pre
Bad: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090407 Minefield/3.6a1pre

Removing "load;" from prefObserver.observe causes this to fail for builds before upvar2.
Attachment #379368 - Attachment is obsolete: true
Ok, sounds like something really is weird here, per comment 26, and that it has to do with upvar2.  Back to js.
Assignee: nobody → general
Component: XPCOM → JavaScript Engine
QA Contact: xpcom → general
Restoring summary.
Summary: Using createInstance to instantiate a XPCOM service breaks Firefox → Stockticker breaks Firefox since upvar2 patch has landed
I wonder if removing the reference to the enclosing function is causing us to pass a null/flat/strange/charmed closure to XPConnect.  But I know basically nothing about the upvar analysis, so maybe use as a call argument already ensures that we don't do anything that would be broken by being called back on a different context or after the enclosing frame has died or whatever....
Assignee: general → jorendorff
I don't understand that test case: it sets an observer on "this.pref.does.not.exist" and then _gets_ a _different_ pref, and we expect what to happen?
Here's my theory.

1.  This code creates an nsPrefBranch.  You're not supposed to do that, by the way.

2.  Then the nsPrefBranch gets GC'd, which destroys the global hash table of prefs (gHashTable in prefapi.cpp) and causes all subsequent pref accesses to fail with NS_ERROR_NOT_INITIALIZED.

So two questions: why did this work before? and can we do something to mitigate the pain?

It worked before upvar2 because the lambda in the observer kept all enclosing scopes alive, which prevented the first nsPrefBranch from getting GC'd. This was arguably a bug, as the nsPrefBranch really does become unreachable when the function exits, and it should be GC'd.  upvar2 simply fixed that bug!

sdwilsh suggested mitigating this by making the appropriate objects singletons.
Yeah, I agree that it's a bug in the extension, but it sucks that we're going to be changing that behaviour after the add-on compat beta promise.  Pref observers are quite common, so I wouldn't be surprised if this affects a lot of add-ons.

(I don't quite follow why destroying a single prefBranch causes all future pref accesses to fail that way, but that code is...imperfect anyway.)

Makes this a non-blocker, though, IMO.
Flags: blocking1.9.1+ → blocking1.9.1-
Un-assigning.  This seems to be a case of bug 494946.
Assignee: jorendorff → general
Depends on: 494946
Assignee: general → nobody
Component: JavaScript Engine → Preferences: Backend
Priority: P1 → P5
QA Contact: general → prefs
So this bug is more a Wontfix?

I have contacted the author of the extension so it will hopefully be fixed for the final release or RC.
Yes this bug is basically WONTFIX except for the generic long-term fix for bug 494946
QA Contact: preferences → preferences-backend
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
No longer depends on: 494946
Priority: P5 → --
Resolution: --- → DUPLICATE
Whiteboard: [explained in comment 31 and comment 35]
Target Milestone: mozilla1.9.1 → ---
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: