641227 - Crash in mozilla::plugins::PluginInstanceParent::GetImage [@ mozilla::layers::MacIOSurfaceImageOGL::SetData ]

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Scoobidiver (away)]

It is a new crash signature that first appeared in 4.0b12pre/20110217.
It starts showing up as #16 top crasher on Mac OS X in 4.0 RC1.

Here are some comments:
"I was logging into my back via Mint.com"
"had two windows each with at least 4 tabs open reading a live text feed of the new iipad release, on twitter and tumblr, reading the huffington post"

Signature	mozilla::layers::MacIOSurfaceImageOGL::SetData
UUID	8c3ff73b-cb3a-4a2c-b03d-3ff432110311
Time 	2011-03-11 23:51:31.698713
Uptime	58310
Install Age	136967 seconds (1.6 days) since version was first installed.
Product	Firefox
Version	4.0
Build ID	20110303140758
Branch	2.0
OS	Mac OS X
OS Version	10.6.6 10J567
CPU	amd64
CPU Info	family 6 model 37 stepping 5
Crash Reason	EXC_BAD_ACCESS / KERN_INVALID_ADDRESS
Crash Address	0x0
User Comments	I was logging into my back via Mint.com
App Notes 	Renderers: 0x22600,0x24300,0x20400GL Context? GL Context+
GL Layers? GL Layers+

Frame 	Module 	Signature [Expand] 	Source
0 	XUL 	mozilla::layers::MacIOSurfaceImageOGL::SetData 	
1 	XUL 	mozilla::plugins::PluginInstanceParent::GetImage 	dom/plugins/PluginInstanceParent.cpp:628
2 	XUL 	nsNPAPIPluginInstance::GetImage 	modules/plugin/base/src/nsNPAPIPluginInstance.cpp:882
3 	XUL 	nsPluginInstanceOwner::SetCurrentImage 	layout/generic/nsObjectFrame.cpp:2000
4 	XUL 	nsObjectFrame::BuildLayer 	layout/generic/nsObjectFrame.cpp:1991
5 	XUL 	nsDisplayPlugin::BuildLayer 	layout/generic/nsObjectFrame.h:366
6 	XUL 	mozilla::::ContainerState::ProcessDisplayItems 	layout/base/FrameLayerBuilder.cpp:1271
7 	XUL 	mozilla::::ContainerState::ProcessDisplayItems 	layout/base/FrameLayerBuilder.cpp:1236
8 	XUL 	mozilla::FrameLayerBuilder::BuildContainerLayerFor 	layout/base/FrameLayerBuilder.cpp:1590
9 	XUL 	nsDisplayOwnLayer::BuildLayer 	layout/base/nsDisplayList.cpp:1688
10 	XUL 	mozilla::::ContainerState::ProcessDisplayItems 	layout/base/FrameLayerBuilder.cpp:1271
11 	XUL 	mozilla::::ContainerState::ProcessDisplayItems 	layout/base/FrameLayerBuilder.cpp:1236
12 	XUL 	mozilla::::ContainerState::ProcessDisplayItems 	layout/base/FrameLayerBuilder.cpp:1236
13 	XUL 	mozilla::::ContainerState::ProcessDisplayItems 	layout/base/FrameLayerBuilder.cpp:1236
14 	XUL 	mozilla::FrameLayerBuilder::BuildContainerLayerFor 	layout/base/FrameLayerBuilder.cpp:1590
15 	XUL 	nsDisplayList::PaintForFrame 	layout/base/nsDisplayList.cpp:516
16 	XUL 	nsLayoutUtils::PaintFrame 	layout/base/nsLayoutUtils.cpp:1577
17 	XUL 	PresShell::Paint 	layout/base/nsPresShell.cpp:6203
18 	XUL 	nsViewManager::RenderViews 	view/src/nsViewManager.cpp:458
19 	XUL 	nsViewManager::Refresh 	view/src/nsViewManager.cpp:424
20 	XUL 	nsViewManager::DispatchEvent 	view/src/nsViewManager.cpp:925
21 	XUL 	HandleEvent 	view/src/nsView.cpp:161
22 	XUL 	nsChildView::DispatchEvent 	widget/src/cocoa/nsChildView.mm:1807
23 	XUL 	nsChildView::DispatchWindowEvent 	widget/src/cocoa/nsChildView.mm:1817
24 	XUL 	-[ChildView drawRect:inContext:] 	widget/src/cocoa/nsChildView.mm:2874
25 	XUL 	-[ChildView drawRect:] 	widget/src/cocoa/nsChildView.mm:2780
26 	AppKit 	-[NSView _drawRect:clip:] 	
27 	AppKit 	-[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] 	
28 	AppKit 	-[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] 	
29 	AppKit 	-[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] 	
30 	AppKit 	-[NSThemeFrame _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] 	
31 	AppKit 	-[NSView _displayRectIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:] 	
32 	AppKit 	-[NSView displayIfNeeded] 	
33 	libSystem.B.dylib 	gettimeofday

