[vlc-devel] [PATCH] lib: core: Use vlc_atomic_rc_t for refcounting

[Hugo Beauzée-Luyssen]

On Wed, Jun 10, 2020, at 3:36 PM, Rémi Denis-Courmont wrote:
> Is the lock still used for anything?
> 
> Le 10 juin 2020 16:32:22 GMT+03:00, "Hugo Beauzée-Luyssen" 
> <hugo at beauzee.fr> a écrit :  lib/core.c            | 17 
> +++--------------
> >  lib/libvlc_internal.h |  3 ++-
> >  2 files changed, 5 insertions(+), 15 deletions(-)
> > 
> > diff --git a/lib/core.c b/lib/core.c
> > index 56b985f597..8a998e3095 100644
> > --- a/lib/core.c
> > +++ b/lib/core.c
> > @@ -60,7 +60,7 @@ libvlc_instance_t * libvlc_new( int argc, const char *const *argv )
> >      }
> >  
> >      p_new->p_libvlc_int = p_libvlc_int;
> > -    p_new->ref_count = 1;
> > +    vlc_atomic_rc_init( &p_new->ref_count );
> >      p_new->p_callback_list = NULL;
> >      vlc_mutex_init(&p_new->instance_lock);
> >      return p_new;
> > @@ -74,24 +74,13 @@ error:
> >  void libvlc_retain( libvlc_instance_t *p_instance )
> >  {
> >      assert( p_instance != NULL );
> > -    assert( p_instance->ref_count < UINT_MAX );
> >  
> > -    vlc_mutex_lock( &p_instance->instance_lock );
> > -    p_instance->ref_count++;
> > -    vlc_mutex_unlock( &p_instance->instance_lock );
> > +    vlc_atomic_rc_inc( &p_instance->ref_count );
> >  }
> >  
> >  void libvlc_release( libvlc_instance_t *p_instance )
> >  {
> > -    vlc_mutex_t *lock = &p_instance->instance_lock;
> > -    int refs;
> > -
> > -    vlc_mutex_lock( lock );
> > -    assert( p_instance->ref_count > 0 );
> > -    refs = --p_instance->ref_count;
> > -    vlc_mutex_unlock( lock );
> > -
> > -    if( refs == 0 )
> > +    if(vlc_atomic_rc_dec( &p_instance->ref_count ))
> >      {
> >          libvlc_Quit( p_instance->p_libvlc_int );
> >          libvlc_InternalCleanup( p_instance->p_libvlc_int );
> > diff --git a/lib/libvlc_internal.h b/lib/libvlc_internal.h
> > index 5c1107b84f..6c8ff49a81 100644
> > --- a/lib/libvlc_internal.h
> > +++ b/lib/libvlc_internal.h
> > @@ -33,6 +33,7 @@
> >  #include <vlc/libvlc_picture.h>
> >  #include <vlc/libvlc_media.h>
> >  #include <vlc/libvlc_events.h>
> > +#include <vlc_atomic.h>
> >  
> >  #include <vlc_common.h>
> >  
> > @@ -60,7 +61,7 @@ VLC_API void libvlc_SetExitHandler( libvlc_int_t *, void (*) (void *), void * );
> >  struct libvlc_instance_t
> >  {
> >      libvlc_int_t *p_libvlc_int;
> > -    unsigned      ref_count;
> > +    vlc_atomic_rc_t ref_count;
> >      vlc_mutex_t   instance_lock;
> >      struct libvlc_callback_entry_list_t *p_callback_list;
> >      struct
> 
> -- 
> 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

It's still used by libvlc_dialog_set_callbacks, but I can't see any other use, which feels like we're missing a lock when reading from the callback stored in the instance.

-- 
  Hugo Beauzée-Luyssen
  hugo at beauzee.fr