[VDPAU] [PATCH 4/4] vdpau_wrapper: protect concurrent access to _imp_get_proc_address

[Aaron Plattner]

On 10/29/2014 05:47 AM, Rémi Denis-Courmont wrote:
> From: Rémi Denis-Courmont <remid at nvidia.com>
>
> The wrapper, as it's currently written, cannot cope with more than one
> VdpGetProcAddress implementation. Luckily, this should hardly ever
> happen.

Does this ever actually happen?  As you said, there can only ever be one 
_vdp_imp_device_create_x11_proc at the moment.  This is protecting 
against an implementation that returns different gpa pointers, e.g. for 
different screens?  If so, that seems okay to me.

Reviewed-by: Aaron Plattner <aplattner at nvidia.com>

> This patch protects access to the _imp_get_proc_address variable to
> conform to the memory model, and ensures that a single VDPAU
> implementation is used - failing safe if not so.
> ---
>   src/vdpau_wrapper.c | 31 ++++++++++++++++++++++++-------
>   1 file changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/src/vdpau_wrapper.c b/src/vdpau_wrapper.c
> index 7d4885d..196050b 100644
> --- a/src/vdpau_wrapper.c
> +++ b/src/vdpau_wrapper.c
> @@ -526,6 +526,7 @@ VdpStatus vdp_device_create_x11(
>   {
>       static pthread_once_t once = PTHREAD_ONCE_INIT;
>       static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
> +    VdpGetProcAddress *gpa;
>       VdpStatus status = VDP_STATUS_OK;
>
>       pthread_once(&once, init_fixes);
> @@ -541,17 +542,33 @@ VdpStatus vdp_device_create_x11(
>       if (status != VDP_STATUS_OK)
>           return status;
>
> -    status = _vdp_imp_device_create_x11_proc(
> -        display,
> -        screen,
> -        device,
> -        &_imp_get_proc_address
> -    );
> +    status = _vdp_imp_device_create_x11_proc(display, screen, device, &gpa);
>       if (status != VDP_STATUS_OK) {
>           return status;
>       }
>
>       *get_proc_address = vdp_wrapper_get_proc_address;
>
> -    return VDP_STATUS_OK;
> +    pthread_mutex_lock(&lock);
> +    if (_imp_get_proc_address != gpa) {
> +        if (_imp_get_proc_address == NULL)
> +            _imp_get_proc_address = gpa;
> +        else
> +        /* Currently the wrapper can only deal with one back-end.
> +         * This should never happen, but better safe than sorry. */
> +            status = VDP_STATUS_NO_IMPLEMENTATION;
> +    }
> +    pthread_mutex_unlock(&lock);
> +
> +    if (status != VDP_STATUS_OK) {
> +        void *pv;
> +
> +        if (gpa(*device, VDP_FUNC_ID_DEVICE_DESTROY, &pv) == VDP_STATUS_OK) {
> +            VdpDeviceDestroy *device_destroy = pv;
> +
> +            device_destroy(*device);
> +        }
> +    }
> +
> +    return status;
>   }
>


-- 
Aaron