1371546 - Intermittent short.mp4.lastframe.html,test_WaitingToEndedTransition_mp4.html,test_selftest.html | application crashed [@ mozilla::wr::RenderTextureHost::~RenderTextureHost]

Status: RESOLVED FIXED

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

Description

[Treeherder Bug Filer]

Filed by: philringnalda [at] gmail.com

https://treeherder.mozilla.org/logviewer.html#?job_id=105691170&repo=mozilla-central

https://queue.taskcluster.net/v1/task/et2tFYknTPOFx3tEDtR7HA/runs/0/artifacts/public/logs/live_backing.log

https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=https://queue.taskcluster.net/v1/task/et2tFYknTPOFx3tEDtR7HA/runs/0/artifacts/public/logs/live_backing.log&only_show_unexpected=1

Comment 1

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Regression from bug 1366502 because these failures are after:

[task 2017-06-09T02:13:08.194291Z] 02:13:08     INFO - Assertion failure: RenderThread::IsInRenderThread(), at /home/worker/workspace/build/src/gfx/webrender_bindings/RenderTextureHost.cpp:19
[task 2017-06-09T02:13:08.194595Z] 02:13:08     INFO - #01: mozilla::wr::RenderBufferTextureHost::~RenderBufferTextureHost [gfx/webrender_bindings/RenderBufferTextureHost.cpp:44]
[task 2017-06-09T02:13:08.194675Z] 02:13:08     INFO - 
[task 2017-06-09T02:13:08.194877Z] 02:13:08     INFO - #02: mozilla::wr::RenderTextureHost::Release [gfx/webrender_bindings/RenderTextureHost.h:22]
[task 2017-06-09T02:13:08.195008Z] 02:13:08     INFO - 
[task 2017-06-09T02:13:08.195270Z] 02:13:08     INFO - #03: mozilla::detail::RunnableMethodImpl<mozilla::wr::RenderThread*, void (mozilla::wr::RenderThread::*)(RefPtr<mozilla::wr::RenderTextureHost>), true, (mozilla::RunnableKind)0u, RefPtr<mozilla::wr::RenderTextureHost> >::~RunnableMethodImpl [xpcom/threads/nsThreadUtils.h:1109]
[task 2017-06-09T02:13:08.196019Z] 02:13:08     INFO - 
[task 2017-06-09T02:13:08.197962Z] 02:13:08     INFO - #04: mozilla::detail::RunnableMethodImpl<mozilla::wr::RenderThread*, void (mozilla::wr::RenderThread::*)(RefPtr<mozilla::wr::RenderTextureHost>), true, (mozilla::RunnableKind)0u, RefPtr<mozilla::wr::RenderTextureHost> >::~RunnableMethodImpl [xpcom/threads/nsThreadUtils.h:1109]
[task 2017-06-09T02:13:08.198497Z] 02:13:08     INFO - 
[task 2017-06-09T02:13:08.198642Z] 02:13:08     INFO - #05: mozilla::Runnable::Release [xpcom/threads/nsThreadUtils.cpp:44]
[task 2017-06-09T02:13:08.199134Z] 02:13:08     INFO - 
[task 2017-06-09T02:13:08.199699Z] 02:13:08     INFO - #06: MessageLoop::RunTask [ipc/chromium/src/base/message_loop.cc:364]
[task 2017-06-09T02:13:08.200055Z] 02:13:08     INFO - 
[task 2017-06-09T02:13:08.200658Z] 02:13:08     INFO - #07: MessageLoop::DeferOrRunPendingTask [ipc/chromium/src/base/message_loop.cc:369]
[task 2017-06-09T02:13:08.201255Z] 02:13:08     INFO - 
[task 2017-06-09T02:13:08.201800Z] 02:13:08     INFO - #08: MessageLoop::DoWork [ipc/chromium/src/base/message_loop.cc:444]
[task 2017-06-09T02:13:08.202183Z] 02:13:08     INFO - 
[task 2017-06-09T02:13:08.202972Z] 02:13:08     INFO - #09: base::MessagePumpDefault::Run [ipc/chromium/src/base/message_pump_default.cc:37]
[task 2017-06-09T02:13:08.203488Z] 02:13:08     INFO - 
[task 2017-06-09T02:13:08.203951Z] 02:13:08     INFO - #10: MessageLoop::RunInternal [ipc/chromium/src/base/message_loop.cc:239]
[task 2017-06-09T02:13:08.204534Z] 02:13:08     INFO - 
[task 2017-06-09T02:13:08.205133Z] 02:13:08     INFO - #11: MessageLoop::Run [ipc/chromium/src/base/message_loop.cc:505]
[task 2017-06-09T02:13:08.205632Z] 02:13:08     INFO - 
[task 2017-06-09T02:13:08.206242Z] 02:13:08     INFO - #12: base::Thread::ThreadMain [ipc/chromium/src/base/thread.cc:183]
[task 2017-06-09T02:13:08.206605Z] 02:13:08     INFO - 
[task 2017-06-09T02:13:08.211478Z] 02:13:08     INFO - #13: ThreadFunc [ipc/chromium/src/base/platform_thread_posix.cc:40]
[task 2017-06-09T02:13:08.212689Z] 02:13:08     INFO - 
[task 2017-06-09T02:13:08.214103Z] 02:13:08     INFO - #14: libpthread.so.0 + 0x76ba
[task 2017-06-09T02:13:08.215413Z] 02:13:08     INFO - 
[task 2017-06-09T02:13:08.216454Z] 02:13:08     INFO - #15: libc.so.6 + 0x10682d
[task 2017-06-09T02:13:08.217296Z] 02:13:08     INFO - 
[task 2017-06-09T02:13:08.219899Z] 02:13:08     INFO - #16: ??? (???:???)

