2053464 - Webrender worker threads' nsThreads can be leaked in GPU process

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jamie Nicol [:jnicol]]

I've seen this with my local patches to run webrender on Metal ANGLE:

     |<----------------Class--------------->|<-----Bytes------>|<----Objects---->|
     |                                      | Per-Inst   Leaked|   Total      Rem|
   0 |TOTAL                                 |       67      504|   11937        2|
  94 |Mutex                                 |       96       96|     390        1|
 304 |nsThread                              |      408      408|      25        1|

nsTraceRefcnt::DumpStatistics: 316 entries
TEST-INFO | leakcheck | gpu leaked 1 Mutex
TEST-INFO | leakcheck | gpu leaked 1 nsThread
TEST-UNEXPECTED-FAIL | leakcheck | gpu 504 bytes leaked (Mutex, nsThread)

And tracked it down to nsThreads that appear to be be wrapping webrender's rayon worker threads. Doesn't appear to be directly caused by Metal ANGLE patches, but presumably changes some timing that makes reproducing it more likely.

I can reproduce on Windows and Mac with no other changes other than adding a short sleep here in the worker threads' exit_handler.

To fix this, we should join() the worker threads in wr_thread_pool_delete(). But to do that, we have to ensure that we own the last remaining reference to the worker threads. Seems we can achieve that by destroying mShaders and mProgramCache just before releasing the thread pools here.

We should probably only do the join() in NS_FREE_PERMANENT_DATA builds, otherwise we don't really care about the leak and just want to shutdown quickly. We do similar for stylo's threads here

Comment 1

[Jamie Nicol [:jnicol]]

Attachment: Bug 2053464 - Wait for webrender worker threads to join on shutdown in leak-checking builds. r?#gfx-reviewers

In NS_FREE_PERMANENT_DATA builds, we must join webrender's worker thread
handles when shutting down the thread pools. Without doing so, leak
logging can run before the worker threads exit and release the nsThread
wrappers lazily created for them, manifesting in leakcheck complaining
about leaked nsThread objects.

In non-NS_FREE_PERMANENT_DATA builds, we do not care about this and just
want to shut down quickly.

In order to join() the worker threads in wr_thread_pool_delete(), we
must ensure that we have just dropped the final reference to the
Arc<rayon::ThreadPool>. RenderThread::ShutDownTask() must therefore
release its reference to the program cache prior to shutting down the
thread pools. The renderer/backend-owned references should already have
been released when the renderers were destroyed. As a precaution, the
result of Arc::into_inner(workers) is checked to ensure there are no
other owners of the thread pool when wr_thread_pool_delete() is called.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/b3cbbefbd062
https://hg.mozilla.org/integration/autoland/rev/46d9618f38e7
Wait for webrender worker threads to join on shutdown in leak-checking builds. r=gfx-reviewers,lsalzman

Comment 3

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

https://hg.mozilla.org/mozilla-central/rev/46d9618f38e7