[Mesa-dev] [PATCH] Fix locking of GLsync objects

[Emil Velikov]

Hello Steinar,

On 8 December 2015 at 01:01, Steinar H. Gunderson
<sgunderson at bigfoot.com> wrote:
> Hi,
>
> I was told that it's easier for people to review my patch if it comes in via
> email than being stuck in the bug tracker; FWIW, this is for bug 120238.
Which bugtracker it this ? bugs.fd.o does not like the number
mentioned. Please add the full URL to the commit message with a
Bugzilla: tag.

> (It's the same patch as is already in the tracker.)
>
> /* Steinar */
>
> ===
>
> From 6e3d1880fa78a3a965cb7eb51ee12b1f785f84bb Mon Sep 17 00:00:00 2001
> From: "Steinar H. Gunderson" <sesse at google.com>
> Date: Tue, 1 Dec 2015 22:05:11 +0100
> Subject: [PATCH] Fix locking of GLsync objects.
>
> GLsync objects had a race condition when used from multiple threads
> (which is the main point of the extension, really); it could be
> validated as a sync object at the beginning of the function, and then
> deleted by another thread before use, causing crashes. Fix this by
> changing all casts from GLsync to struct gl_sync_object to a new
> function _mesa_get_sync() that validates and increases the refcount.
>
Might be worth keeping _mesa_ref_sync_object(), even if it's an inline
wrapper around the above. As things get a bit confusing - foo_get vs
foo_unref.

Alternatively one could even throw the locking (+extra checks) into
the validate, use it in _mesa_IsSync(), while using the ref/unref
combo elsewhere and drop the "amount" argument from unref.

I'm biased towards the latter, although let's see how others feel on the topic.

> In a similar vein, validation itself uses _mesa_set_search(), which
> requires synchronization -- it was called without a mutex held, causing
> spurious error returns and other issues. Since _mesa_get_sync() now
> takes the shared context mutex, this problem is also resolved.
>
Please mention if this commit fixes a certain game/program.
Can you also add the following tag. This way it'll be harder to miss
the patch when picking things for the stable branch(es).

Cc: "11.0 11.1" <mesa-stable at lists.freedesktop.org>

Thanks
Emil