More reports at:
https://crash-stats.mozilla.com/report/list?product=Firefox&range_value=4&range_unit=weeks&signature=mozilla%3A%3Alayers%3A%3AMacIOSurfaceImageOGL%3A%3ASetData

Comment 1

[Marcia Knous [:marcia]]

http://tinyurl.com/6znpenv links to the RC Crashes so far. Not much to go in comments except one person mentioning logging into mint.com.

Comment 2

[Scoobidiver (away)]

It is #12 top crasher on Mac OS X in 4.0.

A lot of comments talk about moving tabs.

Comment 3

[Josh Aas]

Can you look into this, Benoit?

Comment 4

[Benoit Girard (:BenWa)]

Attachment: Check for a null manager?

Perhaps we want to fail safe on having a null LayerManager? Or is having no longer manager an error that should itself be fixed?

Another possible cause for this crash could be that LookupSurface somehow fails, but I can't find a good reason why it would fail as we're just copying an existing surface. Wouldn't hurt to return a result code and fail safe if it were to fail.

http://mxr.mozilla.org/mozilla-central/source/gfx/layers/opengl/MacIOSurfaceImageOGL.mm#79

Comment 5

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 534188 [details] [diff] [review]
Check for a null manager?

At the least, this looks like a safe change that will hopefully fix the crash.

I'm not really sure what circumstances would cause the layer manager to be null. The only way I can see this happening is if the LayerManagerOGL instance gets destructed.

We always have a valid GL context at render time (ImageLayerOGL::RenderLayer) and context sharing is always enabled for mac, so we could delay creating and binding the textures until then if we don't have a manager.

I'd be happy to just land this fix as is, and do the rest as a follow-up if necessary.

Comment 6

[Benoit Girard (:BenWa)]

Comment on attachment 534188 [details] [diff] [review]
Check for a null manager?

Alright, this patch isn't risky and has a reasonable chance of fixing the crash.

Since this crash started in 4.0 what branches should we land the fix on?

Comment 7

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 534188 [details] [diff] [review]
Check for a null manager?

I believe you need to land it on m-c and wait for approval to get it landed on aurora/beta.

Comment 8

[Benjamin Smedberg]

Comment on attachment 534188 [details] [diff] [review]
Check for a null manager?

The aurora branch is dormant this week. For FF6, land on trunk. For FF5 you want beta approval.

Comment 9

[Daniel Veditz [:dveditz]]

Comment on attachment 534188 [details] [diff] [review]
Check for a null manager?

Approved for the mozilla-beta repository, a=dveditz for release-drivers

Comment 10

[LegNeato]

http://hg.mozilla.org/mozilla-central/rev/574b55afc991

Comment 11

[LegNeato]

http://hg.mozilla.org/releases/mozilla-aurora/rev/61c442066408

Comment 12

[LegNeato]

http://hg.mozilla.org/releases/mozilla-beta/rev/cb28ee0930cc

(note the file moved on trunk so I couldn't just transplant the patch to beta. I s/ipc\///g on the patch, applied it to mozilla-beta, check it, and pushed)