[vlc-devel] [PATCH] win32: the vout lock is recursive

[Rémi Denis-Courmont]

Le 2016-06-05 14:58, Steve Lhomme a écrit :
> On Sun, Jun 5, 2016 at 2:16 PM, Rémi Denis-Courmont <remi at remlab.net> 
> wrote:
>> Le 2016-06-05 13:28, Steve Lhomme a écrit :
>>>
>>> given by default they're all recursive it's ok, but when we detect
>>> possible
>>> issues it's not.
>>> ---
>>>  modules/video_output/win32/events.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/modules/video_output/win32/events.c
>>> b/modules/video_output/win32/events.c
>>> index c41fd89..22ad40e 100644
>>> --- a/modules/video_output/win32/events.c
>>> +++ b/modules/video_output/win32/events.c
>>> @@ -467,7 +467,7 @@ event_thread_t *EventThreadCreate( 
>>> vout_display_t *vd)
>>>          return NULL;
>>>
>>>      p_event->vd = vd;
>>> -    vlc_mutex_init( &p_event->lock );
>>> +    vlc_mutex_init_recursive( &p_event->lock );
>>
>>
>> It looks more like a work-around than a fix. Recursive locking is 
>> very
>> rarely done right.
>
> I agree but the lock seems widely used for a lot of things. I got the
> assert when called via Windows callbacks called through Windows API
> calls. It's hard to tell if these functions are only called in the UI
> thread, if the variables are used by other threads or only the UI 
> one.
> I'll see if I find only a few things that need changes.

One can do recursive locking right (e.g. libpulse threaded mainloop), 
but it's more difficult than not using recursive locking at all. Our 
history is not great (specifically the events manager): you easily end 
up with lock inversion or use-after-free.

I don't really care here, but I doubt it will really fix the bugs here.

>
>>>      vlc_cond_init( &p_event->wait );
>>>
>>>      p_event->is_cursor_hidden = false;
>>
>>
>> --
>> Rémi Denis-Courmont
>> http://www.remlab.net/
>> _______________________________________________
>> 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

-- 
Rémi Denis-Courmont
http://www.remlab.net/