crash in mozilla::layers::RotatedContentBuffer::BeginPaint(mozilla::layers::PaintedLayer*, unsigned int)

VERIFIED FIXED in Firefox 36

Status

()

--
critical
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: JasnaPaka, Assigned: milan, NeedInfo)

Tracking

(Blocks: 1 bug, {crash, crashreportid})

unspecified
mozilla38
x86_64
Windows 7
crash, crashreportid
Points:
---
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox36+ verified, firefox37 verified, firefox38 verified)

Details

(crash signature, URL)

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

5 years ago
This bug was filed from the Socorro interface and is 
report bp-ded58434-e081-476f-84ab-09ce52141013.
=============================================================
Random crash on Facebook.
Severity: normal → critical
Component: Layout → Graphics: Layers
Keywords: crashreportid

Comment 1

4 years ago
[Tracking Requested - why for this release]:

This signature seems to have roughly increased by a factor 5 or 6 when 36 landed on Beta (250-300 crashes per million ADI) compared to where it was in 35 (~50 crashes / 1M ADI).

In early 36.0b4 data, it's ~1.4% of all browser crashes.

See https://crash-stats.mozilla.com/report/list?signature=mozilla%3A%3Alayers%3A%3ARotatedContentBuffer%3A%3ABeginPaint%28mozilla%3A%3Alayers%3A%3APaintedLayer%2A%2C%20unsigned%20int%29 for more info.
tracking-firefox36: --- → ?
Matt, do you think you could help on this? Thanks
status-firefox36: --- → affected
tracking-firefox36: ? → +
Flags: needinfo?(matt.woodrow)
I'm a bit busy with media stuff at the moment, Milan do you have anyone that can look at this?

The crash reports shows a bunch d2d/d3d allocation failures with the error code 0x8007000e (Out of memory or system resources).

The crash is because we failed to allocate destDTBufferOnWhite, and we don't check for this anywhere. We do however check for allocation failures of destDTBuffer and avoid crashing there. Fixing that and avoiding this particular crash is an easy fix.

The fact that this has spiked is more worrying, since it suggests that we're using more memory in general. Fixing this particular crash might just move crashes to a different callsite, or in the best case result in broken rendering.

I'm not sure what would have changed in 36 to cause this, is that the first version with d2d 1.1?
Flags: needinfo?(matt.woodrow) → needinfo?(milan)
I'll follow up.  Yes, D2D1.1 is in 36, but some of the reports are "just" D2D, and don't have the failed allocations.  We've also run into scenarios where the out of memory message comes from calls that shouldn't even be able to return them, so it'll be interesting.
This is admittedly a wallpaper patch, but if it stops the crash, it may be worth it.
Flags: needinfo?(milan)
Attachment #8557156 - Flags: review?(matt.woodrow)
Let's get more information when we fail.
Attachment #8557156 - Attachment is obsolete: true
Attachment #8557156 - Flags: review?(matt.woodrow)
Attachment #8557175 - Flags: review?(matt.woodrow)
Comment on attachment 8557175 [details] [diff] [review]
(wallpaper) patch to check null pointer. r=mattwoodrow

Review of attachment 8557175 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/RotatedBuffer.cpp
@@ +648,5 @@
>              destBufferRect = ComputeBufferRect(neededRegion.GetBounds());
>              CreateBuffer(result.mContentType, destBufferRect, bufferFlags,
>                           &destDTBuffer, &destDTBufferOnWhite);
> +            // Continue if we're missing buffer on white, but return if
> +            // we're missing the destination buffer.

