A clearer comment would be: "Don't destroy the fence when it's in the
middle of util_queue_fence_signal (signalled but not unlocked yet
because util_queue_fence_is_signalled doesn't lock). Instead, wait
until util_queue_fence_signal returns and then destroy it."
Reviewed-by: Marek Olšák <marek.ol...@amd.com>
Marek
On Thu, Sep 28, 2017 at 6:10 PM, Nicolai Hähnle <nhaeh...@gmail.com> wrote:
> From: Nicolai Hähnle <nicolai.haeh...@amd.com>
>
> A tempting alternative fix would be adding a lock/unlock pair in
> util_queue_fence_is_signalled. However, that wouldn't actually
> improve anything in the semantics of util_queue_fence_is_signalled,
> while making that test much more heavy-weight. So this lock/unlock
> pair in util_queue_fence_destroy for "flushing out" other threads
> that may still be in util_queue_fence_signal looks like the better
> fix.
>
> Cc: mesa-sta...@lists.freedesktop.org
> ---
> src/util/u_queue.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/src/util/u_queue.c b/src/util/u_queue.c
> index 449da7dc9ab..82a761e8420 100644
> --- a/src/util/u_queue.c
> +++ b/src/util/u_queue.c
> @@ -113,20 +113,33 @@ util_queue_fence_init(struct util_queue_fence *fence)
> memset(fence, 0, sizeof(*fence));
> (void) mtx_init(&fence->mutex, mtx_plain);
> cnd_init(&fence->cond);
> fence->signalled = true;
> }
>
> void
> util_queue_fence_destroy(struct util_queue_fence *fence)
> {
> assert(fence->signalled);
> +
> + /* Protect against the following race:
> + *
> + * util_queue_fence_signal (begin)
> + * fence->signalled = true;
> + *
> util_queue_fence_is_signalled
> + * util_queue_fence_destroy
> + * cnd_broadcast(&fence->cond); <-- use-after-free
> + * util_queue_fence_signal (end)
> + */
> + mtx_lock(&fence->mutex);
> + mtx_unlock(&fence->mutex);
> +
> cnd_destroy(&fence->cond);
> mtx_destroy(&fence->mutex);
> }
>
> /****************************************************************************
> * util_queue implementation
> */
>
> struct thread_input {
> struct util_queue *queue;
> --
> 2.11.0
>
> _______________________________________________
> mesa-stable mailing list
> mesa-sta...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