Comment 2

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

Vincent, could you please check this?

Comment 3

[Vincent Liu[:vliu]]

After looked into this issue, I believe the crash *not* relativing to running ~RenderTextureHost() in the wrong thread.

     [task 2017-06-09T02:13:08.194291Z] 02:13:08     INFO - Assertion failure: RenderThread::IsInRenderThread(), at /home/worker/workspace/build/src/gfx/webrender_bindings/RenderTextureHost.cpp:19

The thing is, when FF called RenderThread::IsInRenderThread(), it would call sRenderThread->mThread->thread_id() to retrieve thread id to check. 
But if FF called it after RenderThread::ShutDown() was called, we would hit crash because ShutDown() sets sRenderThread to nullptr.

The below try link[1] can reproduce this case. 

[1]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f25368d09b3261e4127ea0fa1c28aa5fe354e5fd&selectedJob=107545730

In it, some information can explain this situation.

[task 2017-06-16T04:36:02.649403Z] 04:36:02     INFO - REFTEST SUITE-END | Shutdown

....
....
....

[task 2017-06-16T04:36:05.398931Z] 04:36:05     INFO -  RenderTextureHost::~RenderTextureHost() CurrentPlatformId=1047
[task 2017-06-16T04:36:05.398974Z] 04:36:05     INFO -  RenderThread::ShutDown()
[task 2017-06-16T04:36:05.399308Z] 04:36:05     INFO -  RenderThread::DeferredRenderTextureHostDestroy aTexture=0x7f8540dfd680 Compositor=0 Main=0 renderer=0 sRenderThread=(nil)
[task 2017-06-16T04:36:05.399389Z] 04:36:05     INFO - Assertion failure: mRawPtr, at /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/StaticPtr.h:141
[task 2017-06-16T04:36:05.399963Z] 04:36:05     INFO - [1017] WARNING: NS_ENSURE_TRUE(maybeContext) failed: file /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp, line 1037
[task 2017-06-16T04:36:05.400331Z] 04:36:05     INFO - [1017] WARNING: NS_ENSURE_TRUE(maybeContext) failed: file /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp, line 1037
[task 2017-06-16T04:36:05.400408Z] 04:36:05     INFO - #01: mozilla::wr::RenderThread::DeferredRenderTextureHostDestroy [gfx/webrender_bindings/RenderThread.cpp:329]
[task 2017-06-16T04:36:05.400452Z] 04:36:05     INFO - 
[task 2017-06-16T04:36:05.400825Z] 04:36:05     INFO - #02: mozilla::detail::RunnableMethodImpl<mozilla::wr::RenderThread*, void (mozilla::wr::RenderThread::*)(RefPtr<mozilla::wr::RenderTextureHost>), true, (mozilla::RunnableKind)0u, RefPtr<mozilla::wr::RenderTextureHost> >::Run [xpcom/threads/nsThreadUtils.h:1084]
[task 2017-06-16T04:36:05.400877Z] 04:36:05     INFO - 
[task 2017-06-16T04:36:05.400932Z] 04:36:05     INFO - #03: MessageLoop::RunTask [mfbt/RefPtr.h:62]
[task 2017-06-16T04:36:05.400965Z] 04:36:05     INFO - 
[task 2017-06-16T04:36:05.401029Z] 04:36:05     INFO - #04: MessageLoop::DeferOrRunPendingTask [ipc/chromium/src/base/message_loop.cc:369]

