Closed Bug 476526 Opened 15 years ago Closed 15 years ago

Crash [@ nsStyleContext::Release] with bindings, DOMAttrModified, observes

Categories

(Core :: XUL, defect, P2)

x86
Windows XP
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: martijn.martijn, Assigned: roc)

Details

(4 keywords)

Crash Data

Attachments

(6 files)

See upcoming testcase, which crashes current trunk build on load.
It also crashes Firefox 3, so marking security sensitive for now.
It doesn't seem to crash Firefox 2.

I once got this crash stack in an unminimized testcase:
http://crash-stats.mozilla.com/report/index/eb70b117-585c-4cce-8de8-dfa372090201?p=1
0  	xul.dll  	nsStyleContext::Release  	 obj-firefox/dist/include/layout/nsStyleContext.h:100
1 	xul.dll 	nsElementSH::PostCreate 	dom/src/base/nsDOMClassInfo.cpp:7601
2 	xul.dll 	XPCWrappedNative::GetNewOrUsed 	js/src/xpconnect/src/xpcwrappednative.cpp:548
3 	xul.dll 	XPCConvert::NativeInterface2JSObject 	js/src/xpconnect/src/xpcconvert.cpp:1146
4 	xul.dll 	nsXPConnect::WrapNativeToJSVal 	js/src/xpconnect/src/nsXPConnect.cpp:1260
5 	xul.dll 	nsContentList::GetNodeAt 	content/base/src/nsContentList.cpp:533

But with the minimized testcase, I just get useless stacks.

In a debug build I don't seem to crash, but I'm seeing these assertions:
###!!! ASSERTION: killing mutation events: 'nsContentUtils::IsSafeToRunScript()'
, file c:\mozilla-build-1.3\src\_firefox\dist\include\content\nsContentUtils.h,
line 1549
###!!! ASSERTION: Dying in the middle of our own update?: 'mUpdateCount == 0', f
ile c:\mozilla-build-1.3\src\layout\base\nsCSSFrameConstructor.h, line 90
###!!! ASSERTION: Some objects allocated with AllocateFrame were not freed: 'mFr
ameCount == 0', file c:/mozilla-build-1.3/src/layout/base/nsPresShell.cpp, line
681
###!!! ASSERTION: killing mutation events: 'nsContentUtils::IsSafeToRunScript()'
, file c:\mozilla-build-1.3\src\_firefox\dist\include\content\nsContentUtils.h,
line 1549
Perhaps related to bug 454276?
I can't reproduce the crash nor the assertions (on trunk/linux/debug).
Yeah, seems to be worksforme, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090201 Minefield/3.2a1pre (.NET CLR 3.5.30729)
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → WORKSFORME
Attached file zipped up testcase
This is still crashing, which is what this zipped up testcase is showing.
It seems the Bugzilla change that makes every testcase redirect to its own subdomain is causing the original testcase to not crash anymore.

Gerv, I guess this was an intentional change in Bugzilla? I'm afraid this will probably more problems with other bugs that have testcase that do cross-frame scripting.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Yep, sorry. It's an intentional change to isolate attachments from each other and stop them being able to steal cookies. We can't really revert the change wholesale - it's the only fix for the issues - but if it's causing problems, we'd be happy to hear the exact details of what you'd like to do but can't, and we can see if we can tweak things.

