Closed Bug 226744 Opened 21 years ago Closed 21 years ago

[FIXr]Crash with stylesheet element in xbl binding

Categories

(Core :: XBL, defect, P2)

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: 21 years ago
Resolution: --- → FIXED
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: