795591 - crash in mozilla::FrameLayerBuilder::ClearManagerData

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Scoobidiver (away)]

It's #2 top crasher in today's build. The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=895f66c4eada&tochange=c09a0c022b2e
It's likely a regression from bug 539356 part 14.

Signature 	mozilla::FrameLayerBuilder::ClearManagerData(nsIFrame*, mozilla::LayerManagerData*) More Reports Search
UUID	02ca5cf8-d056-4992-9820-6fadd2120929
Date Processed	2012-09-29 13:37:41
Uptime	242
Last Crash	4.1 minutes before submission
Install Age	8.0 minutes since version was first installed.
Install Time	2012-09-29 13:29:13
Product	Firefox
Version	18.0a1
Build ID	20120929030624
Release Channel	nightly
OS	Windows NT
OS Version	6.1.7601 Service Pack 1
Build Architecture	x86
Build Architecture Info	GenuineIntel family 6 model 14 stepping 8
Crash Reason	EXCEPTION_ACCESS_VIOLATION_READ
Crash Address	0xfffffffff0de801b
User Comments	crash while sign out of gmail
App Notes 	
AdapterVendorID: 0x8086, AdapterDeviceID: 0x27a2, AdapterSubsysID: 00000000, AdapterDriverVersion: 8.15.10.1930
D3D10 Layers? D3D10 Layers- D3D9 Layers? D3D9 Layers- 
EMCheckCompatibility	True
Adapter Vendor ID	0x8086
Adapter Device ID	0x27a2
Total Virtual Memory	2147352576
Available Virtual Memory	1652236288
System Memory Use Percentage	43
Available Page File	2629947392
Available Physical Memory	1813180416

Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::FrameLayerBuilder::ClearManagerData 	layout/base/FrameLayerBuilder.cpp:692
1 	xul.dll 	mozilla::FrameLayerBuilder::RemoveDisplayItemDataForFrame 	layout/base/FrameLayerBuilder.cpp:1011
2 	xul.dll 	nsTHashtable<mozilla::FrameLayerBuilder::DisplayItemDataEntry>::s_EnumStub 	obj-firefox/dist/include/nsTHashtable.h:487
3 	xul.dll 	PL_DHashTableEnumerate 	obj-firefox/xpcom/build/pldhash.cpp:716
4 	xul.dll 	mozilla::LayerManagerData::`scalar deleting destructor' 	
5 	xul.dll 	mozilla::layers::LayerManagerUserDataDestroy 	obj-firefox/dist/include/Layers.h:118
6 	xul.dll 	mozilla::gfx::UserData::Destroy 	obj-firefox/dist/include/mozilla/gfx/UserData.h:100
7 	xul.dll 	mozilla::layers::BasicLayerManager::~BasicLayerManager 	gfx/layers/basic/BasicLayerManager.cpp:247
8 	xul.dll 	mozilla::layers::BasicLayerManager::`vector deleting destructor' 	
9 	xul.dll 	mozilla::layers::LayerManager::Release 	obj-firefox/dist/include/Layers.h:145
10 	xul.dll 	nsRefPtr<mozilla::FrameLayerBuilder::DisplayItemData>::~nsRefPtr<mozilla::FrameL 	obj-firefox/dist/include/nsAutoPtr.h:874
11 	xul.dll 	nsTArray<nsRefPtr<mozilla::FrameLayerBuilder::DisplayItemData>,nsTArrayDefaultAl 	obj-firefox/dist/include/nsTArray.h:945
12 	xul.dll 	nsTArray<nsRefPtr<mozilla::FrameLayerBuilder::DisplayItemData>,nsTArrayDefaultAl 	obj-firefox/dist/include/nsTArray.h:956
13 	xul.dll 	nsTArray<nsRefPtr<mozilla::FrameLayerBuilder::DisplayItemData>,nsTArrayDefaultAl 	obj-firefox/dist/include/nsTArray.h:442
14 	xul.dll 	mozilla::FrameLayerBuilder::DisplayItemDataEntry::~DisplayItemDataEntry 	layout/base/FrameLayerBuilder.h:597
15 	xul.dll 	mozilla::FrameLayerBuilder::DisplayItemDataEntry::`scalar deleting destructor' 	
16 	xul.dll 	nsTHashtable<mozilla::FrameLayerBuilder::DisplayItemDataEntry>::s_ClearEntry 	obj-firefox/dist/include/nsTHashtable.h:464
17 	xul.dll 	PL_DHashTableOperate 	obj-firefox/xpcom/build/pldhash.cpp:650
18 	xul.dll 	mozilla::FrameLayerBuilder::RemoveFrameFromLayerManager 	layout/base/FrameLayerBuilder.cpp:869
19 	xul.dll 	xul.dll@0x6f82f 	
20 	xul.dll 	mozilla::FramePropertyTable::PropertyValue::DestroyValueFor 	layout/base/FramePropertyTable.h:148
...

More reports at:
https://crash-stats.mozilla.com/report/list?signature=mozilla%3A%3AFrameLayerBuilder%3A%3AClearManagerData%28nsIFrame*%2C+mozilla%3A%3ALayerManagerData*%29
https://crash-stats.mozilla.com/report/list?signature=mozilla%3A%3AFrameLayerBuilder%3A%3AClearManagerData

Comment 1

[Scoobidiver (away)]

Loading the URL makes it crash.

Comment 2

[Semtex]

str.
1. Open Gmail
2. Reload page/tab
3. Nightly crash with: https://crash-stats.mozilla.com/report/index/bp-4b5c4460-7fa5-4d8a-a601-e67292120929

Same problem in save mode.

Comment 3

[Scoobidiver (away)]

There are currently 1652 crashes so far. It makes the latest nightly unusable.

Comment 4

[Daniel Holbert [:dholbert]]

(Based on crash report messages, it sounds like scrolling on Facebook & Google Plus will trigger this, too.)

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

I think it would be easier to fix this bug than back out.  Nightlies won't be spun for another 12 hours.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Backing out and triggering nightlies would be the right thing.
Of course if fixing is trivial, and the patch gets review asap we can land the patch and
retrigger nightlies.

Comment 7

[David Baron :dbaron:]

One way to debug this is to uncomment:
//#define DEBUG_TRACEMALLOC_PRESARENA 1
in nsPresArena.h, and then run valgrind on the testcase (scrolling Google Plus crashes for me).

That said, I think the problem may well be visible on the stack and that may not be necessary.

Comment 8

[David Baron :dbaron:]

(In reply to Olli Pettay [:smaug] from comment #6)
> Backing out and triggering nightlies would be the right thing.

For a patch complex enough that keeping it up-to-date against mozilla-central is a substantial percentage of a full-time job, I don't think that's necessarily the right thing.

Comment 9

[Jesse Ruderman]

I have a testcase in bug 795646 that crashes in a similar way.

Comment 10

[Matt Woodrow (:mattwoodrow)]

Attachment: Disable merged frame tracking

This patch disables the merged frame tracking code that lets us detect when the ordering of merged frames has changed, but the content drawn by the merged item should be the same. With this patch we'll just invalidate unnecessarily instead. It should be fairly rare, and not a huge issue.

This was a recent regression in DLBI, will try find the bug that added this code.


The actual crash is because our retained data tracking code assumes that all references to a frame live within a single subtree and we store a pointer to ancestor of this subtree. On frame deletion, we prune the subtree and all references to the deleted frame.

Storing merged frame data in other places broke this assumption, and we left dangling pointers.

I'll look into a way to fix this properly, but we should definitely take this for now.

Comment 11

[Matt Woodrow (:mattwoodrow)]

Sorry Chris, this will undo your work from bug 785333 :(

Comment 13

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 666263 [details] [diff] [review]
Disable merged frame tracking

I don't particularly like the #if 0 but it's fine since it's coming out soon.

Comment 14

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/mozilla-central/rev/85f561c755f6

Comment 15

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Matt, do you want to do the followup work to reorganize the display item key here or in a separate bug?

Comment 16

[Matt Woodrow (:mattwoodrow)]

Probably a follow-up, it's going to be a decent amount of work.

Comment 17

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

OK, please to be filing.

Comment 19

[Manuela Muntean [Away]]

I can't reproduce the crash with the STR from comment 2, nor the suggestions in comment 4, with Firefox 18 beta 2.

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/20100101 Firefox/18.0
Build ID: 20121128060531

While checking on Socorro, I haven't found any of the signatures on Firefox 18 beta 2.