Closed Bug 1154003 Opened 9 years ago Closed 9 years ago

crash in DrawingContext::FillRectangle(D2D_RECT_F const*, ID2D1Brush*)

Categories

(Core :: Graphics, defect)

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

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla40
Tracking Flags:
Tracking Status
firefox38 + fixed
firefox38.0.5 --- fixed
firefox39 + fixed
firefox40 + fixed

People

(Reporter: milan, Assigned: bas.schouten)

References

Details

(Keywords: crash, topcrash)

Crash Data

Attachments

(1 file, 1 obsolete file)

More protection when driver resets. Carry r=bas
2.97 KB, patch
milan
: review+
Sylvestre
: approval-mozilla-aurora+
Sylvestre
: approval-mozilla-release+
Details | Diff | Splinter Review 
+++ This bug was initially created as a clone of Bug #1089454 +++

More reports are coming in with the same signature as tracked in bug 1089454, which are really "we don't recover well from driver resets", so it's somewhat expected.  Here are the new items after the bug was marked fixed, from the last few comments in bug 1089454:

---------------
I encountered this crash https://crash-stats.mozilla.com/report/index/ccb67422-af23-43fc-960f-915862150401 during my testing around loop tab share, on Firefox 38 Beta 1 (20150330154247) using Windows 8.1 32bit.
---------------
Crash on http://reshade.me/
https://crash-stats.mozilla.com/report/index/1fb536a4-8832-41e2-ac5c-955112150411
AMD driver restart after crash.  This crash on FF 38 beta3.
---------------
[Tracking Requested - why for this release]: We tracked this for 36 and 37, in the ongoing battle of recovering from driver resets.
Assignee: nobody → bas
tracking-firefox38: --- → ?
tracking-firefox39: --- → ?
gfxSurfaceDrawable::DrawWithSamplingRect protects against calling gfxSurfaceDrawable::DrawInternal with null mSourceSurface.  gfxSurfaceDrawable::Draw does not, so you can get through and that's likely the crash?
Comment on attachment 8591873 [details] [diff] [review]
More protection when driver resets. r=bas

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

