crash in IUnknown::QueryInterface<IDXGISurface>(IDXGISurface**)

VERIFIED FIXED in Firefox 33

Status

()

defect
--
critical
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: bjacob, Assigned: nical)

Tracking

(Blocks 1 bug, {crash, topcrash-win})

Other Branch
mozilla35
x86
Windows NT
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox32 unaffected, firefox33+ verified, firefox34+ verified, firefox35+ verified)

Details

(crash signature)

Attachments

(1 attachment)

This bug was filed from the Socorro interface and is 
report bp-20bd23c1-d46a-4a45-9fe7-28f402140901.
=============================================================

This is the 5th top crasher for users of builds attempting D3D11 layers.
It's the #8 topcrash for Firefox 34.0a2 users right now, with 109/5958 crashes in the last 7 days. More reports: https://crash-stats.mozilla.com/report/list?product=Firefox&range_value=7&range_unit=days&date=2014-09-09&signature=IUnknown%3A%3AQueryInterface%3CIDXGISurface%3E%28IDXGISurface**%29&version=Firefox%3A34.0a2#tab-reports


Crashing thread from https://crash-stats.mozilla.com/report/index/e0d981c0-24d8-4593-8bcc-0bb9d2140905:

 0 	xul.dll 	IUnknown::QueryInterface<IDXGISurface>(IDXGISurface**) 	c:/program files (x86)/windows kits/8.0/include/um/Unknwnbase.h:131
1 	xul.dll 	mozilla::gfx::AutoSaveRestoreClippedOut::Save() 	gfx/2d/DrawTargetD2D.cpp
2 	xul.dll 	mozilla::gfx::DrawTargetD2D::ClearRect(mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits> const&) 	gfx/2d/DrawTargetD2D.cpp
3 	xul.dll 	mozilla::layers::RotatedBuffer::DrawBufferQuadrant(mozilla::gfx::DrawTarget*, mozilla::layers::RotatedBuffer::XSide, mozilla::layers::RotatedBuffer::YSide, mozilla::layers::RotatedBuffer::ContextSource, float, mozilla::gfx::CompositionOp, mozilla::gfx::SourceSurface*, mozilla::gfx::Matrix const*)

Comment 2

5 years ago
[Tracking Requested - why for this release]:
This is also one of our top issues on 33 beta and it's one of the GFX issues we need to solve in this cycle. It's the #4 crash on 33.0b right now.
Assignee

Comment 3

5 years ago
Bas, it be fairly easy to have release builds early return in AutoSaveRestoreClippedOut::Save if allocating the temporary texture failed, instead of crashing. we'd risk having slightly broken rendering but I assume that it's better than crashing. Does this sound good to you?
Flags: needinfo?(bas)
(In reply to Nicolas Silva [:nical] from comment #3)
> Bas, it be fairly easy to have release builds early return in
> AutoSaveRestoreClippedOut::Save if allocating the temporary texture failed,
> instead of crashing. we'd risk having slightly broken rendering but I assume
> that it's better than crashing. Does this sound good to you?

We sort of could.. but we're in a really bad situation if this happens anyway. The good thing is this code is going away with D2D 1.1, is there some correlation with driver versions/devices, etc.? Fwiw, this should only be happening to people with D2D, not for people with just D3D11, so this is even worse for them.
Flags: needinfo?(bas)
Assignee

Comment 5

5 years ago
(In reply to Bas Schouten (:bas.schouten) from comment #4)
> (In reply to Nicolas Silva [:nical] from comment #3)
> > Bas, it be fairly easy to have release builds early return in
> > AutoSaveRestoreClippedOut::Save if allocating the temporary texture failed,
> > instead of crashing. we'd risk having slightly broken rendering but I assume
> > that it's better than crashing. Does this sound good to you?
> 
> We sort of could.. but we're in a really bad situation if this happens
> anyway. 

It's part of this big family of "failed to allocate a D3D texture" bugs that are causing crashes in many different places since we enabled OMTC, so either there is one thing we are doing badly and I hope to find out soon, or it's just that we effectlively use more texture memory than we used too (double buffering, etc.) and there is very little we can do except try to recover from them...

> The good thing is this code is going away with D2D 1.1,

Man, this is good news. It's a sad little code path, I hope we don't run into it too often already.

Is D2D1.1 going to be uplifted, though? If this is a top crasher we need something to fix it (or paper over it) in the mean time.
Assignee

Comment 6

5 years ago
Not ideal but if this code is going away thanks to D2D1.1 soon-ish, it's better than crashing in the mean time.
Assignee: nobody → nical.bugzilla
Attachment #8487871 - Flags: review?(bas)
Attachment #8487871 - Flags: review?(bas) → review+
https://hg.mozilla.org/mozilla-central/rev/fb65a6ab2fd6
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Nical, could you fill an uplift request for aurora & beta? Merci!
Flags: needinfo?(nical.bugzilla)
Assignee

Comment 10

5 years ago
Comment on attachment 8487871 [details] [diff] [review]
Don't crash release builds when failing to allocate a surface in AutoRestoreClippedOut::save

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: Crashes on Windows.
[Describe test coverage new/current, TBPL]: baking in central. No specific tests because the issue only happens with some gpu drivers.
[Risks and why]: very low, trivial patch.
[String/UUID change made/needed]:
Attachment #8487871 - Flags: approval-mozilla-beta?
Attachment #8487871 - Flags: approval-mozilla-aurora?
Flags: needinfo?(nical.bugzilla)
Attachment #8487871 - Flags: approval-mozilla-beta?
Attachment #8487871 - Flags: approval-mozilla-beta+
Attachment #8487871 - Flags: approval-mozilla-aurora?
Attachment #8487871 - Flags: approval-mozilla-aurora+
Assignee

Updated

5 years ago
Duplicate of this bug: 1021179
Flags: qe-verify+
Crash stats indicate this is fixed across branches.
Marking as verified since it was verified on target milestone build.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.