248071 - FF09 [@ nsXPCWrappedJSClass::GetInterfaceName] [@

Status: RESOLVED FIXED

(Core Graveyard :: RDF, defect)

Description

[Matthew Mastracci]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7) Gecko/20040614 Firefox/0.8
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7) Gecko/20040614 Firefox/0.8

For a short period of time, Firefox crashed on startup after installing *any*
extension or theme.  I captured five different talkback traces with different
sets of extensions, each with the same stack trace:



Reproducible: Couldn't Reproduce
Steps to Reproduce:
1.  Install extension
2.  Restart

Actual Results:  
Firefox crashes

Expected Results:  
Firefox does not crash

The talkback entries I sent are:
TB112046Y, TB111706Y, TB111645Y, TB111584Z, TB111563K

Please note that the crashes only happened during the period of a day or so. 
Also, clearing the profile worked only until the first theme or extension was
installed.  Starting Firefox in safe mode also avoided the crash.  I cannot get
Firefox to crash any more, so it might be related to an unavailable internet
resource.

Here's one of the stack traces:

http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB111563K

Stack signature:  0x8b2775c8 b6cd060c
Product ID	Firefox10
Build ID	2004061423

nsXPCWrappedJSClass::GetInterfaceName 
[c:/builds/tinderbox/firefox_branch/WINNT_5.0_Clobber/mozilla/js/src/xpconnect/src/xpcwrappedjsclass.cpp,
line 1592]
nsXPCWrappedJS::CallMethod 
[c:/builds/tinderbox/firefox_branch/WINNT_5.0_Clobber/mozilla/js/src/xpconnect/src/xpcwrappedjs.cpp,
line 450]
SharedStub 
[c:/builds/tinderbox/firefox_branch/WINNT_5.0_Clobber/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp,
line 147]
RDFXMLDataSourceImpl::EndLoad 
[c:/builds/tinderbox/firefox_branch/WINNT_5.0_Clobber/mozilla/rdf/base/src/nsRDFXMLDataSource.cpp,
line 1059]
RDFContentSinkImpl::DidBuildModel 
[c:/builds/tinderbox/firefox_branch/WINNT_5.0_Clobber/mozilla/rdf/base/src/nsRDFContentSink.cpp,
line 670]
CNavDTD::DidBuildModel 
[c:/builds/tinderbox/firefox_branch/WINNT_5.0_Clobber/mozilla/htmlparser/src/CNavDTD.cpp,
line 707]
nsParser::DidBuildModel 
[c:/builds/tinderbox/firefox_branch/WINNT_5.0_Clobber/mozilla/htmlparser/src/nsParser.cpp,
line 1245]

Comment 1

[José Jeria]

You seem to have installed 0.9 over 0.8

Can you uninstall 0.9 and 0.8 (in that order) and try with a new profile?

Also, see if bug 247284, comment 16 helps you.

Comment 2

[Matthew Mastracci]

The problem no longer occurs for me - it only happened for a number of hours and
then went away (without having changed anything!).

It might be related to bug 247284, comment 21 since it only happened for a short
period of time.

I did install 0.9 on top of 0.8, but it seems to be working fine in this
configuration now.  Perhaps the bug is when update.rdf is empty and 0.9 is
installed on 0.8.  :)

Comment 3

[Jay Patel [:jay]]

This is the #2 topcrash for Firefox 0.9.  And also appears to be showing up
under the 0x8b2775c8 stack signature:

0x8b2775c8  
	 nsXPCWrappedJSClass::GetInterfaceName
[c:/builds/tinderbox/firefox_branch/WINNT_5.0_Clobber/mozilla/js/src/xpconnect/src/xpcwrappedjsclass.cpp
 line 1592] 
	 nsXPCWrappedJS::CallMethod
[c:/builds/tinderbox/firefox_branch/WINNT_5.0_Clobber/mozilla/js/src/xpconnect/src/xpcwrappedjs.cpp
 line 450] 
	 SharedStub
[c:/builds/tinderbox/firefox_branch/WINNT_5.0_Clobber/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp
 line 147] 
	 RDFXMLDataSourceImpl::EndLoad
[c:/builds/tinderbox/firefox_branch/WINNT_5.0_Clobber/mozilla/rdf/base/src/nsRDFXMLDataSource.cpp
 line 1059] 
	 RDFContentSinkImpl::DidBuildModel
[c:/builds/tinderbox/firefox_branch/WINNT_5.0_Clobber/mozilla/rdf/base/src/nsRDFContentSink.cpp
 line 670] 
	 CNavDTD::DidBuildModel
