Closed Bug 226744 Opened 16 years ago Closed 16 years ago

[FIXr]Crash with stylesheet element in xbl binding

Categories

(Core :: XBL, defect, P2, critical)

defect

Tracking

()

RESOLVED FIXED
mozilla1.7alpha

People

(Reporter: martijn.martijn, Assigned: bzbarsky)

References

Details

(Keywords: crash, stackwanted)

Attachments

(2 files, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6a) Gecko/20031107 Firebird/0.7+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6a) Gecko/20031107 Firebird/0.7+

Recent builds of the mozilla 1.6 alpha version crash when I use the stylesheet
element in an xbl binding.
Builds of at least 7 november 2003 don't  crash.
Builds of 24 november 2003 will crash.
I don't get a talkback response or anything. Mozilla just disappears.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.
This testcase should crash recent builds of Mozilla 1.6a.
I've put the binding, stylesheet and xhtml in one file for bugzilla
convenience.

Source testcase:
<?xml version="1.0" encoding="utf-8"?>
<html xmlns="http://www.w3.org/1999/xhtml"
    xmlns:html="http://www.w3.org/1999/xhtml"
    xmlns:xbl="http://www.mozilla.org/xbl">

    <xbl:bindings>
	<xbl:binding id="rd">
				<xbl:resources>
<xbl:stylesheet src="data:text/html;charset=utf-8,"/>
</xbl:resources>
	    <xbl:content>
<xbl:children/>
	    </xbl:content>
	</xbl:binding>

    </xbl:bindings>

    <head>
	<title>testcase bug ?</title>
	
	<style type="text/css">
	.hoofdstuk {
	    -moz-binding: url(#rd);
	}
	</style>
    </head>
    
    <body>    
<div class="hoofdstuk">test
</div>
<div class="hoofdstuk">test
</div>
    </body>
Sorry, the first one didn't crash. This one should, because it has the right
mime-type.
Attachment #136295 - Attachment is obsolete: true
Well, it seems that it doesn't crash when you first load it, but it does crash
when you click back and then forward again.
Or if you save it local and then open it.
Severity: normal → critical
Keywords: crash
Keywords: stackwanted
Looks to me like something is clobbering the stack -- we should not end up in
XPConnect here:

#0  0x40af9869 in XPC_WN_NoHelper_JSClass ()
   from
/home/bzbarsky/mozilla/debug/obj-debug/dist/bin/components/libxpconnect.so
#1  0x08960fa4 in ?? ()
#2  0x40948494 in nsCOMArray_base::InsertObjectAt(nsISupports*, int) (
    this=0x8960fa4, aObject=0x89d2f90, aIndex=0)
    at /home/bzbarsky/mozilla/debug/mozilla/xpcom/ds/nsCOMArray.cpp:87
#3  0x41449c5c in nsCOMArray_base::AppendObject(nsISupports*) (this=0x8960fa4, 
    aObject=0x89d2f90) at ../../../dist/include/xpcom/nsCOMArray.h:84
#4  0x41446565 in nsCOMArray<nsIStyleSheet>::AppendObject(nsIStyleSheet*) (
    this=0x8960fa4, aObject=0x89d2f90)
    at ../../../dist/include/xpcom/nsCOMArray.h:239
#5  0x4169d2bf in nsXBLResourceLoader::StyleSheetLoaded(nsICSSStyleSheet*, int)
    (this=0x87ebd50, aSheet=0x89d2f90, aNotify=1)
    at
/home/bzbarsky/mozilla/debug/mozilla/content/xbl/src/nsXBLResourceLoader.cpp:186
#6  0x41610a8f in CSSLoaderImpl::SheetComplete(SheetLoadData*, int) (
    this=0x893f8d0, aLoadData=0x89d2cd0, aSucceeded=0)
    at
/home/bzbarsky/mozilla/debug/mozilla/content/html/style/src/nsCSSLoader.cpp:1499
Note that the debug build I'm using here has the XBL code using nsCOMArray
instead of nsISupportsArray... I doubt that matters, but I'll see whether I have
a clean build...
#0  0x4169b650 in nsXBLResourceLoader::StyleSheetLoaded(nsICSSStyleSheet*, int)
    (this=0x89cc9c0, aSheet=0x89cd620, aNotify=1)
    at
/home/bzbarsky/mozilla/xlib/mozilla/content/xbl/src/nsXBLResourceLoader.cpp:191
#1  0x4160eca3 in CSSLoaderImpl::SheetComplete(SheetLoadData*, int) (
    this=0x8951940, aLoadData=0x89cd678, aSucceeded=0)
    at
