[Mesa-dev] [PATCH] egl: Create queryable strings in eglInitialize().

[Matt Turner]

On Tue, Mar 10, 2015 at 8:19 PM, Ian Romanick <idr at freedesktop.org> wrote:
> On 03/10/2015 05:20 PM, Matt Turner wrote:
>> Creating/recreating the strings in eglQueryString() is extra work and
>> isn't thread-safe, as exhibited by shader-db's run.c using libepoxy.
>>
>> Multiple threads in run.c call eglReleaseThread() around the same time.
>> libepoxy calls eglQueryString() to determine whether eglReleaseThread()
>> exists, and our EGL implementation passes a pointer to the version
>> string to libepoxy while simultaneously overwriting the string, leading
>> to a failure in libepoxy.
>>
>> Moreover, the EGL spec says (emphasis mine):
>>
>> "eglQueryString returns a pointer to a *static*, zero-terminated string"
>>
>> This patch moves some auxiliary functions from eglmisc.c to eglapi.c so
>> that they may be used to create the extension, API, and version strings
>> once during eglInitialize(). The auxiliary functions are renamed from
>> _eglUpdate* to _eglCreate*, and some checks made unnecessary by calling
>> the functions from eglInitialize() are removed.
>>
>> It was suggested to me to simply make the generation of the version
>> string behave like that of the extension and API strings -- to do a
>> check like
>>
>>    if (exts[0])
>>       return;
>>
>> before creating the string, but that is not thread-safe either.
>
> It is safe because eglQueryString calls _eglQueryString (via
> drv->API.QueryString) inside a _eglLockDisplay region.

Oh, you're right. I'll drop that bit from the commit message, pending
removal of API.QueryString.

> The method you use here presumes that every implementation will use the
> _eglQueryString fallback for drv->API.QueryString.  Right now it does,
> and I suspect that they always will.  A little git archaeology leads me
> to believe that this has always been the case.
>
> Perhaps we should just remove API.QueryString altogether?  Perhaps Brian
> knows of a reason not to...

Getting rid of that could be nice. We could inline _eglQueryString,
avoid locking the display, and throw out eglmisc.c/eglmisc.h.