Gerv
But why the redirecting? Why not go directly to https://bug476526.bugzilla.mozilla.org/attachment.cgi?id=360160&t=CE1UEgwIuY for example, instead of redirecting from https://bugzilla.mozilla.org/attachment.cgi?id=360160 ?
Why do attachments need to be isolated from each other?
Flags: blocking1.9.2?
Flags: blocking1.9.1?
Can't reproduce the crash, using .zip testcase. Tested using linux.
I do get ###!!! ASSERTION: Some objects allocated with AllocateFrame were not freed
It still crashes for me in current trunk build on windows XP.
Should probably block.
I'm not seeing a crash, but I'm seeing the second and third assertions in comment 0.  The third assertion seems likely to be the result of the second.
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
Assignee: nobody → roc
Attached file simpler testcase
This triggers the "dying in the middle of our own update" assertion. The problem is simple:
-- We change the attribute of an element in the subdocument.
-- This enters a document update
-- The attribute is removed and the mutation event fires.
-- The mutation event handler removes the iframe from the parent document.
-- This tears down the presentation, destroying the nsCSSFrameConstructor, which asserts because we're in the update.
I'm not sure of the right way to fix this. Can we end the document update before we fire the mutation event?
Ending the document update before we fire the mutation event fixes the "dying in the middle of our own update" assertion, but we still assert "some objects allocated with AllocateFrame were not freed".
I think Jonas has been pushing on allowing async, or at least "at a safe point" firing of mutation events.  We could probably do the latter already for a lot of them; the only one it's a problem for is DOMNodeRemoved, no?

That is, the mutation event would fire from EndUpdate.
Attachment #375106 - Attachment is patch: false
Attachment #375106 - Attachment mime type: text/plain → text/html
The remaining assertion is because we leak a style context (and some stuff hanging off it) allocated here:

