465998 - Extension crashes FF3.1b2pre on startup

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[John J. Barton]

I tried to create a small extension test case for bug 465993. The extension crashes FF3.1b2pre before it shows up.

I'll attach the xpi

Comment 1

[John J. Barton]

Attachment: extension that crashes me in FF3.1b2pre

Comment 2

[John J. Barton]

Attachment: extension that crashes

off by one level of directory, try 2

Comment 3

[John J. Barton]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081120 Minefield/3.1b2pre

Comment 4

[John J. Barton]

verified crash with attached xpi
http://crash-stats.mozilla.com/report/pending/6fce10b6-7a81-416a-b539-411020081120

Comment 5

[John J. Barton]

The stack trace is the same one I get on my builds of FF3.

The crashing line is
rv = gRDF->GetResource(uri, aResult);
gRDF is zero.

gRDF is initialized in nsXULContentUtils::Init() if and only if gRefCnt++ == 0, but gRefCnt is a large integer value. 

A possible fix to the crash would initialize gRefCnt to zero:
nsrefcnt nsXULContentUtils::gRefCnt = 0;

I cannot test this idea because I cannot get the build to work if I make this change.

The change does not answer the question of why the crash does not happen all of the time.

Comment 6

[John J. Barton]

This crash prevents my progress on Firebug 1.4 for Firefox 3.1 because it blocks chromebug on 3.1

Comment 7

[timeless]

http://msdn.microsoft.com/en-us/library/s1sb61xd.aspx
      When you declare a variable or function at file scope (global and/or namespace scope), the static keyword specifies that the variable or function has internal linkage. When you declare a variable, the variable has static duration and the compiler initializes it to 0 unless you specify another value. 

http://mxr.mozilla.org/mozilla-central/search?string=RefCnt&find=xul&filter=nsXULContentUtils

/content/xul/templates/src/nsXULContentUtils.h
    * line 67 -- static nsrefcnt gRefCnt;
/content/xul/templates/src/nsXULContentUtils.cpp
    * line 97 -- nsrefcnt nsXULContentUtils::gRefCnt;
    * line 115 -- if (gRefCnt++ == 0) {
    * line 150 -- if (--gRefCnt == 0) {

Comment 8

[John J. Barton]

(In reply to comment #7)
> duration and the compiler initializes it to 0 unless you specify another value. 

So we have to assume that memory is overwritten.  That's not good.

Comment 9

[John J. Barton]

At the crash point in my trunk build the value in gRefCnt is 		
gRefCnt	0xffffffff	unsigned long

But I get here in a weird way: start firefox.exe with visual studio, it runs, exits, then I get a dialog allowing debug, and another dialog from Windows and finally back to visual studio where we are halted on the exception.  Its like there are two processes, one that exits then the next crashes?

Comment 10

[John J. Barton]

The two process thing must be the xpcom registration bit. Subsequent runs are one process. 
gRefCnt starts at 0,  in >	gklayout.dll!nsLayoutStatics::Initialize()  Line 189	C++

then 0xfffffffe,  in	gklayout.dll!nsXULContentUtils::Init()  Line 115	C++

then 0xffffffff. at the crash

Comment 11

[John J. Barton]

gRefCnt is zero at gklayout.dll!nsLayoutStatics::Initialize()  Line 196	C++
which is :
  rv = nsXULContentUtils::Init();
when I step into  nsXULContentUtils::Init(); the value is 
		gRefCnt	0xfffffffe	unsigned long

Comment 12

[[e-mail]]

OK, so it looks like this extension sets some caps prefs. That doesn't sound too bad in itself, but the following chain of events is somewhat unfortunate:
* XPCOM tries to start up
* It asks the JS component loader to load a component
* The JS component loader asks the script secman for the system principal
* The script security manager loads the caps prefs
* A principal tries to create a URI
* The HTTP handler tries to loads the intl.accept_languages localised pref
* The pref service tries to open chrome://global/locale/intl.properties
* The chrome protocol handler tries to create the XUL prototype cache
* This tries to initialise layout
* Layout tries to load the script secman, but fails due to recursive GS
This then happens again for chrome://global-platform/locale/intl.properties for the intl.charset.default complex preference.
Each time, layout then tries to shut down. However, nsXULContentUtils unconditionally decrements its reference count, so its reference count is now -2U at this point, so it never inits gRDF when its Init finally gets called. nsXULContentUtils seems to be the only call with this issue.
Other assertions:
* Script secman tries to clear prefs during a pref observe callback

Comment 13

[Boris Zbarsky [:bzbarsky]]

So what's different in this case from the case of CAPS prefs just being set in the user's profile?

It sounds like nsXULContentUtils just needs fixing, no?

Comment 14

[John J. Barton]

(In reply to comment #12)
> OK, so it looks like this extension sets some caps prefs. 

Does that mean there is some workaround? What is a 'caps' pref?

Comment 15

[Boris Zbarsky [:bzbarsky]]

CAPS is the capability policy system in Gecko's script security manager.  The preferences in question are the "capability.principal.*" prefs in the extension's test.js.

Comment 16

[[e-mail]]

(In reply to comment #13)
> So what's different in this case from the case of CAPS prefs just being set in
> the user's profile?
Profile startup hasn't happened yet.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Should the pref service perhaps refuse to allow prefs to be set at that point?  That would make a lot more sense to me...

Comment 18

[John J. Barton]

Removing the five calls to user_pref("capability.principal... in the test extension's default test.js file allows the browser to start without crashing, thanks!

Comment 19

[[e-mail]]

(In reply to comment #17)
> Should the pref service perhaps refuse to allow prefs to be set at that point? 
It's always started up with the default prefs when a profile hasn't loaded yet. Note that this includes starting with profile manager or migration.

Comment 20

[[e-mail]]

Attachment: Fix the crash

Comment 21

[[e-mail]]

Pushed changeset 9ae606f28363 to mozilla-central.

Comment 22

[[e-mail]]

Comment on attachment 350225 [details] [diff] [review]
Fix the crash

Crash fix has baked on trunk.

Comment 23

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 350225 [details] [diff] [review]
Fix the crash

a191=beltzner

Comment 24

[[e-mail]]

Pushed changeset 92f4915946aa to releases/mozilla-1.9.1

Comment 25

[[e-mail]]

(In reply to comment #12)
> * The chrome protocol handler tries to create the XUL prototype cache
This step would additionally be fixed by attachment 358702 [details] [diff] [review].

Comment 26

[Aakash Desai [:aakashd]]

verified FIXED on builds: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090422 Minefield/3.6a1pre ID:20090422044118

and 

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b4pre) Gecko/20090422 Shiretoko/3.5b4pre ID:20090422042031