1699533 - fairly reproducible canvas-y crashes on https://lab.hakim.se/hypnos/

Status: VERIFIED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Mayank Bansal]

1. Use my usual latest Nightly profile
2. Go to https://lab.hakim.se/hypnos/
3. Try to duplicate the tab a couple of times by right click on tab-> Duplicate tab

Do this a couple of times. Then try closing the tabs one by one

AR: Either a "tab turns white" crash, or a silent background crash.
ER : not so

I get a variety of crash signatures. Posting a few.

Comment 1

[Mayank Bansal]

Attachment: about:upport

Comment 2

[Mayank Bansal]

https://crash-stats.mozilla.org/report/index/7c0cadec-f57f-4f64-ab8d-d73e80210318
https://crash-stats.mozilla.org/report/index/07ebec32-c419-4bf3-b5ba-a3d820210318
https://crash-stats.mozilla.org/report/index/929ab342-4856-4796-9fe0-931e70210318
https://crash-stats.mozilla.org/report/index/faeacede-63e5-4df2-af71-3fe640210318
https://crash-stats.mozilla.org/report/index/7a93942f-9fd9-4ed8-89bc-587350210318
https://crash-stats.mozilla.org/report/index/53d00e10-3c8c-4d01-bc92-6844f0210318
https://crash-stats.mozilla.org/report/index/909f77d2-2514-4f3b-9443-baf300210318

Comment 3

[Bob Owen (:bobowen)]

Hi Mayank, thanks for the bug, would you mind testing with: pref gfx.canvas.remote=false

Comment 4

[Mayank Bansal]

(In reply to Bob Owen (:bobowen) from comment #3)

Hi Mayank, thanks for the bug, would you mind testing with: pref gfx.canvas.remote=false

I do not crash with gfx.canvas.remote=false

Comment 5

[Mayank Bansal]

NB: these are not new crashes or new regression. I have been reproducing these crashes for atleast a month.

Comment 6

[Bob Owen (:bobowen)]

Thanks for the report.
I can also reproduce a crash on my old laptop with only Intel graphics, although the signature is different.
I wonder if we need to be using AutoSerializeWithMoz2D for uses of the Direct3D and DXGI APIs in the compositor and webrender threads, like we do in the canvas ones.

Essentially nothing has changed with remote canvas with respect to the Direct2D writing calls, except we are now processing the Direct2D calls in the same process.
So, the Direct2D device is in the same process, which might make the difference here.

Comment 7

[Bob Owen (:bobowen)]

https://treeherder.mozilla.org/jobs?repo=try&duplicate_jobs=visible&selectedTaskRun=aytJziTwQi-OtTKzAnqP7A.0&revision=1ab74f72f4ec6ab4ce6b6205c8b9e699f5204b83

Comment 8

[Bob Owen (:bobowen)]

Attachment: Bug 1699533: Push most uses of AutoSerializeWithMoz2D in canvas classes into D3D11TextureData. r=mattwoodrow!

This should ensure that we are more rigorous about always serializing when
required. For example in this crash it appears the issue is that we are not
using it when destroying the textures still held by the CanvasTranslator on its
destruction. It also means that we can minimize the code that is serialized,
which should reduce contention and possible deadlocks.

This patch also ensures that we drop any other references to the texture's
DrawTarget before dropping the texture, so that it can control the destruction
of its DrawTarget including any locking.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/95dbe830d8bf
Push most uses of AutoSerializeWithMoz2D in canvas classes into D3D11TextureData. r=mattwoodrow

Comment 10

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/95dbe830d8bf

Comment 11

[Bob Owen (:bobowen)]

Comment on attachment 9211574 [details]
Bug 1699533: Push most uses of AutoSerializeWithMoz2D in canvas classes into D3D11TextureData. r=mattwoodrow!

Beta/Release Uplift Approval Request

• User impact if declined: This stops a number of crashes within graphics drivers in the GPU process.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See description.
• List of other uplifts needed: None
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): This isn't a trivial change, so not putting low.
  However, it is mainly making the locking more consistent, i.e. doing what we already do, but covering places we've missed.
• String changes made/needed: None

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9211574 [details]
Bug 1699533: Push most uses of AutoSerializeWithMoz2D in canvas classes into D3D11TextureData. r=mattwoodrow!

Approved for 88.0b5.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/4d7a47f1cc1a

Comment 14

[Cornel Ionce [:noni] [Hubs QA]]

I've managed to reproduce this issue on Windows 10 using Firefox 87.0 using the STR from the description. After duplicating ~20 tabs and closing several of them, the page turned white.

Confirming that with Firefox 88.0b5 (build ID 20210330185720) and latest 89.0a1 Nightly (build ID 20210331215444) I don't see any white page / crash.
Tested on Windows 10x64 and Ubuntu 20.04.