I'm a little worried about this as we'll end up drawing nothing.. and we might not know we're drawing nothing with this patch.. which may be subtly worse than crashing. But for now let's at least see how this does on nightly.
Attachment #8591873 - Flags: review?(bas) → review+
(In reply to Bas Schouten (:bas.schouten) from comment #4)
> Comment on attachment 8591873 [details] [diff] [review]
> More protection when driver resets. r=bas
> 
> Review of attachment 8591873 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'm a little worried about this as we'll end up drawing nothing.. and we
> might not know we're drawing nothing with this patch.. which may be subtly
> worse than crashing. But for now let's at least see how this does on nightly.

Right - I was hoping we can survive long enough for things to reset and start working again, but I don't know if just time and surviving longer would actually help that happen.
Tracking as it is a critical issue.
status-firefox38: --- → affected
status-firefox39: --- → affected
status-firefox40: --- → affected
tracking-firefox38: ? → +
tracking-firefox39: ? → +
tracking-firefox40: --- → +
There was a missing include in the reviewed patch.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c37a5172e1fe
Attachment #8591873 - Attachment is obsolete: true
Attachment #8592953 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/82dd5f118d14
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox40: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Milan, is it possible to have an uplift request to aurora & beta? thanks
Flags: needinfo?(milan)
Comment on attachment 8592953 [details] [diff] [review]
More protection when driver resets. Carry r=bas

Approval Request Comment

This is a continuing battle from driver resets.  Are we seeing the occurrence of these crashes go down in 40?  It was a bit of a speculative fix.  I'm also a bit worried about the comment 4, we don't want to replace crashes with "nothing is being drawn", but the idea with this patch was to survive a bit longer for the system to recover from those driver resets.
Flags: needinfo?(milan)
Attachment #8592953 - Flags: approval-mozilla-beta?
Attachment #8592953 - Flags: approval-mozilla-aurora?
Comment on attachment 8592953 [details] [diff] [review]
More protection when driver resets. Carry r=bas

[Triage Comment]
Let's take the chance. If we don't take it, we won't know the impact...
Can betabreakers check that for us?
Flags: needinfo?(milan)
Flags: needinfo?(lmandel)
Attachment #8592953 - Flags: approval-mozilla-release+
Attachment #8592953 - Flags: approval-mozilla-beta?
Attachment #8592953 - Flags: approval-mozilla-aurora?
Attachment #8592953 - Flags: approval-mozilla-aurora+
Passing the ni buck to ashughes
Flags: needinfo?(lmandel) → needinfo?(anthony.s.hughes)
Don't think we saw these crashes reported by betabreakers.
Flags: needinfo?(milan)
(In reply to Sylvestre Ledru [:sylvestre] from comment #13)
> Can betabreakers check that for us?

In theory, if we had steps that triggered a driver reset *and* reproduced the bugs we're seeing without any doubt. Unfortunately, I am unaware of any such steps.
Flags: needinfo?(anthony.s.hughes)
(In reply to Milan Sreckovic [:milan] from comment #15)
> Don't think we saw these crashes reported by betabreakers.
I know it wasn't but we could ask for their help in such bug.
(In reply to Sylvestre Ledru [:sylvestre] from comment #20)
> (In reply to Milan Sreckovic [:milan] from comment #15)
> > Don't think we saw these crashes reported by betabreakers.
> I know it wasn't but we could ask for their help in such bug.

In my opinion that's going to be no more viable and far less efficient than simply asking for help from Mozilla QA or Softvision. I can go into details privately if you'd like to discuss this further.
Just to clarify what I've said in comment 21 and 19, I think what you're hoping for is that Betabreakers can take some of the pain out of rolling the dice in these types of situations but that's not really what they're designed to do. Throwing them at this type of problem isn't going to make that pill any easier to swallow.

Again, happy to discuss this further outside of this bug report.
This happens randomly on websites which contains html5 or flash videos.
https://crash-stats.mozilla.com/report/index/80f9e177-1cf7-476a-9e8f-91c342150504
Don't think this really fixed all of these.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to Bas Schouten (:bas.schouten) from comment #26)
> Don't think this really fixed all of these.

Unless we're backing out the changes I think we should address other cases in a new bug report.
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #27)
> (In reply to Bas Schouten (:bas.schouten) from comment #26)
> > Don't think this really fixed all of these.
> 
> Unless we're backing out the changes I think we should address other cases
> in a new bug report.

I'm fine with that :). No, I don't propose backing these changes out.
(In reply to Bas Schouten (:bas.schouten) from comment #28)
> (In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #27)
> > (In reply to Bas Schouten (:bas.schouten) from comment #26)
> > > Don't think this really fixed all of these.
> > 
> > Unless we're backing out the changes I think we should address other cases
> > in a new bug report.
> 
> I'm fine with that :). No, I don't propose backing these changes out.

Would you mind cloning the bug? I'm not 100% clear on the details of the edge cases and guess that you may be able to articulate it better than I.
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
(In reply to Stas Denisov from comment #30)
> Crashes every time when I hit play http://www.youtube.com/embed/YaNz4DlGuqw
> (html5)
> https://crash-stats.mozilla.com/report/index/a35c5505-708b-4a7a-9aa8-
> 8a0f62150507

Hrm, that's a pretty messed up stack. That crash report also has no indications of a device reset, that's a very odd one. But it's interesting that it happens on a specific YouTube video, could you send some more of your reports? Also, did you notice anything like your screen flashing or a graphics driver reset notification around the bottom left of your screen?
Blocks: 1162520
I cloned this to bug 1162520. Please move all future conversation about these crashes to there.
(In reply to Bas Schouten (:bas.schouten) from comment #31)
> Also, did you notice anything like your screen flashing or a
> graphics driver reset notification around the bottom left of your screen?

Yes, browser freezes and after a few seconds black screen appears. Display driver stopped responding and has recovered.
(In reply to Stas Denisov from comment #33)
> (In reply to Bas Schouten (:bas.schouten) from comment #31)
> > Also, did you notice anything like your screen flashing or a
> > graphics driver reset notification around the bottom left of your screen?
> 
> Yes, browser freezes and after a few seconds black screen appears. Display
> driver stopped responding and has recovered.

As mentioned earlier, please move this information to bug 1162520. There's nothing more to be done in this bug report.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: