1415754 - Crash in rx::d3d11::DynamicCastComObject<T>

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect, P1)

Description

[Marcia Knous [:marcia]]

This bug was filed from the Socorro interface and is
report bp-37dc5d77-f8c7-46f9-8dd3-fc6ab0171026.
=============================================================

Seen while looking at crash stats - http://bit.ly/2ztT45D. 10 crashes/8 installs. Crashes started using 20171025100449.

Top 10 frames of crashing thread:

0 libglesv2.dll rx::d3d11::DynamicCastComObject<ID3D11Texture2D> gfx/angle/src/libANGLE/renderer/d3d/d3d11/renderer11_utils.h:154
1 libglesv2.dll rx::SwapChain11::resetOffscreenColorBuffer gfx/angle/src/libANGLE/renderer/d3d/d3d11/SwapChain11.cpp:218
2 libglesv2.dll rx::SwapChain11::resetOffscreenBuffers gfx/angle/src/libANGLE/renderer/d3d/d3d11/SwapChain11.cpp:162
3 libglesv2.dll rx::SwapChain11::reset gfx/angle/src/libANGLE/renderer/d3d/d3d11/SwapChain11.cpp:644
4 libglesv2.dll rx::SurfaceD3D::resetSwapChain gfx/angle/src/libANGLE/renderer/d3d/SurfaceD3D.cpp:198
5 libglesv2.dll rx::SurfaceD3D::resetSwapChain gfx/angle/src/libANGLE/renderer/d3d/SurfaceD3D.cpp:155
6 libglesv2.dll rx::SurfaceD3D::initialize gfx/angle/src/libANGLE/renderer/d3d/SurfaceD3D.cpp:95
7 libglesv2.dll egl::Surface::initialize gfx/angle/src/libANGLE/Surface.cpp:141
8 libglesv2.dll egl::Display::createPbufferFromClientBuffer gfx/angle/src/libANGLE/Display.cpp:624
9 libglesv2.dll egl::CreatePbufferFromClientBuffer gfx/angle/src/libGLESv2/entry_points_egl.cpp:973

=============================================================

Comment 1

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P1: make OpenSharedResource() call become fallible in SwapChain11::resetOffscreenColorBuffer().

In crash report https://crash-stats.mozilla.com/report/index/37dc5d77-f8c7-46f9-8dd3-fc6ab0171026,
the OpenSharedResource() call was failed. And libangle just refers to this failed value
unconditionally in release build. I think that's why we can see this crash.
I check the minidump. The mHandle looks good. I don't know why the OpenSharedResource() failed here.

Comment 2

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P2: check the surface status before use in RenderDXGITextureHostOGL::EnsureLockable().

MozReview-Commit-ID: 4e0lyXsVOgT

Comment 3

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P3: check the mHandle status before using in RenderDXGITextureHostOGL::EnsureLockable().

The libangle will crash if we pass a 0 handle to CreatePbufferFromClientBuffer().

Comment 4

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Comment on attachment 8927766 [details] [diff] [review]
P1: make OpenSharedResource() call become fallible in SwapChain11::resetOffscreenColorBuffer().

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

I don't familiar with the libangle updating process, so I just try to have a local gecko patch here.
Is it worth to upstream to libangle?

Comment 5

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8927766 [details] [diff] [review]
P1: make OpenSharedResource() call become fallible in SwapChain11::resetOffscreenColorBuffer().

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

We should tell ANGLE about this, but we also shouldn't be using this entrypoint here.

::: gfx/angle/src/libANGLE/renderer/d3d/d3d11/SwapChain11.cpp
@@ +215,5 @@
>                                                          (void **)&tempResource11);
> +
> +            if (FAILED(result))
> +            {
> +                ERR() << "Could not open shared handle. hr:" << result;

hr:<space>"

Comment 6

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8927767 [details] [diff] [review]
P2: check the surface status before use in RenderDXGITextureHostOGL::EnsureLockable().

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

::: gfx/webrender_bindings/RenderD3D11TextureHostOGL.cpp
@@ +86,4 @@
>          LOCAL_EGL_MIPMAP_TEXTURE, LOCAL_EGL_FALSE,
>          LOCAL_EGL_NONE
>      };
>      mSurface = egl->fCreatePbufferFromClientBuffer(egl->Display(),

We really shouldn't be using this entrypoint here. We should be handling the opening of the shared surface ourselves, pulling out a D3D11Texture, and using the EGLStreamProducer for it.

You should only use this entrypoint when you need a renderable surface.

I see below that we use the EGLStream API already for NV12 textures. It should be a nice cleanup to merge most of these branches.

Comment 7

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8927767 [details] [diff] [review]
P2: check the surface status before use in RenderDXGITextureHostOGL::EnsureLockable().

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

Wait, I see this is marked 58:unaffected. If so, we should take this opportunity to stop using this entrypoint here.

Comment 8

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P4: try to use egl stream instead of CreatePbufferFromClientBuffer() for d3d rgb format texture.

MozReview-Commit-ID: Bydbxtx3oGU

Comment 9

[Kelsey Gilbert [:jgilbert]]

I'll review this tomorrow.

Comment 10

[Jerry Shih[:jerry] (UTC+8) (inactive)]

try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a093cefc79e4e579c4e9285cc5e4c67c10427d45

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3884192fe88a
make OpenSharedResource() call become fallible in SwapChain11::resetOffscreenColorBuffer(). r=jgilbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/f18d59f20d01
check the surface status before use in RenderDXGITextureHostOGL::EnsureLockable(). r=jgilbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/695cd84f9d4c
check the mHandle status before using in RenderDXGITextureHostOGL::EnsureLockable(). r=jgilbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac50a38689a0
try to use egl stream instead of CreatePbufferFromClientBuffer() for d3d rgb format texture. r=jgilbert

Comment 12

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/3884192fe88a
https://hg.mozilla.org/mozilla-central/rev/f18d59f20d01
https://hg.mozilla.org/mozilla-central/rev/695cd84f9d4c
https://hg.mozilla.org/mozilla-central/rev/ac50a38689a0

Comment 13

[Kelsey Gilbert [:jgilbert]]

I'll check if this needs to be upstreamed.

Comment 14

[Kelsey Gilbert [:jgilbert]]

Upstreaming:
https://bugs.chromium.org/p/angleproject/issues/detail?id=2367
https://bugs.chromium.org/p/angleproject/issues/detail?id=2368
https://bugs.chromium.org/p/angleproject/issues/detail?id=2369