1086670 - crash in @0x0 | mozilla::layers::RotatedContentBuffer::BeginPaint(mozilla::layers::ThebesLayer*, unsigned int)

Status: RESOLVED WORKSFORME

(Core :: Graphics: Layers, defect)

Description

[Liz Henry (:lizzard) (relman/hg->git project)]

This bug was filed from the Socorro interface and is 
report bp-f4c555ab-d455-468b-8ca4-9c0fe2141020.
=============================================================
This is the #7 topcrash for Firefox 34.0b1 with 968/36533 crashes in the last 7 days. They are all on the 20141014134955 build.  

Crashing thread:

0 		@0x0 	
1 	xul.dll 	mozilla::layers::RotatedContentBuffer::BeginPaint(mozilla::layers::ThebesLayer*, unsigned int) 	gfx/layers/RotatedBuffer.cpp
2 	xul.dll 	mozilla::layers::ContentClientRemoteBuffer::BeginPaintBuffer(mozilla::layers::ThebesLayer*, unsigned int) 	gfx/layers/client/ContentClient.h
3 	xul.dll 	mozilla::layers::ClientThebesLayer::PaintThebes() 	gfx/layers/client/ClientThebesLayer.cpp
4 	xul.dll 	mozilla::layers::ClientThebesLayer::RenderLayerWithReadback(mozilla::layers::ReadbackProcessor*) 	gfx/layers/client/ClientThebesLayer.cpp
5 	xul.dll 	mozilla::layers::ClientContainerLayer::RenderLayer() 	gfx/layers/client/ClientContainerLayer.h
6 	xul.dll 	mozilla::layers::Layer::ApplyPendingUpdatesForThisTransaction() 	gfx/layers/Layers.cpp
7 	xul.dll 	mozilla::layers::ClientLayerManager::EndTransactionInternal(void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) 	gfx/layers/client/ClientLayerManager.cpp

Comment 1

[Milan Sreckovic [:milan] (needinfo for best results)]

:nical, is this similar/same to the one you recently dealt with?

Comment 2

[Nicolas Silva [:nical]]

(In reply to Milan Sreckovic [:milan] from comment #1)
> :nical, is this similar/same to the one you recently dealt with?

Looks like a new one.

Comment 3

[Nicolas Silva [:nical]]

So at the crash site we are dereferencing a null pointer to one of the two DrawTargets of a component alpha pair. as it turns out we managed to create the first texture and not the second one. The stack doesn't provide any information about what caused the failure to create the second DrawTarget. Could be OOM, or maybe something else.
If we just early return, the content of the layer will not be rendered or updated properly which could make the browser glitchy or unusable. If we just crash there, well we have a lot of crashes.

I am looking into adding more fatal assertions in the code that is involved, to see if it turns out to be an error in code that is called beforehand. If it's OOM we don't have a good way to fix it in the short term so we may want to just glitch rather than crash (not sure, although we already do if we fail to create the first texture of the pair).

Comment 4

[Milan Sreckovic [:milan] (needinfo for best results)]

Let's add gfxCriticalError() as well. If it's OOM situation, glitchy may be better than crashing, especially if it's a temporary situation.

Comment 5

[Nicolas Silva [:nical]]

Attachment: don't crash release builds if allocating the buffer on white fails, add some debug assertions and gfxCriticalError logs

Comment 7

[Nicolas Silva [:nical]]

Bas, can I haz a review?

Comment 8

[Nicolas Silva [:nical]]

Attachment: updated patch

This patch doesn't attempt to paper over the issue, but adds some logging that will help with the investigation.

Comment 9

[Nicolas Silva [:nical]]

Attachment: Same patch without MOZ_ASSERT when there is gfxCriticalError

Up to you.
At the moment we don't assert in gfxCriticalError but we'll most probably do soon and I am more interested in getting the gfxCriticalError data in the app notes.

Comment 10

[Nicolas Silva [:nical]]

Landed the patch without the MOZ_ASSERTS https://hg.mozilla.org/integration/mozilla-inbound/rev/ef55c94657dc

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ef55c94657dc

Comment 12

[Nicolas Silva [:nical]]

Interestingly nothing in the appnotes since the patch landed

https://crash-stats.mozilla.com/search/?signature=~mozilla%3A%3Alayers%3A%3ARotatedContentBuffer%3A%3ABeginPaint&version=35.0a2&build_id=%3E20141120000000&_facets=signature&_columns=version&_columns=build_id&_columns=app_notes

If we don't restrict to build ids containing the patch, there are some E_OUTOFMEMORY D2D errors in the app notes but not as many as I expected.

Comment 13

[Nicolas Silva [:nical]]

Attachment: Even more gfxCritical error logging, this time with D2D1.1

I was silly enough to forget D2D 1.1 in the previous patch, so lots of possible interesting code paths wouldn't have shown in the app notes. This patch will log if we fail to create a D2D1.1 DrawTarget.

Comment 14

[Nicolas Silva [:nical]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f34d8f041add

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f34d8f041add

Comment 16

[Milan Sreckovic [:milan] (needinfo for best results)]

No crashes with this signature since 34?

Comment 17

[Robert Kaiser]

(In reply to Milan Sreckovic [:milan] from comment #16)
> No crashes with this signature since 34?

It's entirely possible that it migrated to bug 1081911 which is still high in the latest 36 beta.

Comment 18

[Robert Kaiser]

This isn't happening in post-34, so I'm closing the bug as WFM.