We should return here if we don't have the buffer on white, there's not much point in drawing the other one without it.
Crash Signature: [@ mozilla::layers::RotatedContentBuffer::BeginPaint(mozilla::layers::PaintedLayer*, unsigned int)] → [@ mozilla::layers::RotatedContentBuffer::BeginPaint(mozilla::layers::PaintedLayer*, unsigned int)] [@ xul.dll@0x6adfa0 | xul.dll@0x255e34 | xul.dll@0x3b8d7c | xul.dll@0x21ef27b | xul.dll@0x3c0003 | xul.dll@0x3c0338 | xul.dll@0x36a379 | xul.dll@0x36a21b …
Blocks: 778617
With review comments.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4a61d323e4a6
Attachment #8557175 - Attachment is obsolete: true
Attachment #8557175 - Flags: review?(matt.woodrow)
Attachment #8558125 - Flags: review?(matt.woodrow)
Comment on attachment 8558125 [details] [diff] [review]
(wallpaper) patch to check null pointer. r=mattwoodrow

Review of attachment 8558125 [details] [diff] [review]:
-----------------------------------------------------------------

I wouldn't bother with MOZ_UNLIKELY, branch prediction should handle this fine and be simpler to read.
Attachment #8558125 - Flags: review?(matt.woodrow) → review+
Without MOZ_UNLIKELY, claiming the try run is still valid.
Attachment #8558125 - Attachment is obsolete: true
Attachment #8558172 - Flags: review+

Comment 11

4 years ago
I was reviewing this bug and looking at how I could get our outsource graphics folk to help reproduce this in their second run and I noticed something. You note above that it is a memory related crash. This crash did not spike with 36b1 or 36b2. It spiked with 36b3. And we landed the MSE support which has known memory issues in 36b3. 

There is nothing video-specific about the URLs though, except that they are avenues to video but they are also avenues to images, and games etc: It's mostly a facebook crash. But it also happens on twitter and youtube.

Most of the comments do not state anything about watching video either. But what we are seeing is that we leak memory while watching video. So, it would make sense to crash later in some unrelated area of intensive processing after the video watching has increased our memory footprint.

This ^ at least is my theory.

Sheila & Marcia, I think we should track this with the memory related issues in MSE and see if it's rate of occurrence decreases as those fixes roll out. Sheila NI'ing you to add this to your tracking  please.
Flags: needinfo?(smooney)
Assignee: nobody → milan
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/6105b754be10
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-firefox38: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Comment on attachment 8558172 [details] [diff] [review]
(wallpaper) patch to check null pointer. Carry r=mattwoodrow

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: This is 1.4% of crashes on Beta.
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: We're avoiding dereferencing a null pointer, so it's possible we'll crash elsewhere.
[String/UUID change made/needed]: n/a
Attachment #8558172 - Flags: approval-mozilla-beta?
Attachment #8558172 - Flags: approval-mozilla-aurora?
Attachment #8558172 - Flags: approval-mozilla-beta?
Attachment #8558172 - Flags: approval-mozilla-beta+
Attachment #8558172 - Flags: approval-mozilla-aurora?
Attachment #8558172 - Flags: approval-mozilla-aurora+
Flags: qe-verify+
Socorro stats over the past 4 weeks show:
[1] - [@ mozilla::layers::RotatedContentBuffer::BeginPaint(mozilla::layers::PaintedLayer*, unsigned int)]
* Firefox 36 - 1 crash in Beta 9 + 2 crashes in RC (build1)
* Firefox 37 - 0 crashes in builds after February 4th
* Firefox 38 - 1 crash in builds after February 3rd
[2] - [@ xul.dll@0x6adfa0 | xul.dll@0x255e34 | xul.dll@0x3b8d7c | xul.dll@0x21ef27b | xul.dll@0x3c0003...]
* all reported crashes are on Firefox 36 Beta 5

[1] - https://crash-stats.mozilla.com/report/list?product=Firefox&range_unit=days&range_value=28&signature=mozilla%3A%3Alayers%3A%3ARotatedContentBuffer%3A%3ABeginPaint%28mozilla%3A%3Alayers%3A%3APaintedLayer%2A%2C+unsigned+int%29#tab-reports
[2] - https://crash-stats.mozilla.com/report/list?product=Firefox&range_unit=days&range_value=28&signature=xul.dll%400x6adfa0+%7C+xul.dll%400x255e34+%7C+xul.dll%400x3b8d7c+%7C+xul.dll%400x21ef27b+%7C+xul.dll%400x3c0003+%7C+xul.dll%400x3c0338+%7C+xul.dll%400x36a379+%7C+xul.dll%400x36a21b+%7C+xul.dll%400x22b349+%7C+xul.dll%400x3c12b3+%7C+xul.dll%400x9bc74#tab-sigsummary
Status: RESOLVED → VERIFIED
status-firefox36: fixed → verified
status-firefox37: fixed → verified
status-firefox38: fixed → verified
You need to log in before you can comment on or make changes to this bug.