1160295 - DisplayLink (dlumd10.dll, dlumd11.dll) Startup crash in @0x0 | CContext::UMQueryHS_ConstBuf_(D3D10DDI_HRTCORELAYER, unsigned int, unsigned int)

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

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

+++ This bug was initially created as a clone of Bug #1021265 +++

This bug was filed from the Socorro interface and is 
report bp-7553c1b0-b28e-45a2-837d-494972140530.
=============================================================

See https://bugzilla.mozilla.org/show_bug.cgi?id=1021265#c109 in particular.  We want to also look for "dlumd10.dll" and "dlumd11.dll", not just "dlumd32.dll".

Comment 1

[[:philipp]]

this startup crash is spiking after the firefox 40 update

Comment 2

[[:philipp]]

[Tracking Requested - why for this release]:
also setting the tracking flag to get this startup crash on the radar of relman, since this signature is #8 on windows in early crash data for firefox 40

Comment 3

[Sylvestre Ledru [:Sylvestre]]

Startup crash... tracking.
Milan, Jeff, is it something actionable? thanks

Comment 4

[(Away)]

We may need a different bug. The proposal in comment 0 would not fix this issue. The reports on v40 are not about dlumd10 or dlumd11.

The reports we're seeing have dlumd32.dll, and igd10umd32.dll, and their DisplayLink driver version is <= V(8,6,1,36484). The block in DoesD3D11DeviceWork ought to have kicked in on these systems.

The only thing I can imagine is that these machines satisfied this check:

  if (gfxPrefs::Direct2DForceEnabled() ||
      gfxPrefs::LayersAccelerationForceEnabled())

Jeff/Milan does my reasoning make sense? What can we do, if anything?

Comment 5

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

The block in DoesD3D11DeviceWork did kick in, at least on a few crashes I looked at, because we got the "[GFX1-]:DisplayLink: too old version" message which signifies this.  Which probably means it isn't the force/force situation above.  I think we have a bug in the initialization, will take a look.

Comment 6

[(Away)]

Ah, good call to check the error annotations!

displaylink    85 	100.00 %
old            85 	100.00 %
too            85 	100.00 %

Comment 7

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

Bug 1107299 removed some of the code paths from bug 1021265; given the spike in 40, this isn't a direct regression from that bug, but need to trace code a bit more.

Comment 8

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

I think I can reproduce this locally.

Comment 9

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

I haven't searched for when, but we have regressed bug 1021265 completely.  When I simulate "bad DisplayLink driver" locally, I still end up in accelerated everything.  I'll get a trunk patch shortly.

Comment 10

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

Attachment: Actually do something if DisplayLink driver is of a bad version. r=jrmuizel

Comment 11

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

Comment on attachment 8647045 [details] [diff] [review]
Actually do something if DisplayLink driver is of a bad version. r=jrmuizel

Bad version of DisplayLink driver leads to a startup crash.  This got fixed in bug 1021265, and then got unfixed somewhere along the way.

Comment 12

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8647045 [details] [diff] [review]
Actually do something if DisplayLink driver is of a bad version. r=jrmuizel

Preapproving it. Please land that as soon as Jeff approved the patch (hopefully, he will)

Comment 13

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

Attachment: Beta patch. r=jrmuizel

Comment 14

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

Attachment: Aurora patch. r=jrmuizel

Comment 15

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

The "beta" and "aurora" patches are based on what's currently mozilla-beta and mozilla-aurora.  Right around the merge, I don't know what that actually is.

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

None of these patches appear to apply to mozilla-release.

Comment 17

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c67e052b3db8

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c67e052b3db8
https://hg.mozilla.org/releases/mozilla-aurora/rev/29300721bb12
https://hg.mozilla.org/releases/mozilla-beta/rev/c4eeb37d22d7

Comment 19

[Jeff Muizelaar [:jrmuizel]]

Attachment: release-patch

Comment 20

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-release/rev/81353f2735a4

Comment 21

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Is there any way to manually verify this fix?

Comment 22

[Sylvestre Ledru [:Sylvestre]]

Milan, can you verify that?

Comment 23

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

When testing, I was simulating the problem and the fix in a local build by forcing the "you have a bad version of  a DisplayLink driver" even though I don't even have that driver.

To test on the installed version, one would need to find and install an old DisplayLink driver, we don't have a spoofing setup for these.  "Old version" is defined as older or same as version 8.6.1.36484

Comment 24

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

Oh, and the test is - without this fix, Firefox would crash, and if it doesn't, the about:support would be showing D3D11 as the compositor of choice.  With the fix, no crash, compositor of choice Basic OMTC.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

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

Comment 26

[u279076]

Milan and I talked about this in our 1:1. We were wondering if this could get some unit test coverage.

Comment 27

[[:philipp]]