Back to original reported try link, it is easy to see the same information. 

[task 2017-06-09T02:13:06.728686Z] 02:13:06     INFO - REFTEST INFO | Known problems: 124 (78 known fail, 0 known asserts, 14 random, 32 skipped, 0 slow)
[task 2017-06-09T02:13:06.729216Z] 02:13:06     INFO - REFTEST SUITE-END | Shutdown
[task 2017-06-09T02:13:06.734321Z] 02:13:06     INFO - REFTEST INFO | Slowest test took 16523ms (file:///home/worker/workspace/build/tests/reftest/tests/editor/reftests/caret_on_positioned.html)
[task 2017-06-09T02:13:06.736145Z] 02:13:06     INFO - REFTEST INFO | Total canvas count = 2
[task 2017-06-09T02:13:07.053117Z] 02:13:07     INFO - --DOCSHELL 0x7fc8a50d9800 == 5 [pid = 1006] [id = {0e093012-6435-41d4-9902-fd126216833f}]
[task 2017-06-09T02:13:07.135242Z] 02:13:07     INFO - --DOCSHELL 0x7fc8a506c800 == 4 [pid = 1006] [id = {65f4e8c9-609a-411e-867a-570d3cc2373f}]
[task 2017-06-09T02:13:07.264035Z] 02:13:07     INFO - --DOCSHELL 0x7fc8ba31a000 == 3 [pid = 1006] [id = {a4c8a27e-c049-4d3b-879c-dc54426f523d}]
[task 2017-06-09T02:13:07.865906Z] 02:13:07     INFO - --DOCSHELL 0x7fc8b0006000 == 2 [pid = 1006] [id = {27ce614d-305d-47a5-b93e-093505fe9e0f}]
[task 2017-06-09T02:13:07.866446Z] 02:13:07     INFO - --DOCSHELL 0x7fc8b041e000 == 1 [pid = 1006] [id = {217ab7de-f6b5-48a9-b10b-33dba5c9b3c8}]
[task 2017-06-09T02:13:08.010564Z] 02:13:08     INFO - --DOCSHELL 0x7fc895368000 == 0 [pid = 1006] [id = {e4bbb278-a497-42e1-9ba4-50566ea22be2}]
[task 2017-06-09T02:13:08.194291Z] 02:13:08     INFO - Assertion failure: RenderThread::IsInRenderThread(), at /home/worker/workspace/build/src/gfx/webrender_bindings/RenderTextureHost.cpp:19
[task 2017-06-09T02:13:08.194595Z] 02:13:08     INFO - #01: mozilla::wr::RenderBufferTextureHost::~RenderBufferTextureHost [gfx/webrender_bindings/RenderBufferTextureHost.cpp:44]
[task 2017-06-09T02:13:08.194675Z] 02:13:08     INFO - 
[task 2017-06-09T02:13:08.194877Z] 02:13:08     INFO - #02: mozilla::wr::RenderTextureHost::Release [gfx/webrender_bindings/RenderTextureHost.h:22]
[task 2017-06-09T02:13:08.195008Z] 02:13:08     INFO - 

I will keep thinking how to solve this.

Comment 4

[Intermittent Failures Robot]

1 failures in 814 pushes (0.001 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* mozilla-central: 1

Platform breakdown:
* linux64-qr: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1371546&startday=2017-06-12&endday=2017-06-18&tree=all

Comment 5

[Vincent Liu[:vliu]]

Attachment: 0001-Bug-1371546-Sync-with-the-render-thread-when-ShutDow.patch

Hi Jerry,
The attached patch intends to sync with the render thread when ShutDown() was called in main thread. Two phases were covered.

1. When shutdown was called in Main, mHasShutdown was set to make a early return to prevent putting/removing any RenderTextureHost into mRenderTextures.
2. Post a specific task into message loop in RenderThread. When the runnable of this task invoked, it means all pending tasks were done as expected. After that, it was safe to destruct sRenderThread.

Comment 6

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

Comment on attachment 8880295 [details] [diff] [review]
0001-Bug-1371546-Sync-with-the-render-thread-when-ShutDow.patch

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

Please use SynchronousTask[1] instead. That could simplify your code.
There are a lot of samples in [2].

[1]
https://dxr.mozilla.org/mozilla-central/rev/b1b9129838ade91684574f42219b2010928d7db4/gfx/layers/ipc/SynchronousTask.h
[2]
https://dxr.mozilla.org/mozilla-central/search?q=SynchronousTask

::: gfx/webrender_bindings/RenderThread.cpp
@@ +24,5 @@
>    , mPendingFrameCountMapLock("RenderThread.mPendingFrameCountMapLock")
>    , mRenderTextureMapLock("RenderThread.mRenderTextureMapLock")
> +  , mHasShutdown(false)
> +  , mFinish(false)
> +  , mMonitor("DecodePoolImpl")

The monitor's name is not related to render thread.

@@ +87,5 @@
>    sRenderThread = nullptr;
>  }
>  
> +void
> +RenderThread::DeferredWaitForRenderThreadFinish()

The name of this function is not clear. Maybe just use "shutdownTask".

Comment 7

[Vincent Liu[:vliu]]

Attachment: 0001-Bug-1371546-Sync-with-the-render-thread-when-ShutDow.patch

Thanks for the review. SynchronousTask is more simple and more clear to implement. Thanks for the suggestion. Could you please have a review? Thanks.

Comment 8

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

Comment on attachment 8880738 [details] [diff] [review]
0001-Bug-1371546-Sync-with-the-render-thread-when-ShutDow.patch

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

::: gfx/webrender_bindings/RenderThread.cpp
@@ +74,2 @@
>  
> +  layers::SynchronousTask task("RenderThread");

Use "RenderThreadShutdown" instead.

@@ +299,5 @@
>  RenderThread::RegisterExternalImage(uint64_t aExternalImageId, already_AddRefed<RenderTextureHost> aTexture)
>  {
>    MutexAutoLock lock(mRenderTextureMapLock);
>  
> +  if (mHasShutdown) {

I don't know that is enough if we check the shutdown status for RegisterExternalImage() and UnregisterExternalImage() function.

Is it possible that the render thread still receive NewFrameReady() from WR backend thread when the render thread is in shutdown stage?
If it is yes, all functions in render thread should check the shutdown status.

Or we don't need to care about this problem. Since we are in shutdown stage, the the compositorBridge should also in shutdown state. Then, there should no another new task put into the render thread.

Comment 9

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

Hi nical,
Could you please check the question in comment 8?

Comment 10

[Intermittent Failures Robot]

2 failures in 892 pushes (0.002 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* mozilla-inbound: 2

Platform breakdown:
* linux64-qr: 2

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1371546&startday=2017-06-19&endday=2017-06-25&tree=all

Comment 11

[Nicolas Silva [:nical]]

> Is it possible that the render thread still receive NewFrameReady() from WR backend thread when the render thread is in shutdown stage?
> If it is yes, all functions in render thread should check the shutdown status.

Sorry for the late answer. Yes, I think that it is possible for the render thread to get into the shutdown state while the render backend is still processing a frame. Ideally we would propagate shutdown through the render backend thread so that there is no race between setting the shutdown state and frames being built on the render backend thread (but right now we don't do anything like that and we set the shutdown directly from the compositor thread).

Comment 12

[Nicolas Silva [:nical]]

Actually, nevermind what I just said about propagating the shutdown state through the render backend thread, because there can be several render backend threads (one per renderer, so one per window), all communicating to the same renderer. This may change in the future but it's not sure.

Comment 13

[Nicolas Silva [:nical]]

(In reply to Nicolas Silva [:nical] from comment #12)
> [..] all communicating to the same renderer.

Sorry, I meant communicating to the same render thread (but each with their own Renderer on the render thread).

Comment 14

[Vincent Liu[:vliu]]

Attachment: 0001-Bug-1371546-Sync-with-the-render-thread-when-ShutDow.patch

Hi nical, Jerry,
I also added shutdown check in both  NewFrameReady() and NewScrollFrameReady() to get into the shutdown state while the render backend is still processing a frame. Could you please have a review for the patch? Really thanks.

Comment 15

[Vincent Liu[:vliu]]

Attachment: 0001-Bug-1371546-Sync-with-the-render-thread-when-ShutDow.patch

Adding shutdown check in AddRenderer() and RemoveRenderer() for the patch.

Hi :nical, Jerry,

As talked, could you please have review? Thanks.

Comment 16

[Intermittent Failures Robot]

4 failures in 718 pushes (0.006 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* autoland: 2
* try: 1
* mozilla-inbound: 1

Platform breakdown:
* linux64-qr: 4

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1371546&startday=2017-06-26&endday=2017-07-02&tree=all

Comment 17

[Vincent Liu[:vliu]]

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

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c25fb462da79
Sync with the render thread when ShutDown() was called in main thread. r=nical, jerry

Comment 19

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

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