[vlc-devel] [PATCH 10/10] vout: control: replace the custom hold/release/wait by a vlc_sem_t

[Steve Lhomme]

On 2020-07-17 13:50, Steve Lhomme wrote:
> The goal is to protect the access to the command array.
> 
> We still allow pushing commands from the thread holding the command array, to
> avoid a deadlock if the same thread tries to request this resource twice.
> ---
>   src/video_output/control.c | 28 ++++++++--------------------
>   src/video_output/control.h |  4 +---
>   2 files changed, 9 insertions(+), 23 deletions(-)
> 
> diff --git a/src/video_output/control.c b/src/video_output/control.c
> index 52960d66591..da08d362164 100644
> --- a/src/video_output/control.c
> +++ b/src/video_output/control.c
> @@ -43,10 +43,8 @@ void vout_control_cmd_Clean(vout_control_cmd_t *cmd)
>   /* */
>   void vout_control_Init(vout_control_t *ctrl)
>   {
> -    vlc_mutex_init(&ctrl->lock);
> -    vlc_cond_init(&ctrl->wait_available);
> +    vlc_sem_init(&ctrl->lock, 1);
>   
> -    ctrl->is_held = false;
>       atomic_init(&ctrl->is_dead, false);
>       ARRAY_INIT(ctrl->cmd);
>   }
> @@ -68,13 +66,14 @@ void vout_control_Dead(vout_control_t *ctrl)
>   
>   static void vout_control_Push(vout_control_t *ctrl, vout_control_cmd_t *cmd)
>   {
> -    vlc_mutex_lock(&ctrl->lock);
> +    bool locked = vlc_sem_trywait(&ctrl->lock) == 0;

I misread what this does. If another thread has the resource it will 
consider it's already locked and assume it's lock by the current thread.

So forget this patch for now.

>       if (!atomic_load(&ctrl->is_dead)) {
>           ARRAY_APPEND(ctrl->cmd, *cmd);
>       } else {
>           vout_control_cmd_Clean(cmd);
>       }
> -    vlc_mutex_unlock(&ctrl->lock);
> +    if (locked)
> +        vlc_sem_post(&ctrl->lock);
>   }
>   
>   void vout_control_PushVoid(vout_control_t *ctrl, int type)
> @@ -110,28 +109,17 @@ void vout_control_PushMouse(vout_control_t *ctrl, int type, const vlc_mouse_t *m
>   
>   void vout_control_Hold(vout_control_t *ctrl)
>   {
> -    vlc_mutex_lock(&ctrl->lock);
> -    while (ctrl->is_held)
> -        vlc_cond_wait(&ctrl->wait_available, &ctrl->lock);
> -    ctrl->is_held = true;
> -    vlc_mutex_unlock(&ctrl->lock);
> +    vlc_sem_wait(&ctrl->lock);
>   }
>   
>   void vout_control_Release(vout_control_t *ctrl)
>   {
> -    vlc_mutex_lock(&ctrl->lock);
> -    assert(ctrl->is_held);
> -    ctrl->is_held = false;
> -    vlc_cond_signal(&ctrl->wait_available);
> -    vlc_mutex_unlock(&ctrl->lock);
> +    vlc_sem_post(&ctrl->lock);
>   }
>   
>   int vout_control_Pop(vout_control_t *ctrl, vout_control_cmd_t *cmd)
>   {
> -    vlc_mutex_lock(&ctrl->lock);
> -
> -    while (ctrl->is_held)
> -        vlc_cond_wait(&ctrl->wait_available, &ctrl->lock);
> +    vlc_sem_wait(&ctrl->lock);
>   
>       int ret;
>       if (ctrl->cmd.i_size > 0) {
> @@ -141,7 +129,7 @@ int vout_control_Pop(vout_control_t *ctrl, vout_control_cmd_t *cmd)
>       } else {
>           ret = VLC_EGENERIC;
>       }
> -    vlc_mutex_unlock(&ctrl->lock);
> +    vlc_sem_post(&ctrl->lock);
>   
>       return ret;
>   }
> diff --git a/src/video_output/control.h b/src/video_output/control.h
> index 882b887b959..91afdd92f64 100644
> --- a/src/video_output/control.h
> +++ b/src/video_output/control.h
> @@ -47,12 +47,10 @@ typedef struct {
>   void vout_control_cmd_Clean(vout_control_cmd_t *);
>   
>   typedef struct {
> -    vlc_mutex_t lock;
> -    vlc_cond_t  wait_available;
> +    vlc_sem_t   lock;
>   
>       /* */
>       atomic_bool is_dead;
> -    bool is_held;
>       DECL_ARRAY(vout_control_cmd_t) cmd;
>   } vout_control_t;
>   
> -- 
> 2.26.2
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>