[vlc-devel] [PATCH 3/5] sap: merge both locks

[Steve Lhomme]

I think you forgot to remove the lock field in struct sap_address_t.

On 2020-02-09 14:33, Rémi Denis-Courmont wrote:
> This is simpler and even necessary for the following fix.
> ---
>   src/stream_output/sap.c | 32 +++++++++++---------------------
>   1 file changed, 11 insertions(+), 21 deletions(-)
> 
> diff --git a/src/stream_output/sap.c b/src/stream_output/sap.c
> index 8c5413fb17..ca5a2f22c6 100644
> --- a/src/stream_output/sap.c
> +++ b/src/stream_output/sap.c
> @@ -102,7 +102,6 @@ static sap_address_t *AddressCreate (vlc_object_t *obj, const char *group)
>       getsockname (fd, (struct sockaddr *)&addr->orig, &addr->origlen);
>   
>       addr->interval = var_CreateGetInteger (obj, "sap-interval");
> -    vlc_mutex_init (&addr->lock);
>       vlc_cond_init (&addr->wait);
>       addr->session_count = 0;
>       vlc_list_init(&addr->sessions);
> @@ -124,7 +123,6 @@ static void AddressDestroy (sap_address_t *addr)
>       vlc_cancel (addr->thread);
>       vlc_join (addr->thread, NULL);
>       vlc_cond_destroy (&addr->wait);
> -    vlc_mutex_destroy (&addr->lock);
>       net_Close (addr->fd);
>       free (addr);
>   }
> @@ -138,8 +136,8 @@ noreturn static void *RunThread (void *self)
>   {
>       sap_address_t *addr = self;
>   
> -    vlc_mutex_lock (&addr->lock);
> -    mutex_cleanup_push (&addr->lock);
> +    vlc_mutex_lock(&sap_mutex);
> +    mutex_cleanup_push(&sap_mutex);
>   
>       for (;;)
>       {
> @@ -147,7 +145,7 @@ noreturn static void *RunThread (void *self)
>           vlc_tick_t deadline;
>   
>           while (vlc_list_is_empty(&addr->sessions))
> -            vlc_cond_wait (&addr->wait, &addr->lock);
> +            vlc_cond_wait(&addr->wait, &sap_mutex);
>   
>           assert (addr->session_count > 0);
>   
> @@ -157,7 +155,7 @@ noreturn static void *RunThread (void *self)
>               send (addr->fd, p_session->data, p_session->length, 0);
>               deadline += vlc_tick_from_samples(addr->interval, addr->session_count);
>   
> -            if (vlc_cond_timedwait (&addr->wait, &addr->lock, deadline) == 0)
> +            if (vlc_cond_timedwait(&addr->wait, &sap_mutex, deadline) == 0)
>                   break; /* list may have changed! */
>           }
>       }
> @@ -293,10 +291,6 @@ sout_AnnounceRegisterSDP (vlc_object_t *obj, const char *sdp,
>       }
>   matched:
>       vlc_list_append(&sap_addr->node, &sap_addrs);
> -    /* Switch locks.
> -     * NEVER take the global SAP lock when holding a SAP thread lock! */
> -    vlc_mutex_lock (&sap_addr->lock);
> -    vlc_mutex_unlock (&sap_mutex);
>   
>       session_descriptor_t *session = malloc(sizeof (*session));
>       if (unlikely(session == NULL))
> @@ -358,7 +352,7 @@ matched:
>       sap_addr->session_count++;
>       vlc_cond_signal (&sap_addr->wait);
>   out:
> -    vlc_mutex_unlock (&sap_addr->lock);
> +    vlc_mutex_unlock(&sap_mutex);
>       return session;
>   }
>   
> @@ -375,27 +369,23 @@ void sout_AnnounceUnRegister (vlc_object_t *obj, session_descriptor_t *session)
>   
>       msg_Dbg (obj, "removing SAP session");
>       vlc_mutex_lock (&sap_mutex);
> -    vlc_mutex_lock(&addr->lock);
>       vlc_list_remove(&session->node);
>   
>       if (vlc_list_is_empty(&addr->sessions))
>           /* Last session for this address -> unlink the address */
>           vlc_list_remove(&addr->node);
> -    vlc_mutex_unlock (&sap_mutex);
> -
> -    if (vlc_list_is_empty(&addr->sessions))
> -    {
> -        /* Last session for this address -> unlink the address */
> -        vlc_mutex_unlock (&addr->lock);
> -        AddressDestroy (addr);
> -    }
>       else
>       {
>           addr->session_count--;
>           vlc_cond_signal (&addr->wait);
> -        vlc_mutex_unlock (&addr->lock);
>       }
>   
> +    vlc_mutex_unlock (&sap_mutex);
> +
> +    if (vlc_list_is_empty(&addr->sessions))
> +        /* Last session for this address -> unlink the address */
> +        AddressDestroy (addr);
> +
>       free(session->data);
>       free(session);
>   }
> -- 
> 2.25.0
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>