/home/bzbarsky/mozilla/xlib/mozilla/content/html/style/src/nsCSSLoader.cpp:1499
#2  0x4160bd48 in SheetLoadData::OnStreamComplete(nsIUnicharStreamLoader*,
nsISupports*, unsigned, nsIUnicharInputStream*) (this=0x89cd678,
aLoader=0x89cd8b0, 
    aContext=0x0, aStatus=0, aDataStream=0x0)
    at
/home/bzbarsky/mozilla/xlib/mozilla/content/html/style/src/nsCSSLoader.cpp:705

(gdb) frame 0
#0  0x4169b647 in nsXBLResourceLoader::StyleSheetLoaded(nsICSSStyleSheet*, int)
    (this=0x87ddd58, aSheet=0x89cc710, aNotify=1)
    at
/home/bzbarsky/mozilla/xlib/mozilla/content/xbl/src/nsXBLResourceLoader.cpp:191
191       mResources->mStyleSheetList->AppendElement(aSheet);
(gdb) p mResources->mStyleSheetList
$1 = {mRawPtr = 0x6e6f6974}
(gdb) p mResources->mStyleSheetList.mRawPtr
$2 = (nsISupportsArray *) 0x6e6f6974
(gdb) p *(class nsSupportsArray*)mResources->mStyleSheetList.mRawPtr
Cannot access memory at address 0x6e6f6974

Other times I've just gotten pointers to arrays with obviuosly bogus counts...
Also:

(gdb) p *mResources
$4 = {mLoader = 0x61636f4c, mStyleSheetList = {mRawPtr = 0x6e6f6974}, 
  mRuleProcessors = {mRawPtr = 0x0}}
(gdb) p *(mResources->mLoader)
Cannot access memory at address 0x61636f4c

So chances are, the mResources thing got destroyed or something.... Or we got
destroyed...

2003-11-18-07 works, 2003-11-19-12 crashes (on Linux)

The checkin in there that's most likely to have changed some sort of refcounting
fu is jst's....
OS: Windows XP → All
Hardware: PC → All
OK, here is the skinny on this one:

The checkin for bug 225837 had a logic issue that caused multiple concurrent
async loads of bindings to result in multiple requests for the binding document
(bug 225837 comment 16).  That resulted in two document infos for the same url
being stored in the cache, which blew the first of them out of the cache, sent
its refcount to zero, and caused its destructor to fire.  That wiped out the
prototype bindings associated to it, which deleted the nsXBLPrototypeResources
for those bindings.

But the nsXBLResourceLoader has a (weak) back-ref to the
nsXBLPrototypeResources, and ~nsXBLPrototypeResources does not null this out. 
So nsXBLResourceLoader called some methods on a deleted object, and the rest is
history.

The patch in bug 225837 comment 17 fixes this crash by removing the HTML
issue... but we should still somehow make things work in this case... perhaps
~nsXBLPrototypeResources should just null out the mResources pointer in
nsXBLResourceLoader and nsXBLResourceLoader should deal with a null mResources?
Depends on: 225837
Attached patch Like thisSplinter Review
This patch fixes the crash even without the patch from bug 225837.  The
resulting behavior is not correct, of course....  Should this code assert if
mResources is null there?  It looks like it could happen with some combination
of sync and async loads....

Also, perhaps placement into the document info hashtable should not happen if
there is already something there for that URI?
Attachment #136645 - Flags: superreview?(jst)
Attachment #136645 - Flags: review?(bryner)
Comment on attachment 136645 [details] [diff] [review]
Like this

sr=jst
Attachment #136645 - Flags: superreview?(jst) → superreview+
"The patch in bug 225837 comment 17 fixes this crash"
Uh?
The fix has been checked in, so there should be no crashing anymore?
It still crashes with me.
Using:
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6b) Gecko/20031203
Firebird/0.7+
Or am I missing something?
Heh.  Nice catch.  See bug 225837 comment 22 
It doesn't crash anymore.
Great job guys! Especially the checking in part ;-)
Attachment #136645 - Flags: review?(bryner) → review+
Taking.  Since this is not an urgent crash issue right now, will land in 1.7a
Assignee: hyatt → bz-vacation
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P2
Summary: Crash with stylesheet element in xbl binding → [FIXr]Crash with stylesheet element in xbl binding
Target Milestone: --- → mozilla1.7alpha
Checked in for 1.7a.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.