[Mesa-stable] [PATCH] ralloc: Use __attribute__((destructor)) instead of atexit(3)

[Emil Velikov]

On 7 September 2015 at 10:16, Jean-Sébastien Pédron
<jean-sebastien.pedron at dumbbell.fr> wrote:
> On 06.09.2015 23:40, Emil Velikov wrote:
>> Hi Jean-Sébastien,
>
> Hi!
>
>> On 3 September 2015 at 19:07, Jean-Sébastien Pédron
>> We have 4(5) users of atexit() - EGL, gallium trace driver, core mesa
>> and util/ralloc. The latter of which is used almost everywhere in
>> mesa. So a bit I'm confused how you hit this only with OpenCL :-\
>> Perhaps the others should be updated as well ?
>
> That's what I'm wondering too.
>
> I didn't read the code thoroughly; my guess is that in the case of the
> OpenCL ICD loader, the loader explicitely dlclose() the backend. This
> must not be the case with OpenGL applications.
>
> I can update the patch to change all uses of atexit(3) if you are ok
> with it.
>
Upon a closer look it seems that
 - glx: both dri(1) and drisw dlclose the dri module.
 - gbm: doesn't use dlclose
 - egl/dri2: uses dlclose at eglTerminate time.
 - (some) opencl loaders use dlclose
 - the vdpau loader/wrapper uses a __attribute__((destructor)),  in
which it dlcloses the driver.
 - vaapi - uses dlclose at vaTerminate and in the error paths
 - omx ?

Long story short - although people haven't complained (yet) it might
be nice to get all cases in mesa sorted.

>>> __attribute__((destructor)) fixes the problem because such handlers are
>>> called when a library is unloaded and when the program exits.
>> Considering that atexit() (reportedly) works for Linux/Glibc, Solaris,
>> Android and Windows perhaps we should consider this as a workaround
>> for FreeBSD (and other *BSD) systems ?
>
> On one hand, my concern is this behavior is an extension to the standard
> and it could break for someone else. On the other hand, I don't like the
> standard behavior and would be glad to see FreeBSD implement this extension.
>
> I still think relying on this atexit(3) extension is fragile and I don't
> consider the use of the attribute a workaround.
>
>> Considering the multiple users (mentioned above) should one set
>> priority for the destructors ?
>
> You are right, I didn't think of the calling order of the destructors.
>
Considering the way the discussion is going, perhaps it's better to
add the destructor as a {Free,*}BSD only option for now ?

Bonus points for the "bootstrap"  suggestion from Erik, and checking
if gcc 4.2 (min. mesa requirement) supports ctor/dtor priorities and
converting everyone to use them :-)

We're quite early in the devel cycle, so the latter patch will get
some testing. And if it does not work out, we can revert it.

Thanks for bringing this topic up.
-Emil

P.S. I will be revisiting the loader/libudev sometime soon,
effectively pushing the chaos into libdrm. You should get a lovely
warning (missing implementation of XXX) when compiling libdrm, for
which patches will be gladly accepted :-)