1279136 - Crash in @0x0 | CEnum::GetAdapterDisplayModeImpl

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P1)

Description

[Nicholas Nethercote [inactive]]

This bug was filed from the Socorro interface and is 
report bp-7c498652-daae-4db2-bb67-b4a342160602.
=============================================================

1,368 crashes with this signature in the past 7 days. All on Windows 7. Looks like problems starting D3D9.

Top five versions:

> 1  38.8.0esr  828  60.53 %
> 2  46.0.1     129   9.43 %
> 3  45.1.1esr  117   8.55 %
> 4  45.1.0esr  69    5.04 %
> 5  43.0       65    4.75 %

Top five CPUs:

> 1   GenuineIntel family 6 model 61 stepping 4 | 4   631   46.13 %
> 2   GenuineIntel family 6 model 58 stepping 9 | 4   309   22.59 %
> 3   GenuineIntel family 6 model 60 stepping 3 | 4   134    9.80 %
> 4   GenuineIntel family 6 model 69 stepping 1 | 4   106    7.75 %
> 5   GenuineIntel family 6 model 42 stepping 7 | 4    86    6.29 %

dvander, any ideas?

Comment 1

[Nicholas Nethercote [inactive]]

All the crashes have Intel graphic adapters.

Comment 2

[Milan Sreckovic [:milan] (needinfo for best results)]

Video related, probably a duplicate of one of the bugs being handled as we speak.

Comment 3

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

There are a lot of driver versions affected, all Intel.

Comment 4

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #3)
> There are a lot of driver versions affected, all Intel.

That could be the result of the signature itself; something like this, on Nvidia, looks to be the same issue: https://crash-stats.mozilla.com/report/index/b02af673-32e2-4c3b-a510-b03c52160623

Comment 5

[Milan Sreckovic [:milan] (needinfo for best results)]

Interestingly enough, when calling CreateDeviceEx, we ran afoul of this:

"(Parameter hFocusWindow) - The focus window alerts Direct3D when an application switches from foreground mode to background mode. For full-screen mode, the window specified must be a top-level window. For windowed mode, this parameter may be NULL only if the hDeviceWindow member of pPresentationParameters is set to a valid, non-NULL value..."

From what I can tell, in https://dxr.mozilla.org/mozilla-central/source/dom/media/platforms/wmf/DXVA2Manager.cpp#308, we have both hFocusWindow parameter (third parameter to CreateDeviceEx), as well as hDeviceWindow in params set to nullptr.

But my knowledge of this code is 10 minutes old, so I'm probably misreading it.

Comment 6

[Milan Sreckovic [:milan] (needinfo for best results)]

And, yes, we have a comment in our code that says "we're doing this even though the documentation says we shouldn't, but so is Chrome, so we should be OK".  Still, these crashes look like they're a result of a nullptr...

Comment 7

[Milan Sreckovic [:milan] (needinfo for best results)]

Given that we crash in GetAdapterDisplayMode, and we probably only call that because we pass D3DFMT_UNKNOWN, I wonder what would happen if we called GetAdapterDisplayMode ourselves, then pass the result to CreateDevice.  If we'd crash in GetAdapterDisplayMode when we make the call ourselves.  It would at least give us some more information.

Comment 8

[Milan Sreckovic [:milan] (needinfo for best results)]

Or even more interestingly, call GetDisplayMode instead.

Comment 9

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

The offending change was to make accelerated decoding work under e10s.

Comment 10

[Matt Woodrow (:mattwoodrow)]

It's not obvious that GetAdapterDisplayMode would depend on the window, so I don't see how that could crash.

It's quite believable able that passing D3DFMT_UNKNOWN is why it's being called though, so we could just try passing a normal value (doesn't particularly matter, we never actually present with this device) to see if that helps.

I double checked chrome and they seem to be passing exactly the same parameters (including D3DFMT_UNKNOWN), though they sometimes skip this code entirely and use the device contained within ANGLE. It's not clear how often each code path runs, so maybe this isn't actually affecting them.

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

Crash volume for signature '@0x0 | CEnum::GetAdapterDisplayModeImpl':
 - nightly (version 50): 0 crash from 2016-06-06.
 - aurora  (version 49): 1 crash from 2016-06-07.
 - beta    (version 48): 20 crashes from 2016-06-06.
 - release (version 47): 640 crashes from 2016-05-31.
 - esr     (version 45): 5921 crashes from 2016-04-07.

Crash volume on the last weeks:
             Week N-1   Week N-2   Week N-3   Week N-4   Week N-5   Week N-6   Week N-7
 - nightly          0          0          0          0          0          0          0
 - aurora           1          0          0          0          0          0          0
 - beta             4          1          2          4          3          4          2
 - release         98         80        100         96         86        101         20
 - esr           1049        604        574        724        723        678        377

Affected platform: Windows

Comment 12

[Matt Woodrow (:mattwoodrow)]

Attachment: d3d9-create-device-crash

It seems pretty reasonable to believe that creating the device with D3DFMT_UNKNOWN is what is triggering the driver to call GetAdapterDisplayMode to figure it out, and then that crashes. The docs explicitly say that if BackBufferFormat is set to D3DFMT_UNKNOWN then it will be modified to contain the actual format during the call to CreateDevice(Ex).

We don't have similar crashes with the primary d3d9 device, and that uses ARGB as the format.

We never actually present anything with this device, we only use it for video decoding so the format shouldn't matter.

Comment 13

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8777635 [details] [diff] [review]
d3d9-create-device-crash

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

Please use mozreview. It's easier on reviewers.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e283da610b05
Specify a format for our D3D9 video device to avoid the driver needing to compute one. r=cpearce

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e283da610b05

Comment 16

[Milan Sreckovic [:milan] (needinfo for best results)]

The good news is, this crash seems to have gone away on nightly.  The bad news - the last build that has the crash is 20160802, so if we fixed it, it wasn't this that fixed it :)  If I had to guess, I'd guess bug 1289640 took care of it.

Comment 17

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Matt, Milan, which fix should we consider uplifting to Aurora50? This or bug 1289640 or both?

Comment 18

[Milan Sreckovic [:milan] (needinfo for best results)]

Pretty sure this one didn't move the needle, but it seems like a safe enough fix.  Bug 1289640 is likely the one that did take care of it, but that's a larger change, and there are still some open questions (e.g., bug 1289640 comment 21), so I'm not sure we want the uplift quite yet.  Tough spot - let's see what Matt thinks.

Comment 19

[Matt Woodrow (:mattwoodrow)]

I definitely don't want to uplift bug 1289640 yet, probably not at all. As Milan said, way too many open questions.

We can uplift this, but if it hasn't made much difference then there isn't much point.

Comment 20

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

(In reply to Matt Woodrow (:mattwoodrow) from comment #19)
> I definitely don't want to uplift bug 1289640 yet, probably not at all. As
> Milan said, way too many open questions.
> 
> We can uplift this, but if it hasn't made much difference then there isn't
> much point.

Ok. Thanks Milan and Matt. I agree, this is now a wontfix for 49 and 50.