sorry to bug again, but this is still showing up in 40.0.2 data, for example:
https://crash-stats.mozilla.com/report/index/64931a12-486a-491d-9eca-62c6a2150817
https://support.mozilla.org/en-US/questions/1078464

Comment 28

[(Away)]

(In reply to philipp from comment #27)
> sorry to bug again, but this is still showing up in 40.0.2 data, for example:
> https://crash-stats.mozilla.com/report/index/64931a12-486a-491d-9eca-
> 62c6a2150817
> https://support.mozilla.org/en-US/questions/1078464

dlumd11.dll 	8.6.1.35912 	ACA742FA10574AD0859528BEB55A278D1 	dlumd32.pdb

Now we are back to the issue for which this bug was actually opened:

(In reply to Milan Sreckovic [:milan] from comment #0)
> We want to also look for "dlumd10.dll" and "dlumd11.dll", not just
> "dlumd32.dll".

Comment 29

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

Good point - I opened another bug 1195844, this one got overloaded with the regression fix.

Comment 30

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

See https://bugzilla.mozilla.org/show_bug.cgi?id=1195844#c27

Comment 31

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

The patch above doesn't quite do it.

We need to find out if we have a bad version of DisplayLink driver DLLs before we make a call to create the D3D11 device.  The way the code worked before this patch was to create the D3D11 device, then check the DisplayLink versions - by then it was too late, as we were crashing in the D3D11 device creation.  The patch changed it to check those versions before making the call toe D3D11 device creation.  However, the versions are checked on loaded DisplayLink DLLs, and it is the D3D11 device creation that currently loads those DLLs, so at the time that we want to check, those DLLs are not loaded.

Comment 32

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

Attachment: Part 2. Make sure DLLs are loaded before we check the version.

Looking at the crashes, they were not in the creation, but later, so this change should solve the "asking for DLL versions too early".

Comment 33

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8657152 [details] [diff] [review]
Part 2. Make sure DLLs are loaded before we check the version.

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

Let's do this more by reverting the original and fixing things differently.

Comment 34

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

Comment on attachment 8657152 [details] [diff] [review]
Part 2. Make sure DLLs are loaded before we check the version.

Talked with Jeff, I'll get a patch that backs out the original change and just does minimal diff (from what the code looked like before this bug) instead.

Comment 35

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

Attachment: Part 2. Make sure DLLs are loaded before we check the version. r=bas, jrmuizel

Comment 36

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

OK, let's checkin Part 2 on nightly, and see over the weekend if crashes go away.

Comment 37

[(Away)]

(In reply to Milan Sreckovic [:milan] from comment #36)
> OK, let's checkin Part 2 on nightly, and see over the weekend if crashes go away.

This crash is pretty much never seen on nightly, and very rarely on aurora. We'll likely need a beta build.

Comment 38

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

I'll prepare a beta patch and if this stays on nightly I'll ask for the uplift.

Comment 39

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

Attachment: Part 2. The beta patch equivalent.

Comment 40

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

Attachment: Part 2. The aurora patch equivalent.

Comment 41

[Carsten Book [:Tomcat]]

hey Milan, is there a try run for this ?

Comment 42

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

There wasn't one.  Here's one now: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e9e218424deb

Comment 43

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/60cc73ec4053

Comment 44

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/60cc73ec4053

Comment 45

[Liz Henry (:lizzard) (relman/hg->git project)]

Wes did the patches for the other branches also land (but just not show up here?) They are marked as fixed in the tracking flags but that may be leftover from the earlier landing.

Comment 46

[Liz Henry (:lizzard) (relman/hg->git project)]

Milan does this still need uplift? Thanks!

Comment 47

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I haven't uplifted anything from this bug today.

Comment 48

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

If this needs to be uplifted to Beta41 and Aurora42, we need to reset status-ff41 and status-ff42 to affected. Otherwise it will not show up in RelEng's queries.

Comment 49

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

Comment on attachment 8657262 [details] [diff] [review]
Part 2. The beta patch equivalent.

Approval Request Comment
What the Part 1 was aiming to fix but didn't quite manage.

Comment 50

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

Comment on attachment 8657264 [details] [diff] [review]
Part 2. The aurora patch equivalent.

Approval Request Comment
What the Part 1 was aiming to fix but didn't quite manage.

Comment 51

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

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #46)
> Milan does this still need uplift? Thanks!

Yes, we want this uplifted.

Comment 52

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8657262 [details] [diff] [review]
Part 2. The beta patch equivalent.

Let's try with this patch.

Comment 53

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

Comment on attachment 8657264 [details] [diff] [review]
Part 2. The aurora patch equivalent.

looks like a simple crash fix, let's uplift to Beta41.

Comment 54

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/c47eb9288bab

Comment 55

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/3c6b3284e7cc

Comment 56

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

Let's keep checking if beta 9 has any of these.

Comment 57

[[:philipp]]

it's looking very promising so far, no more reports on 41.0b9 - thank you!