[vlc-devel] [RFC] vdpau/instance: create the vdp/device under lock

[Rémi Denis-Courmont]

No. The mutex is there to guarantee the uniqueness of listed instance per display and serialise memory use.

Le 28 novembre 2019 12:13:09 GMT+02:00, Steve Lhomme <robux4 at ycbcr.xyz> a écrit :
>Isn't the mutex there to protect using the same resource multiple times
>
>? So I don't see why it doesn't actually protect the resource creation 
>itself. In the current code the same resource is created twice and 
>destroyed if created twice (with what side effects ?).
>
>On 2019-11-27 16:57, Rémi Denis-Courmont wrote:
>> Reentrancy is the reason IIRC.
>> 
>> Le 27 novembre 2019 13:48:14 GMT+02:00, Steve Lhomme
><robux4 at ycbcr.xyz> 
>> a écrit :
>> 
>>     There's no reason to unlock before creating more than 1 instance
>and then
>>     destroying the extra ones. No matter what we'll still return only
>after
>>     an instance has been created (or failed to be created).
>Concurrent calls
>>     for the same display name will only create one instance.
>>    
>------------------------------------------------------------------------
>>       modules/hw/vdpau/instance.c | 21 ++++++---------------
>>       1 file changed, 6 insertions(+), 15 deletions(-)
>> 
>>     diff --git a/modules/hw/vdpau/instance.c
>b/modules/hw/vdpau/instance.c
>>     index 4a6d27fe5f0..862e63e0e21 100644
>>     --- a/modules/hw/vdpau/instance.c
>>     +++ b/modules/hw/vdpau/instance.c
>>     @@ -129,7 +129,7 @@ static pthread_mutex_t lock =
>PTHREAD_MUTEX_INITIALIZER;
>>       VdpStatus vdp_get_x11(const char *display_name, int snum,
>>                             vdp_t **restrict vdpp, VdpDevice
>*restrict devicep)
>>       {
>>     -    vdp_instance_t *vi, *vi2;
>>     +    vdp_instance_t *vi;
>>       
>>           if (display_name == NULL)
>>           {
>>     @@ -140,29 +140,20 @@ VdpStatus vdp_get_x11(const char
>*display_name, int snum,
>>       
>>           pthread_mutex_lock(&lock);
>>           vi = vdp_instance_lookup(display_name, snum);
>>     -    pthread_mutex_unlock(&lock);
>>           if (vi != NULL)
>>               goto found;
>>       
>>           vi = vdp_instance_create(display_name, snum);
>>           if (vi == NULL)
>>     -        return VDP_STATUS_ERROR;
>>     -
>>     -    pthread_mutex_lock(&lock);
>>     -    vi2 = vdp_instance_lookup(display_name, snum);
>>     -    if (unlikely(vi2 != NULL))
>>     -    {   /* Another thread created the instance (race condition
>corner case) */
>>     -        pthread_mutex_unlock(&lock);
>>     -        vdp_instance_destroy(vi);
>>     -        vi = vi2;
>>     -    }
>>     -    else
>>           {
>>     -        vi->next = list;
>>     -        list = vi;
>>               pthread_mutex_unlock(&lock);
>>     +        return VDP_STATUS_ERROR;
>>           }
>>     +
>>     +    vi->next = list;
>>     +    list = vi;
>>       found:
>>     +    pthread_mutex_unlock(&lock);
>>           *vdpp = vi->vdp;
>>           *devicep = vi->device;
>>           return VDP_STATUS_OK;
>> 
>> 
>> -- 
>> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez
>excuser 
>> ma brièveté.
>> 
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
>> 
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel

-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20191128/4099d082/attachment.html>