Closed Bug 465998 Opened 11 years ago Closed 11 years ago

Extension crashes FF3.1b2pre on startup

Categories

(Firefox :: General, defect, critical)

x86
Windows XP
defect
Not set
critical

Tracking

()

VERIFIED FIXED

People

(Reporter: johnjbarton, Assigned: neil)

References

Details

(Keywords: crash, testcase, verified1.9.1, Whiteboard: )

Attachments

(2 files, 1 obsolete file)

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
Attached file extension that crashes
off by one level of directory, try 2
Attachment #349236 - Attachment is obsolete: true
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081120 Minefield/3.1b2pre
Blocks: 448602
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.
This crash prevents my progress on Firebug 1.4 for Firefox 3.1 because it blocks chromebug on 3.1
Whiteboard: [firebug-p1]
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) {
(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.
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?
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
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
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
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?
(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?
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.
(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.
Should the pref service perhaps refuse to allow prefs to be set at that point?  That would make a lot more sense to me...
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!
No longer blocks: 448602, 465993
Whiteboard: [firebug-p1] →
(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.
Attached patch Fix the crashSplinter Review
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #350225 - Flags: superreview?(jonas)
Attachment #350225 - Flags: review?(enndeakin)
Keywords: crash, testcase
Attachment #350225 - Flags: review?(enndeakin) → review+
Attachment #350225 - Flags: superreview?(jonas) → superreview+
Pushed changeset 9ae606f28363 to mozilla-central.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Attachment #350225 - Flags: approval1.9.1?
Comment on attachment 350225 [details] [diff] [review]
Fix the crash

Crash fix has baked on trunk.
Comment on attachment 350225 [details] [diff] [review]
Fix the crash

a191=beltzner
Attachment #350225 - Flags: approval1.9.1? → approval1.9.1+
Pushed changeset 92f4915946aa to releases/mozilla-1.9.1
Keywords: fixed1.9.1
Depends on: 471638
(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].
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.