Closed Bug 1415754 Opened 2 years ago Closed 2 years ago

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

Categories

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

Unspecified
Windows 10
defect

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox-esr52 --- unaffected
firefox57 --- unaffected
firefox58 --- unaffected
firefox59 --- fixed

People

(Reporter: marcia, Assigned: jerry)

References

(Blocks 2 open bugs)

Details

(Keywords: crash, Whiteboard: [wr-reserve])

Crash Data

Attachments

(4 files)

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&lt;ID3D11Texture2D&gt; 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

=============================================================
Whiteboard: [wr-mvp] [triage]
Assignee: nobody → hshih
Status: NEW → ASSIGNED
Priority: P2 → P1
Whiteboard: [wr-mvp] [triage] → [wr-reserve]
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.
Attachment #8927766 - Flags: review?(jgilbert)
The libangle will crash if we pass a 0 handle to CreatePbufferFromClientBuffer().
Attachment #8927769 - Flags: review?(jgilbert)
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?
Blocks: 1388995
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>"
Attachment #8927766 - Flags: review?(jgilbert) → review+
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.
Attachment #8927767 - Flags: review?(jgilbert) → review+
Attachment #8927769 - Flags: review?(jgilbert) → review+
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.
Attachment #8927767 - Flags: review+ → review-
I'll review this tomorrow.
Attachment #8930420 - Flags: review?(jgilbert) → review+
Pushed by hshih@mozilla.com:
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
I'll check if this needs to be upstreamed.
Flags: needinfo?(jgilbert)
Flags: needinfo?(jgilbert)
You need to log in before you can comment on or make changes to this bug.