Closed Bug 1262390 Opened 7 years ago Closed 6 years ago

crash in mozilla::gfx::DrawTargetD2D1::GetImageForLayerContent

Categories

(Core :: Graphics, defect)

Product:
Core
Component:
Graphics
Version:
45 Branch
Platform:
x86
Windows
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla54
Tracking Flags:
Tracking Status
firefox46 - wontfix
firefox47 - wontfix
firefox48 --- wontfix
firefox49 --- wontfix
firefox50 --- wontfix
firefox51 --- wontfix
firefox52 --- fixed
firefox-esr52 --- fixed
firefox53 --- fixed
firefox54 --- fixed

People

(Reporter: philipp, Assigned: bas.schouten)

References

Details

(Keywords: crash, Whiteboard: [gfx-noted])

Crash Data

Attachments

(1 file)

Bug 1262390: In some cases, we fail with the small allocation because of the device reset situation. .schouten
59 bytes, text/x-review-board-request
bas.schouten
: review+
gchang
: approval-mozilla-aurora+
jcristau
: approval-mozilla-beta+
Details 
[Tracking Requested - why for this release]:
request tracking, this is a new signature regressing in 46. it is a rather low volume crash until now though.

This bug was filed from the Socorro interface and is 
report bp-be57b035-6289-4d44-a5fd-2e7172160406.
=============================================================
Crashing Thread (0)
Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::gfx::DrawTargetD2D1::GetImageForLayerContent() 	gfx/2d/DrawTargetD2D1.cpp
1 	xul.dll 	mozilla::gfx::DrawTargetD2D1::FinalizeDrawing(mozilla::gfx::CompositionOp, mozilla::gfx::Pattern const&) 	gfx/2d/DrawTargetD2D1.cpp
2 	xul.dll 	mozilla::gfx::DrawTargetD2D1::DrawSurface(mozilla::gfx::SourceSurface*, mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float> const&, mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float> const&, mozilla::gfx::DrawSurfaceOptions const&, mozilla::gfx::DrawOptions const&)

new crash, currently in windows 7 and higher which appears to happen in code that landed with bug 1220629.
Assignee: nobody → bas
From what I can tell in these reports the creation call is E_OUTOFMEMORY. Which judging by the available virtual memory and taking into account fragmentation, this might actually be the case.
I don't believe this is a new crash.  When bug 1220629 landed, what used to be a DrawTargetD2D1::FinalizeDrawing() crash became this instead.  Can we see if there is a similar reduction in those "old" crashes to match this?
Keywords: regression
Whiteboard: [gfx-noted]
It looks the mozilla::gfx::DrawTargetD2D1::FinalizeDrawing signature went almost non-existent after 46.0b5 while the signature in here actually started rising with 46.0b5 (was existent at a lower level before). If what comment #2 says is true then I wonder why in 46.0b5, both seem to have been relatively high - but otherwise it could be true.
Too late for a fix for 46, we are about to build beta 11. We should check back in 47 though once 47 is in beta.
status-firefox46: affected → wontfix
tracking-firefox46: ? → -
tracking-firefox47: --- → ?
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #3)
> It looks the mozilla::gfx::DrawTargetD2D1::FinalizeDrawing signature went
> almost non-existent after 46.0b5 while the signature in here actually
> started rising with 46.0b5 (was existent at a lower level before). If what
> comment #2 says is true then I wonder why in 46.0b5, both seem to have been
> relatively high - but otherwise it could be true.

That's an excellent question.  Seems the old FinalizeDrawing one had at least two crashes in it, or at least there is a new flavour of the FinalizeDrawing crash now, that isn't covered by the GetImageForLayerContent.  The other direction is true though - what is now GetImageForLayerContent crash used to be FinalizeDrawing crash before.

I just opened bug 1264736 for the "other" FinalizeDrawing crashes.
I see a total of 20 instances of this crash signature for 47.0b1. That is not high volume at all. I'd like to untrack this for Fx47. If there is a fix ready before 47 goes live, I can consider uplifting to Beta47 pending risk assessment of the patch.
tracking-firefox47: ? → -
This has gone down from averaging ~200 crashes/day in June to ~130 crashes/day in July and is currently sitting at #145 or 0.10% in Firefox 47.0.1. I suspect this has more to do with declining ADIs: 

> 06-01: 92,996,085 ADI / 195 crashes => 2.097 crashes per 1M users
> 06-15: 95,288,346 ADI / 181 crashes => 1.899 crashes per 1M users
> 07-01: 84,763,508 ADI / 137 crashes => 1.616 crashes per 1M users
> 07-15: 84,171,159 ADI / 133 crashes => 1.580 crashes per 1M users
ADI -9.49%, Crashes -31.79%, Crash rate -24.65%
status-firefox47: affected → wontfix
status-firefox49: --- → affected
OS: Windows NT → Windows
Summary: crash in mozilla::gfx::DrawTargetD2D1::GetImageForLayerContent in Firefox 46 → crash in mozilla::gfx::DrawTargetD2D1::GetImageForLayerContent
Crash volume for signature 'mozilla::gfx::DrawTargetD2D1::GetImageForLayerContent':
 - nightly (version 50): 7 crashes from 2016-06-06.
 - aurora  (version 49): 29 crashes from 2016-06-07.
 - beta    (version 48): 770 crashes from 2016-06-06.
 - release (version 47): 7092 crashes from 2016-05-31.
 - esr     (version 45): 0 crashes from 2016-04-07.