[c:/builds/tinderbox/firefox_branch/WINNT_5.0_Clobber/mozilla/htmlparser/src/CNavDTD.cpp
 line 707] 
	 nsParser::DidBuildModel
[c:/builds/tinderbox/firefox_branch/WINNT_5.0_Clobber/mozilla/htmlparser/src/nsParser.cpp
 line 1245]  

Comment 4

[Johnny Stenback (:jst)]

Attachment: Make RDFXMLDataSourceImpl safe against observers that remove themselves when called...

Never having seen this crash, I can't say for sure that this will fix it, but
it really does seem like it would.

The problem is that RDFXMLDataSourceImpl makes calls on its observers w/o
holding strong references to them, so if an observer removes itself from the
RDFXMLDataSourceImpl's list of observers, and there are no other references to
the observer (or the reference counted XPConnect wrapper for the JS observer),
the observer will go away before the call unwinds, and depending on the
implementation, it may or may not crash. In the XPConnect wrapped JS case, the
observer (i.e. XPConnect JS object wrapper) will crash, at least in some cases,
if it's destroyed during the call to nsXPCWrappedJSClass::CallMethod().

Comment 5

[Brendan Eich [:brendan]]

Comment on attachment 151566 [details] [diff] [review]
Make RDFXMLDataSourceImpl safe against observers that remove themselves when called...

sr=[e-mail], thanks again for doing this.

/be

Comment 6

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 151566 [details] [diff] [review]
Make RDFXMLDataSourceImpl safe against observers that remove themselves when called...

Why the null checks of |obs| inside the loops?	You're iterating backwards, so
that shouldn't be possible even if an observer removes itself, and it doesn't
really seem legitimate for an observer to remove other observers too...

Other than that, r=dbaron.

Comment 7

[Johnny Stenback (:jst)]

(In reply to comment #6)
> (From update of attachment 151566 [details] [diff] [review])
> Why the null checks of |obs| inside the loops?	You're iterating backwards, so
> that shouldn't be possible even if an observer removes itself, and it doesn't
> really seem legitimate for an observer to remove other observers too...

The idea behind that was to make this crash proof in the case where an observer
removes itself *and* another observer. Not something that anyone should ever do
(and this code doesn't attempt to do the *right* thing in that case anycase),
but I'd rather have the code not *crash* if some code ends up doing that.

Thanks for the reviews!

Comment 8

[Johnny Stenback (:jst)]

Fix checked in on the trunk.

Comment 9

[Johnny Stenback (:jst)]

Fixed on the aviary branch as well.

Comment 10

[Johnny Stenback (:jst)]

Moving to Browser, as this is really a RDF bug.

Comment 11

[Axel Hecht]

Comment on attachment 151566 [details] [diff] [review]
Make RDFXMLDataSourceImpl safe against observers that remove themselves when called...

I'd appreciate seing RDF bugs before they land.
Anyway, the fix is ok.

Comment 12

[Mike Kaply [:mkaply]]

Comment on attachment 151566 [details] [diff] [review]
Make RDFXMLDataSourceImpl safe against observers that remove themselves when called...

a=mkaply for 1.7.1

Comment 13

[Johnny Stenback (:jst)]

(In reply to comment #11)
> (From update of attachment 151566 [details] [diff] [review])
> I'd appreciate seing RDF bugs before they land.
> Anyway, the fix is ok.
> 

Duh, sorry, totally forgot to cc you here...

Comment 14

[Jay Patel [:jay]]

Marking verified base on Talkback data.  There are other crashes with this stack
signature, but after A LOT of crashes with this particular stacktrace in Firefox
0.9, I don't see any in recent releases or nightlies.

Comment 15

[Johnny Stenback (:jst)]

Attachment: Fix the forgotten weak observer pointer.

Comment 16

[Darin Fisher]

Comment on attachment 158445 [details] [diff] [review]
Fix the forgotten weak observer pointer.

sr=darin

Comment 17

[Axel Hecht]

Comment on attachment 158445 [details] [diff] [review]
Fix the forgotten weak observer pointer.

r=me

Comment 18

[Johnny Stenback (:jst)]

Reopening as there was a mistake in the original patch, forgot to make one of
the observer pointers an nsCOMPtr, so it remained weak, and part of this problem
still lives (even though it seems to show its face rarely nowadays).

Comment 19

[Johnny Stenback (:jst)]

Comment on attachment 158445 [details] [diff] [review]
Fix the forgotten weak observer pointer.

Asa says a=asa

Comment 20

[Johnny Stenback (:jst)]

Mistake corrected on trunk, aviary, and 1.7...