469432 - Crash [@ nsStyleContext::~nsStyleContext] on reload with menuitem, select, tooltip and mathml

Status: RESOLVED WORKSFORME

(Core :: Layout, defect, P3)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, which crashes current trunk build and Firefox 3 (so marking security sensitive for now) on reload.
It doesn't crash Firefox 2, I can look for a regression range, if wanted.

http://crash-stats.mozilla.com/report/index/122ea824-3d00-474a-a383-d1cc42081212?p=1
0  	xul.dll  	nsStyleContext::~nsStyleContext  	layout/style/nsStyleContext.cpp:100
1 	xul.dll 	nsStyleContext::Destroy 	layout/style/nsStyleContext.cpp:932
2 	xul.dll 	UndisplayedNode::~UndisplayedNode 	layout/base/nsFrameManager.cpp:214
3 	xul.dll 	UndisplayedNode::~UndisplayedNode 	layout/base/nsFrameManager.cpp:211
4 	xul.dll 	RemoveUndisplayedEntry 	layout/base/nsFrameManager.cpp:1848
5 	plds4.dll 	PL_HashTableEnumerateEntries 	nsprpub/lib/ds/plhash.c:432
6 	xul.dll 	nsFrameManagerBase::UndisplayedMap::Clear 	layout/base/nsFrameManager.cpp:1857
7 	xul.dll 	nsFrameManagerBase::UndisplayedMap::~UndisplayedMap 	layout/base/nsFrameManager.cpp:1729
8 	xul.dll 	nsFrameManager::Destroy 	layout/base/nsFrameManager.cpp:297
9 	xul.dll 	PresShell::Destroy 	layout/base/nsPresShell.cpp:1709
10 	xul.dll 	DocumentViewerImpl::Destroy 	layout/base/nsDocumentViewer.cpp:1527
11 	xul.dll 	DocumentViewerImpl::Show 	layout/base/nsDocumentViewer.cpp:1834
12 	xul.dll 	nsPresContext::EnsureVisible 	layout/base/nsPresContext.cpp:1528
13 	xul.dll 	PresShell::UnsuppressAndInvalidate 	layout/base/nsPresShell.cpp:4323
14 	xul.dll 	PresShell::UnsuppressPainting 	layout/base/nsPresShell.cpp:4371
15 	xul.dll 	DocumentViewerImpl::LoadComplete 	layout/base/nsDocumentViewer.cpp:1022
16 	xul.dll 	nsDocShell::EndPageLoad 	docshell/base/nsDocShell.cpp:5184
17 	xul.dll 	nsWebShell::EndPageLoad 	docshell/base/nsWebShell.cpp:1015
18 	xul.dll 	nsDocShell::OnStateChange 	docshell/base/nsDocShell.cpp:5080
19 	xul.dll 	nsDocLoader::FireOnStateChange 	uriloader/base/nsDocLoader.cpp:1235
20 	xul.dll 	nsDocLoader::doStopDocumentLoad 	uriloader/base/nsDocLoader.cpp:858
21 	xul.dll 	nsDocLoader::DocLoaderIsEmpty 	uriloader/base/nsDocLoader.cpp:763
22 	xul.dll 	nsDocLoader::OnStopRequest 	uriloader/base/nsDocLoader.cpp:679
23 	xul.dll 	nsLoadGroup::RemoveRequest 	netwerk/base/src/nsLoadGroup.cpp:688
24 	xul.dll 	nsDocument::DoUnblockOnload 	content/base/src/nsDocument.cpp:7016
25 	xul.dll 	nsDocument::UnblockOnload 	content/base/src/nsDocument.cpp:6963
26 	xul.dll 	nsDocument::DispatchContentLoadedEvents 	content/base/src/nsDocument.cpp:3945
27 	xul.dll 	nsRunnableMethod<nsJSChannel>::Run 	obj-firefox/dist/include/xpcom/nsThreadUtils.h:264
28 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:510
29 	xul.dll 	NS_ProcessNextEvent_P 	obj-firefox/xpcom/build/nsThreadUtils.cpp:227
30 	xul.dll 	nsBaseAppShell::Run 	widget/src/xpwidgets/nsBaseAppShell.cpp:170
31 	xul.dll 	nsAppStartup::Run 	toolkit/components/startup/src/nsAppStartup.cpp:192
32 	xul.dll 	XRE_main 	toolkit/xre/nsAppRunner.cpp:3283
33 	firefox.exe 	NS_internal_main 	browser/app/nsBrowserApp.cpp:156
34 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp:87
35 	firefox.exe 	__tmainCRTStartup 	obj-firefox/memory/jemalloc/src/crtexe.c:591
36 	kernel32.dll 	BaseProcessStart

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This could be a fun one to dig into

Comment 2

[Zack Weinberg (:zwol)]

The crash is on this line:

  presContext->PresShell()->StyleSet()->
    NotifyStyleContextDestroyed(presContext, this);

so that looks like a classic use-after-free problem (as we're in the middle of destroying the pres shell when that happens) but I can't reproduce the crash on Linux even under valgrind.  Will try Windows later today.

Comment 3

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

But we destroy the frame manager before destroying the style set or clearing out the pres context's back pointer.  (nsPresShell's destructor is pretty carefully ordered; it could use some better comments.)

Comment 4

[Zack Weinberg (:zwol)]

The crash does reproduce on Windows XP in my virtual machine.  I'm not sure yet exactly where it's going off the rails.

Comment 5

[Zack Weinberg (:zwol)]

*@!#!#(&()@#&%@*(#%_!@$&!(@$&*!_%$!(#%&+

The crash disappears with optimization disabled.

Comment 6

[Zack Weinberg (:zwol)]

I'm sorry, I'm going to have to give this one back.  My Windows environment is just too slow (two hours for an unoptimized build) and I'm too unfamiliar with the ins and outs of this compiler to fix this bug efficiently.

Comment 7

[Damon Sicore (:damons)]

Dbaron, can you take this?

Comment 8

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

I'm happy to, since the crash is fixed by the patch in bug 475128.  (There's still another underlying problem shown by the testcase, though, since it still shows the assertions one would expect from a crash fixed by that patch.  That said, the underlying problem in question may well be a duplicate of bug 474377.)

Comment 9

[Jesse Ruderman]

For searching purposes, the currently assertions triggered by this testcase:

###!!! ASSERTION: style context has old rule node: 'n == mRuleTree', file /Users/jruderman/central/layout/style/nsStyleSet.cpp, line 181

###!!! ASSERTION: old rule tree still referenced: 'Not Reached', file /Users/jruderman/central/layout/style/nsStyleSet.cpp, line 947

Comment 10

[Jesse Ruderman]

WFM, no assertions at all now.  I'll add a crashtest.

The crash (and security problem) were fixed by bug 475128 on all active branches, so I'm making this bug public.

Comment 11

[Jesse Ruderman]

Crashtest: http://hg.mozilla.org/mozilla-central/rev/b64a7e4a35e2