Crash volume on the last weeks:
            W. N-1  W. N-2  W. N-3  W. N-4  W. N-5  W. N-6  W. N-7
 - nightly       2       0       1       0       2       1       1
 - aurora        1       4       2       6       3       7       5
 - beta        106     100      88      82      97     143     113
 - release    1080     831     892     714    1035    1078    1048
 - esr           0       0       0       0       0       0       0

Affected platform: Windows
status-firefox50: --- → affected
Crash volume for signature 'mozilla::gfx::DrawTargetD2D1::GetImageForLayerContent':
 - nightly (version 51): 2 crashes from 2016-08-01.
 - aurora  (version 50): 5 crashes from 2016-08-01.
 - beta    (version 49): 122 crashes from 2016-08-02.
 - release (version 48): 512 crashes from 2016-07-25.
 - esr     (version 45): 0 crashes from 2016-05-02.

Crash volume on the last weeks (Week N is from 08-22 to 08-28):
            W. N-1  W. N-2  W. N-3
 - nightly       1       0       0
 - aurora        2       1       0
 - beta         43      47      10
 - release     153     170      72
 - esr           0       0       0

Affected platform: Windows

Crash rank on the last 7 days:
           Browser   Content     Plugin
 - nightly #837      #617
 - aurora  #1560     #223
 - beta    #450      #850
 - release #130      #139
 - esr
status-firefox51: --- → affected
Crash volume for signature 'mozilla::gfx::DrawTargetD2D1::GetImageForLayerContent':
 - nightly (version 52): 2 crashes from 2016-09-19.
 - aurora  (version 51): 5 crashes from 2016-09-19.
 - beta    (version 50): 30 crashes from 2016-09-20.
 - release (version 49): 420 crashes from 2016-09-05.
 - esr     (version 45): 0 crashes from 2016-06-01.

Crash volume on the last weeks (Week N is from 10-03 to 10-09):
            W. N-1  W. N-2
 - nightly       1       1
 - aurora        4       1
 - beta         27       3
 - release     353      67
 - esr           0       0

Affected platform: Windows

Crash rank on the last 7 days:
           Browser   Content     Plugin
 - nightly #1105
 - aurora            #256
 - beta    #551      #1472
 - release #188      #161
 - esr
status-firefox52: --- → affected
This has spiked back up to ~300/day.  Some crashes definitely have a high memory usage, but some don't - https://crash-stats.mozilla.com/report/index/686623f9-25ac-4027-baf9-7234a2170219 although the device reset plays the part.  The error, D2DERR_RECREATE_TARGET, feels like should be just handled, in the sense that we return without crashing and let the device reset take care of it.
Comment on attachment 8839276 [details]
Bug 1262390: In some cases, we fail with the small allocation because of the device reset situation. .schouten

https://reviewboard.mozilla.org/r/113950/#review115610
Attachment #8839276 - Flags: review?(bas) → review+
Pushed by msreckovic@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/efbbe123d5af
In some cases, we fail with the small allocation because of the device reset situation. r=bas.schouten
https://hg.mozilla.org/mozilla-central/rev/efbbe123d5af
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment on attachment 8839276 [details]
Bug 1262390: In some cases, we fail with the small allocation because of the device reset situation. .schouten

Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]: High volume crash on beta+release.
[Is this code covered by automated tests?]: We haven't seen these crashes in automated tests.
[Has the fix been verified in Nightly?]: No reproducible STR, and no recent crashes on nightly, so difficult to verify.  This is the main reason for uplifting to Aurora. While the volume of crashes on Aurora is low, it isn't zero, as it is on Nightly.
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: Low risk change, null pointer check.
[Why is the change risky/not risky?]: The calling function handles failure by failing itself, and it wasn't the only place where it can fail, so its callers should be ready for it.
[String changes made/needed]: n/a
Attachment #8839276 - Flags: approval-mozilla-aurora?
Comment on attachment 8839276 [details]
Bug 1262390: In some cases, we fail with the small allocation because of the device reset situation. .schouten

Fix a crash and recent crashes on nightly. Aurora53+.
Attachment #8839276 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Not sure if we're going to have enough data to make a call on this for 52 before Monday's RC, but let's keep this on the radar for future dot-release and ESR consideration.
status-firefox-esr52: --- → affected
Comment on attachment 8839276 [details]
Bug 1262390: In some cases, we fail with the small allocation because of the device reset situation. .schouten

I talked to Milan about this on IRC. It's unlikely this patch will make things any worse, so it's probably better to just get it landed on 52 and keep an eye out for any fallout.
Attachment #8839276 - Flags: approval-mozilla-beta?
Comment on attachment 8839276 [details]
Bug 1262390: In some cases, we fail with the small allocation because of the device reset situation. .schouten

avoid a crash on gfx device reset, beta52+

Should be in 52.0rc1
Attachment #8839276 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.