Created attachment 140440 [details]
apitrace output from the example
I have observed a thread leak in Linux when using offscreen rendering in VTK via llvmpipe. A minimal VTK example code is attached, as is the output of running apitrace.
One can observe the thread leak by doing the following:
* down ParaView-5.5.2-Qt5-MPI-Linux-64bit.tar.gz from www.paraview.org/download
* tar xzf ParaView-5.5.2-Qt5-MPI-Linux-64bit.tar.gz
* cd ParaView-5.5.2-Qt5-MPI-Linux-64bit
bin/pvbatch --mesa-llvm vtk-mesa-threads-growth.py
In this example, we are repeatedly resizing the render window. Each time the window is resized, the current context is destroyed with glXDestroyContext. A new context is created with the new desired size, but looking at the output of htop, the threads used from the previous render never rejoin the main thread, causing the total number of threads to grow each time the render window size is changed. For long-running animation creation, this situation can lead to a major system slow down.
If this is not a mesa bug and it looks like I am doing something not correct in the trace, please let me know.
Created attachment 140441 [details]
VTK Python script showing the thread leak
Please note that the apitrace output is from a single iteration of the loop in vtk-mesa-threads-growth.py
I'm near certain this is a bug in the xlib state tracker rather than llvmpipe. swr will leak as well, you just don't notice because it doesn't create additional threads.
glXChooseFBConfig will call choose_visual(), which will call xmesa_init(), which will create the pipe screen (via xmesa_init_display()). (llvmpipe threads are per screen, not per context.)
(mesa/src/gallium/state_trackers/glx/xlib/ xm_api.c and glx_api.c)
Note that there's 2 places where the screens would get destroyed (one in xmesa_close_display(), reached via XCloseDisplay(), the other is in XMesaDestroyContext(), reached via glXDestroyContext()). Both are commented out and marked as FIXME, since apparently it's not safe to destroy the screen at this point since surfaces can still be around...
This looks apparently broken but it's not obvious to me how to fix it. At first glance it looks like screen destruction should be delayed until there's neither contexts nor surfaces referencing it are around but I guess it isn't that easy...
Thank you for your quick reply and insight, Roland. Creating and destroying glX contexts is a bit of a peculiarity in VTK that we may be able to do away with - I'm not sure why VTK does that, so maybe we can work around this issue.
With regards to freeing the screen, I'm afraid I'm nearly completely unversed in the Mesa library, so I'm at a loss as to an appropriate fix. Could a first step be to free the screen if all surfaces and contexts referencing it are free? Is it possible to check that?
(In reply to Cory Quammen from comment #4)
> Thank you for your quick reply and insight, Roland. Creating and destroying
> glX contexts is a bit of a peculiarity in VTK that we may be able to do away
> with - I'm not sure why VTK does that, so maybe we can work around this
> With regards to freeing the screen, I'm afraid I'm nearly completely
> unversed in the Mesa library, so I'm at a loss as to an appropriate fix.
> Could a first step be to free the screen if all surfaces and contexts
> referencing it are free? Is it possible to check that?
I think the problem is precisely that we don't know which surfaces and contexts reference a screen. Surfaces itself aren't tied to contexts, and nearly all surfaces are created by pipe screen methods and have to be destroyed by the equivalent destroy methods, but this isn't true for the surfaces created by X (and you can and will get things like glXSwapBuffers without any glx context around any longer).
(At some point we actually did free the screens in xmesa_close_display(), but this caused crashes, and the commit trading the crashes back for the leaks was:
Author: George Kyriazis <email@example.com>
Date: Fri Mar 4 12:26:00 2016 -0700
st/xlib: Don't destroy screen on XCloseDisplay()
screen may still be used by other resources that are not yet freed.
To correctly fix this there will be a need to account for resources
differently, but this quick fix is not any worse than the original
code that leaked screens anyway.
Reviewed-by: Brian Paul <firstname.lastname@example.org>
I am not entirely sure what actually can still be around if the display is closed though, my knowledge of the glx code there isn't all that great - but certainly this is an old issue which should really be addressed.
-- GitLab Migration Automatic Message --
This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.
You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/mesa/mesa/issues/248.