Closed Bug 1348320 Opened 3 years ago Closed 2 years ago

Crash in igd10umd32.dll | CContext::ID3D11DeviceContext1_UpdateSubresource_<T>

Categories

(Core :: Graphics: Layers, defect, critical)

x86
Windows 7
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- wontfix
firefox55 --- fixed

People

(Reporter: jseward, Assigned: bas.schouten)

References

(Depends on 1 open bug)

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-ce21c9d1-82c5-4ded-98f0-709a92170317.
=============================================================

This is ranked #12 for the Aurora crashes of 20170316004004.
Flags: needinfo?(dvander)
Flags: needinfo?(dvander)
David, did we ever figure out what this was?  It's also showing up as an increase in overall Intel crashes in bug 1359416
Flags: needinfo?(dvander)
bug 1359416 might be device reset related, though it's not clear why or whether the device is stale or newly-bogus. (Or if we're doing something that induces device resets).

The crashes here look like they might be device reset related too though it's hard to tell. Some of the reports have spam about previous DXVA crashes.
Flags: needinfo?(dvander)
I'm guessing this will fix the crash. It may seem like a bad idea to grab a lock that can block the compositor. But realistically this is an upload which the compositor would otherwise just end up doing during the transaction anyway if we wouldn't allow for content side uploads. So you might be changing timings a little bit but wall-clock you're unlikely to take more time in the compositor thread. Also when working correctly the drivers likely internally do locking to prevent two threads uploading through the context at the same time anyway. Let me know if you disagree, Jeff.
Assignee: nobody → bas
Flags: needinfo?(jmuizelaar)
Bas it looks like this crash only ever happens in the UI process, and never in the GPU process. (Maybe this code doesn't run when video decoding is OOP?). It also exclusively happens on Windows 6.2.9200 and lower - I think that's Windows 8. Could we restrict the lock further to only activate under those two conditions?
Comment on attachment 8863284 [details]
Bug 1348320: Use UpdateSubResource on crashy intel device/OS version combinations.

Hrm, good point David. This is different contention than I thought. This patch will likely not fix the problem. But I might have an idea for something else that will.
Attachment #8863284 - Flags: review?(jmuizelaar)
Attachment #8863284 - Attachment is obsolete: true
Comment on attachment 8863284 [details]
Bug 1348320: Use UpdateSubResource on crashy intel device/OS version combinations.

https://reviewboard.mozilla.org/r/135078/#review138888
Attachment #8863284 - Flags: review?(jmuizelaar) → review+
Pushed by bschouten@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c427691fb309
Use UpdateSubResource on crashy intel device/OS version combinations. r=jrmuizel
https://hg.mozilla.org/mozilla-central/rev/c427691fb309
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Please request Beta approval on this when you get a chance.
Flags: needinfo?(jmuizelaar) → needinfo?(bas)
We have already flipped a pref on Beta that should prevent this crash from occurring at the expense of some performance. Has this patch dropped our actual crash rate?
Flags: needinfo?(bas) → needinfo?(ryanvm)
I believe philipp answered your question in bug 1359416.
Flags: needinfo?(ryanvm)
(In reply to Bas Schouten (:bas.schouten) from comment #14)
> We have already flipped a pref on Beta that should prevent this crash from
> occurring at the expense of some performance. Has this patch dropped our
> actual crash rate?

WDYT about philipp's answer in bug 1359416 comment 27?
Flags: needinfo?(bas)
(In reply to Nathan Froyd [:froydnj] from comment #16)
> (In reply to Bas Schouten (:bas.schouten) from comment #14)
> > We have already flipped a pref on Beta that should prevent this crash from
> > occurring at the expense of some performance. Has this patch dropped our
> > actual crash rate?
> 
> WDYT about philipp's answer in bug 1359416 comment 27?

I say we just let it ride the trains. The video perf improvements this caused will still make it into 57, no need to add risk into the 55 cycle.
Flags: needinfo?(bas)
See Also: → 1373937
See Also: → 1377545
Depends on: 1426400
You need to log in before you can comment on or make changes to this bug.