Breakpoint 1, FrameArena::AllocateFrame (this=0x92792e4, aSize=44) at /Users/roc/mozilla-checkin/layout/base/nsPresShell.cpp:727
727	  return result;
$7357 = (void *) 0x14b3c60
#0  FrameArena::AllocateFrame (this=0x92792e4, aSize=44) at /Users/roc/mozilla-checkin/layout/base/nsPresShell.cpp:727
#1  0x037089c2 in PresShell::AllocateFrame (this=0x9279200, aSize=44) at /Users/roc/mozilla-checkin/layout/base/nsPresShell.cpp:1907
#2  0x038ac1eb in nsPresContext::AllocateFromShell (this=0x9363800, aSize=44) at nsPresContext.h:279
#3  0x038b5582 in nsStyleContext::operator new (sz=44, aPresContext=0x9363800) at /Users/roc/mozilla-checkin/layout/style/nsStyleContext.cpp:547
#4  0x038b5ae5 in NS_NewStyleContext (aParentContext=0x0, aPseudoTag=0x0, aRuleNode=0x1501ebc, aPresContext=0x9363800) at /Users/roc/mozilla-checkin/layout/style/nsStyleContext.cpp:573
#5  0x038b8a0d in nsStyleSet::GetContext (this=0xdc80570, aPresContext=0x9363800, aParentContext=0x0, aPseudoTag=0x0) at /Users/roc/mozilla-checkin/layout/style/nsStyleSet.cpp:456
#6  0x038ba1ff in nsStyleSet::ResolveStyleFor (this=0xdc80570, aContent=0xd0a87a0, aParentContext=0x0) at /Users/roc/mozilla-checkin/layout/style/nsStyleSet.cpp:692
#7  0x03ca5241 in nsElementSH::PostCreate (this=0xdc754a0, wrapper=0xdd249f0, cx=0x12fbc00, obj=0xbeae100) at /Users/roc/mozilla-checkin/dom/base/nsDOMClassInfo.cpp:7575
#8  0x00dfa55b in XPCWrappedNative::GetNewOrUsed (ccx=@0xbfffb6dc, Object=0xd0a87a0, Scope=0xdd012a0, Interface=0x0, cache=0xd0a87a4, isGlobal=0, resultWrapper=0xbfffb60c) at /Users/roc/mozilla-checkin/js/src/xpconnect/src/xpcwrappednative.cpp:595
#9  0x00dd0283 in XPCConvert::NativeInterface2JSObject (ccx=@0xbfffb6dc, d=0xbfffb844, dest=0xbfffb840, src=0xd0a87a0, iid=0x0, Interface=0x0, cache=0xd0a87a4, scope=0xdbfb040, allowNativeWrapper=0, isGlobal=0, pErr=0xbfffb76c) at /Users/roc/mozilla-checkin/js/src/xpconnect/src/xpcconvert.cpp:1146
#10 0x00da918e in nsXPConnect::WrapNativeToJSVal (this=0xb242f0, aJSContext=0x12fbc00, aScope=0xdbfb040, aCOMObj=0xd0a87a0, aIID=0x0, aVal=0xbfffb844, aHolder=0xbfffb840) at /Users/roc/mozilla-checkin/js/src/xpconnect/src/nsXPConnect.cpp:1258
#11 0x03c8ad7e in nsDOMClassInfo::WrapNative (cx=0x12fbc00, scope=0xdbfb040, native=0xd0a87a0, aIID=0x0, vp=0xbfffb844, aHolder=0xbfffb840) at /Users/roc/mozilla-checkin/dom/base/nsDOMClassInfo.cpp:1659
#12 0x03ca89f9 in nsDOMClassInfo::WrapNative (cx=0x12fbc00, scope=0xdbfb040, native=0xd0a87a0, vp=0xbfffb844, aHolder=0xbfffb840) at nsDOMClassInfo.h:146
#13 0x03c940dc in nsNodeSH::PreCreate (this=0xb406c30, nativeObj=0xdd510d0, cx=0x12fbc00, globalObj=0xdbfb040, parentObj=0xbfffb97c) at /Users/roc/mozilla-checkin/dom/base/nsDOMClassInfo.cpp:7010
#14 0x00df9bfd in XPCWrappedNative::GetNewOrUsed (ccx=@0xbfffbbb4, Object=0xdd510f0, Scope=0xdd012a0, Interface=0x12f5a00, cache=0xdd510d4, isGlobal=0, resultWrapper=0xbfffba8c) at /Users/roc/mozilla-checkin/js/src/xpconnect/src/xpcwrappednative.cpp:428
#15 0x00dd0283 in XPCConvert::NativeInterface2JSObject (ccx=@0xbfffbbb4, d=0x98f1a30, dest=0x0, src=0xdd510f0, iid=0x0, Interface=0x12f5a00, cache=0xdd510d4, scope=0xdbfb2a0, allowNativeWrapper=1, isGlobal=0, pErr=0xbfffbb54) at /Users/roc/mozilla-checkin/js/src/xpconnect/src/xpcconvert.cpp:1146
#16 0x00e185ed in xpc_qsXPCOMObjectToJsval (ccx=@0xbfffbbb4, p=0xdd510f0, cache=0x0, iface=0x12f5a00, rval=0x98f1a30) at /Users/roc/mozilla-checkin/js/src/xpconnect/src/xpcquickstubs.cpp:1059
#17 0x00e44d87 in nsIDOMDocument_GetElementById (cx=0x12fbc00, argc=1, vp=0x98f1a30) at dom_quickstubs.cpp:2499
#18 0x002e5a04 in js_Interpret (cx=0x12fbc00) at /Users/roc/mozilla-checkin/js/src/jsinterp.cpp:5100
#19 0x002f9f07 in js_Invoke (cx=0x12fbc00, argc=1, vp=0x98f1a24, flags=0) at jsinterp.cpp:1373
We *appear* to leak the style context because we're destroying the frame arena when nsElementSH::PostCreate is calling LoadBindings while holding a strong reference to the style context!

#10 0x036d8a41 in DocumentViewerImpl::DestroyPresShell (this=0xb981f00) at /Users/roc/mozilla-checkin/layout/base/nsDocumentViewer.cpp:4255
#11 0x036dc867 in DocumentViewerImpl::Hide (this=0xb981f00) at /Users/roc/mozilla-checkin/layout/base/nsDocumentViewer.cpp:1963
#12 0x02d5be24 in nsDocShell::SetVisibility (this=0xdca3560, aVisibility=0) at /Users/roc/mozilla-checkin/docshell/base/nsDocShell.cpp:4111
#13 0x03771e21 in nsSubDocumentFrame::HideViewer (this=0x94e250c) at /Users/roc/mozilla-checkin/layout/generic/nsFrameFrame.cpp:797
#14 0x03771fa8 in nsSubDocumentFrame::Destroy (this=0x94e250c) at /Users/roc/mozilla-checkin/layout/generic/nsFrameFrame.cpp:762
#15 0x03739903 in nsBlockFrame::DoRemoveFrame (this=0x94e2330, aDeletedFrame=0x94e250c, aFlags=2) at /Users/roc/mozilla-checkin/layout/generic/nsBlockFrame.cpp:5442
#16 0x03739e89 in nsBlockFrame::RemoveFrame (this=0x94e2330, aListName=0x0, aOldFrame=0x94e250c) at /Users/roc/mozilla-checkin/layout/generic/nsBlockFrame.cpp:5032
#17 0x036ebad8 in nsFrameManager::RemoveFrame (this=0x1730c1c, aParentFrame=0x94e2330, aListName=0x0, aOldFrame=0x94e250c) at /Users/roc/mozilla-checkin/layout/base/nsFrameManager.cpp:714
#18 0x036aa49f in nsCSSFrameConstructor::ContentRemoved (this=0xb9b2a70, aContainer=0x791c480, aChild=0xd072230, aIndexInContainer=1, aDidReconstruct=0xbfff9b78) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:7238
#19 0x0370dfa0 in PresShell::ContentRemoved (this=0x1730c00, aDocument=0x16ae600, aContainer=0x791c480, aChild=0xd072230, aIndexInContainer=1) at /Users/roc/mozilla-checkin/layout/base/nsPresShell.cpp:4901
#20 0x039f482a in nsNodeUtils::ContentRemoved (aContainer=0x791c480, aChild=0xd072230, aIndexInContainer=1) at /Users/roc/mozilla-checkin/content/base/src/nsNodeUtils.cpp:167
#21 0x039de403 in nsGenericElement::doRemoveChildAt (aIndex=1, aNotify=1, aKid=0xd072230, aParent=0x791c480, aDocument=0x16ae600, aChildArray=@0x791c49c) at /Users/roc/mozilla-checkin/content/base/src/nsGenericElement.cpp:3395
#22 0x039de54f in nsGenericElement::RemoveChildAt (this=0x791c480, aIndex=1, aNotify=1) at /Users/roc/mozilla-checkin/content/base/src/nsGenericElement.cpp:3325
#23 0x039d7449 in nsGenericElement::doRemoveChild (aOldChild=0xd07225c, aParent=0x791c480, aDocument=0x16ae600, aReturn=0xbfff9e7c) at /Users/roc/mozilla-checkin/content/base/src/nsGenericElement.cpp:4002
#24 0x039d74b2 in nsGenericElement::RemoveChild (this=0x791c480, aOldChild=0xd07225c, aReturn=0xbfff9e7c) at /Users/roc/mozilla-checkin/content/base/src/nsGenericElement.cpp:3560
#25 0x03aaf7ed in nsHTMLBodyElement::RemoveChild (this=0x791c480, oldChild=0xd07225c, _retval=0xbfff9e7c) at /Users/roc/mozilla-checkin/content/html/content/src/nsHTMLBodyElement.cpp:95
#26 0x00e3aa27 in nsIDOMNode_RemoveChild (cx=0x14d4400, argc=1, vp=0x9541e30) at dom_quickstubs.cpp:4183
#27 0x002e5a04 in js_Interpret (cx=0x14d4400) at /Users/roc/mozilla-checkin/js/src/jsinterp.cpp:5100
#28 0x002f9f07 in js_Invoke (cx=0x14d4400, argc=1, vp=0x9541e24, flags=0) at jsinterp.cpp:1373
#29 0x002fa1b1 in js_InternalInvoke (cx=0x14d4400, obj=0x30fce20, fval=51360416, flags=0, argc=1, argv=0x9541e20, rval=0xbfffa778) at jsinterp.cpp:1426
#30 0x0027d24f in JS_CallFunctionValue (cx=0x14d4400, obj=0x30fce20, fval=51360416, argc=1, argv=0x9541e20, rval=0xbfffa778) at /Users/roc/mozilla-checkin/js/src/jsapi.cpp:5207
#31 0x03c44524 in nsJSContext::CallEventHandler (this=0xd077d90, aTarget=0xd0d7590, aScope=0xb879460, aHandler=0x30fb2a0, aargv=0xd082ce0, arv=0xbfffa8f4) at /Users/roc/mozilla-checkin/dom/base/nsJSEnvironment.cpp:2009
#32 0x03cb612e in nsJSEventListener::HandleEvent (this=0xd0d75c0, aEvent=0xd0b4264) at /Users/roc/mozilla-checkin/dom/src/events/nsJSEventListener.cpp:247
#33 0x03a51f9f in nsEventListenerManager::HandleEventSubType (this=0xdc00230, aListenerStruct=0xdc00258, aListener=0xd0d75c0, aDOMEvent=0xd0b4264, aCurrentTarget=0xd0d7590, aPhaseFlags=2) at /Users/roc/mozilla-checkin/content/events/src/nsEventListenerManager.cpp:1090
#34 0x03a5239d in nsEventListenerManager::HandleEvent (this=0xdc00230, aPresContext=0x0, aEvent=0xbfffaf78, aDOMEvent=0xbfffad10, aCurrentTarget=0xd0d7590, aFlags=2, aEventStatus=0xbfffad14) at /Users/roc/mozilla-checkin/content/events/src/nsEventListenerManager.cpp:1187
#35 0x03a85746 in nsEventTargetChainItem::HandleEvent (this=0x12e9a00, aVisitor=@0xbfffad08, aFlags=2, aMayHaveNewListenerManagers=0) at /Users/roc/mozilla-checkin/content/events/src/nsEventDispatcher.cpp:227
#36 0x03a85ab5 in nsEventTargetChainItem::HandleEventTargetChain (this=0x12e9be0, aVisitor=@0xbfffad08, aFlags=6, aCallback=0x0, aMayHaveNewListenerManagers=1) at /Users/roc/mozilla-checkin/content/events/src/nsEventDispatcher.cpp:315
#37 0x03a86257 in nsEventDispatcher::Dispatch (aTarget=0xdca2920, aPresContext=0x0, aEvent=0xbfffaf78, aDOMEvent=0x0, aEventStatus=0x0, aCallback=0x0) at /Users/roc/mozilla-checkin/content/events/src/nsEventDispatcher.cpp:508
#38 0x039e0a93 in nsGenericElement::SetAttrAndNotify (this=0xdca2920, aNamespaceID=1, aName=0x10090d4, aPrefix=0x0, aOldValue=@0xbfffb064, aParsedValue=@0xbfffb100, aModification=1, aFireMutation=1, aNotify=1, aValueForAfterSetAttr=0xbfffb1c4) at /Users/roc/mozilla-checkin/content/base/src/nsGenericElement.cpp:4408
#39 0x039e0e67 in nsGenericElement::SetAttr (this=0xdca2920, aNamespaceID=1, aName=0x10090d4, aPrefix=0x0, aValue=@0xbfffb1c4, aNotify=1) at /Users/roc/mozilla-checkin/content/base/src/nsGenericElement.cpp:4300
#40 0x03a9b73b in nsGenericHTMLElement::SetAttr (this=0xdca2920, aNameSpaceID=1, aName=0x10090d4, aPrefix=0x0, aValue=@0xbfffb1c4, aNotify=1) at /Users/roc/mozilla-checkin/content/html/content/src/nsGenericHTMLElement.cpp:1151
#41 0x03c13585 in nsXULDocument::SynchronizeBroadcastListener (this=0x16aac00, aBroadcaster=0xdca28f0, aListener=0xdca2940, aAttr=@0xdc05000) at /Users/roc/mozilla-checkin/content/xul/document/src/nsXULDocument.cpp:736
#42 0x03c13251 in nsXULDocument::MaybeBroadcast (this=0x16aac00) at /Users/roc/mozilla-checkin/content/xul/document/src/nsXULDocument.cpp:3334
#43 0x03c1c45e in nsRunnableMethod<nsXULDocument, void>::Run (this=0xdc05020) at nsThreadUtils.h:264
#44 0x03976c92 in nsContentUtils::RemoveScriptBlocker () at /Users/roc/mozilla-checkin/content/base/src/nsContentUtils.cpp:4349
#45 0x036af8a7 in nsAutoScriptBlocker::~nsAutoScriptBlocker (this=0xbfffb427) at nsContentUtils.h:1562
#46 0x036af8bb in nsAutoScriptBlocker::~nsAutoScriptBlocker (this=0xbfffb427) at nsContentUtils.h:1563
#47 0x03bd62ec in nsXBLBinding::InstallAnonymousContent (this=0xdc04dd0, aAnonParent=0xdca28a0, aElement=0xd0d7590) at /Users/roc/mozilla-checkin/content/xbl/src/nsXBLBinding.cpp:381
#48 0x03bd7b67 in nsXBLBinding::GenerateAnonymousContent (this=0xdc04dd0) at /Users/roc/mozilla-checkin/content/xbl/src/nsXBLBinding.cpp:694
#49 0x03bf6330 in nsXBLService::LoadBindings (this=0x717e720, aContent=0xd0d7590, aURL=0xb981670, aOriginPrincipal=0xb98f060, aAugmentFlag=0, aBinding=0xbfffb840, aResolveStyle=0xbfffb834) at /Users/roc/mozilla-checkin/content/xbl/src/nsXBLService.cpp:664
#50 0x03ca53e0 in nsElementSH::PostCreate (this=0xb2fa4d0, wrapper=0xd046a50, cx=0x1367400, obj=0x30fce20) at /Users/roc/mozilla-checkin/dom/base/nsDOMClassInfo.cpp:7592
#51 0x00dfa55b in XPCWrappedNative::GetNewOrUsed (ccx=@0xbfffbbb4, Object=0xd0d75b0, Scope=0xdc03630, Interface=0x12faa00, cache=0xd0d7594, isGlobal=0, resultWrapper=0xbfffba8c) at /Users/roc/mozilla-checkin/js/src/xpconnect/src/xpcwrappednative.cpp:595
#52 0x00dd0283 in XPCConvert::NativeInterface2JSObject (ccx=@0xbfffbbb4, d=0x11b1230, dest=0x0, src=0xd0d75b0, iid=0x0, Interface=0x12faa00, cache=0xd0d7594, scope=0xb8798e0, allowNativeWrapper=1, isGlobal=0, pErr=0xbfffbb54) at /Users/roc/mozilla-checkin/js/src/xpconnect/src/xpcconvert.cpp:1146
Attached patch fixSplinter Review
OK, this fix is simple, we just have to destroy the style context a bit earlier.
Attachment #375160 - Flags: superreview?(jonas)
Attachment #375160 - Flags: review?(jonas)
Whiteboard: [needs review]
Comment on attachment 375160 [details] [diff] [review]
fix

Looks good, but s/load the binding constructor/load the binding/
Attachment #375160 - Flags: superreview?(jonas)
Attachment #375160 - Flags: superreview+
Attachment #375160 - Flags: review?(jonas)
Attachment #375160 - Flags: review+
Whiteboard: [needs review] → [needs landing]
http://hg.mozilla.org/mozilla-central/rev/7db881363a22

Still need to land the testcase.
Status: REOPENED → RESOLVED
Closed: 15 years ago15 years ago
Flags: blocking1.9.2? → in-testsuite?
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs landing] → [needs 191 landing]
Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090509 Minefield/3.6a1pre (.NET CLR 3.5.30729)
Status: RESOLVED → VERIFIED
Crash Signature: [@ nsStyleContext::Release]
Group: core-security
Landed the crashtest:
https://hg.mozilla.org/integration/mozilla-inbound/rev/15705b628343
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.