[Mesa-dev] [PATCH] egl/wayland: Avoid race conditions when on non-main thread

[Pekka Paalanen]

On Wed, 4 May 2016 10:22:35 +0100
Daniel Stone <daniel at fooishbar.org> wrote:

> Hi Jonas,
> 
> On 4 May 2016 at 09:53, Jonas Ådahl <jadahl at gmail.com> wrote:
> > When EGL is used on some other thread than the thread that drives the
> > main wl_display queue, the Wayland EGL dri2 implementation is
> > vulnerable to a race condition related to display round trips and global
> > object advertisements.
> >
> > The race that may happen is that after after a proxy is created, but
> > before the queue is set, events meant to be emitted via the yet to be
> > set queue may already have been queued on the wrong queue.
> >
> > In order to make it possible to avoid this race, wayland 1.11
> > introduced new API that allows creating a proxy wrapper that may be used
> > as the factory proxy when creating new proxies via Wayland requests. The
> > queue of a proxy wrapper can be changed without effecting what queue
> > events emitted by the actual proxy will be queued on, while still
> > effecting what default queue proxies created from it will have.  
> 
> This looks good to me, but I am worried about the hard dependency,
> both at build and run-time. It would be great to make it optional at
> build-time (#ifdef wrapper creation and destruction, just doing a
> straight assignment otherwise to allow the other code to not be
> #ifdefed), but even better if it could also use dlsym() to discover if
> the symbol is available at runtime. Maybe the latter is overkill
> though.

But wouldn't that need #ifdefs in every place where the wrapper will be
used, because if the wrapper is not really the wrapper, there needs to
be a call to set the queue? Setting the queue is racy, yes, but it
would have a chance of working which it wouldn't have otherwise, right?
Potentially breaking also non-threaded apps.

Jonas, is this patch missing one more wl_proxy_wrapper_destroy() for
the display tear-down? I can only see the destroy in the
initialize-failure path.

This patch is also fixing an existing leak of wl_dpy in the case the
connection was created by the EGL, I think. That should also be
mentioned in the commit message.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160510/efb7ee02/attachment.sig>