GC: thread-safety, and offset independence

include/vlc_common.h

@@ -50,6 +50,7 @@
 #include <string.h>
 #include <stdio.h>
 #include <inttypes.h>
+#include <stddef.h>
 
 #ifndef __cplusplus
 # include <stdbool.h>
@@ -142,7 +143,6 @@ typedef struct variable_t variable_t;
 typedef struct date_t date_t;
 typedef struct dict_entry_t dict_entry_t;
 typedef struct dict_t dict_t;
-typedef struct gc_object_t gc_object_t ;
 
 /* Messages */
 typedef struct msg_subscription_t msg_subscription_t;
@@ -558,30 +558,26 @@ typedef int ( * vlc_callback_t ) ( vlc_object_t *,      /* variable's object */
 # define VLC_OBJECT( x ) ((vlc_object_t *)(x))
 #endif
 
-#define VLC_GC_MEMBERS                                                       \
-/** \name VLC_GC_MEMBERS                                                     \
- * these members are common to all objects that wish to be garbage-collected \
- */                                                                          \
-/**@{*/                                                                      \
-    int i_gc_refcount;                                                       \
-    void (*pf_destructor) ( gc_object_t * );                                 \
-    void *p_destructor_arg;                                                  \
-/**@}*/
-
-struct gc_object_t
+typedef struct gc_object_t
 {
-    VLC_GC_MEMBERS
-};
+    vlc_spinlock_t spin;
+    uintptr_t      refs;
+    void          (*pf_destructor) (struct gc_object_t *);
+} gc_object_t;
 
-VLC_EXPORT(void, __vlc_gc_incref, ( gc_object_t * p_gc ));
-VLC_EXPORT(void, __vlc_gc_decref, ( gc_object_t * p_gc ));
-VLC_EXPORT(void, __vlc_gc_init, ( gc_object_t * p_gc,
-    void (*pf_destructor)( gc_object_t * ), void * arg));
+/**
+ * These members are common to all objects that wish to be garbage-collected.
+ */
+#define VLC_GC_MEMBERS gc_object_t vlc_gc_data;
 
-#define vlc_gc_incref( a ) __vlc_gc_incref( (gc_object_t *)a )
-#define vlc_gc_decref( a ) __vlc_gc_decref( (gc_object_t *)a )
-#define vlc_gc_init( a,b,c ) __vlc_gc_init( (gc_object_t *)a,b,c )
+VLC_EXPORT(void *, vlc_gc_init, (gc_object_t *, void (*)(gc_object_t *)));
+VLC_EXPORT(void *, vlc_hold, (gc_object_t *));
+VLC_EXPORT(void, vlc_release, (gc_object_t *));
 
+#define vlc_gc_init( a,b ) vlc_gc_init( &(a)->vlc_gc_data, (b) )
+#define vlc_gc_incref( a ) vlc_hold( &(a)->vlc_gc_data )
+#define vlc_gc_decref( a ) vlc_release( &(a)->vlc_gc_data )
+#define vlc_priv( gc, t ) ((t *)(((char *)(gc)) - offsetof(t, vlc_gc_data)))
 
 /*****************************************************************************
  * Macros and inline functions

include/vlc_input.h

@@ -60,6 +60,7 @@ struct input_item_t
 {
     VLC_GC_MEMBERS
     int        i_id;                 /**< Identifier of the item */
+    libvlc_int_t *p_libvlc;
 
     char       *psz_name;            /**< text describing this item */
     char       *psz_uri;             /**< mrl of this item */

src/input/item.c

@@ -367,23 +367,25 @@ char *input_item_GetInfo( input_item_t *p_i,
     return strdup( "" );
 }
 
-static void input_item_Destroy ( gc_object_t *p_this )
+static void input_item_Destroy ( gc_object_t *gc )
 {
-    vlc_object_t *p_obj = (vlc_object_t *)p_this->p_destructor_arg;
-    libvlc_priv_t *priv = libvlc_priv (p_obj->p_libvlc);
-    input_item_t *p_input = (input_item_t *) p_this;
+    input_item_t *p_input = vlc_priv(gc, input_item_t);
+    libvlc_int_t *p_libvlc = p_input->p_libvlc;
     int i;
 
     input_item_Clean( p_input );
 
-    vlc_object_lock( p_obj->p_libvlc );
+    /* This is broken. Items must be removed from any table before their
+     * reference count drops to zero (unless the table is not used, but then
+     * why have it?). Full point, no buts. -- Courmisch */
+    libvlc_priv_t *priv = libvlc_priv (p_libvlc);
+    vlc_object_lock( p_libvlc );
 
     ARRAY_BSEARCH( priv->input_items,->i_id, int, p_input->i_id, i);
     if( i != -1 )
         ARRAY_REMOVE( priv->input_items, i);
 
-    vlc_object_unlock( p_obj->p_libvlc );
-
+    vlc_object_unlock( p_libvlc );
     free( p_input );
 }
 
@@ -528,7 +530,8 @@ input_item_t *input_item_NewWithType( vlc_object_t *p_obj, const char *psz_uri,
     DECMALLOC_NULL( p_input, input_item_t );
 
     input_item_Init( p_obj, p_input );
-    vlc_gc_init( p_input, input_item_Destroy, (void *)p_obj->p_libvlc );
+    vlc_gc_init( p_input, input_item_Destroy );
+    p_input->p_libvlc = p_obj->p_libvlc;
 
     vlc_object_lock( p_obj->p_libvlc );
     p_input->i_id = ++priv->i_last_input_id;

src/libvlc.c

@@ -104,39 +104,63 @@ static unsigned          i_instances = 0;
 static bool b_daemon = false;
 #endif
 
-/*****************************************************************************
- * vlc_gc_*.
- *****************************************************************************/
-void __vlc_gc_incref( gc_object_t * p_gc )
-{
-    assert( p_gc->i_gc_refcount > 0 );
+#undef vlc_gc_init
+#undef vlc_hold
+#undef vlc_release
 
-    /* FIXME: atomic version needed! */
-    p_gc->i_gc_refcount ++;
+/**
+ * Atomically set the reference count to 1.
+ * @param p_gc reference counted object
+ * @param pf_destruct destruction calback
+ * @return p_gc.
+ */
+void *vlc_gc_init (gc_object_t *p_gc, void (*pf_destruct) (gc_object_t *))
+{
+    p_gc->pf_destructor = pf_destruct;
+
+    /* Nobody else can possibly lock the spin - it's there as a barrier */
+    vlc_spin_init (&p_gc->spin);
+    vlc_spin_lock (&p_gc->spin);
+    p_gc->refs = 1;
+    vlc_spin_unlock (&p_gc->spin);
+    return p_gc;
 }
 
-void __vlc_gc_decref( gc_object_t *p_gc )
+/**
+ * Atomically increment the reference count.
+ * @param p_gc reference counted object
+ * @return p_gc.
+ */
+void *vlc_hold (gc_object_t * p_gc)
 {
     assert( p_gc );
-    assert( p_gc->i_gc_refcount > 0 );
-
-    /* FIXME: atomic version needed! */
-    p_gc->i_gc_refcount -- ;
 
-    if( p_gc->i_gc_refcount == 0 )
-    {
-        p_gc->pf_destructor( p_gc );
-        /* Do not use the p_gc pointer from now on ! */
-    }
+    vlc_spin_lock (&p_gc->spin);
+    assert (p_gc->refs > 0);
+    p_gc->refs++;
+    vlc_spin_unlock (&p_gc->spin);
+    return p_gc;
 }
 
-void
-__vlc_gc_init( gc_object_t * p_gc, void (*pf_destructor)( gc_object_t * ),
-               void * arg)
+/**
+ * Atomically decrement the reference count and, if it reaches zero, destroy.
+ * @param p_gc reference counted object.
+ */
+void vlc_release (gc_object_t *p_gc)
 {
-    p_gc->i_gc_refcount = 1;
-    p_gc->pf_destructor = pf_destructor;
-    p_gc->p_destructor_arg = arg;
+    bool dead;
+
+    assert( p_gc );
+    vlc_spin_lock (&p_gc->spin);
+    assert (p_gc->refs > 0);
+    dead = !--p_gc->refs;
+    vlc_spin_unlock (&p_gc->spin);
+
+    if (dead)
+    {
+        vlc_spin_destroy (&p_gc->spin);
+        p_gc->pf_destructor (p_gc);
+    }
 }
 
 /*****************************************************************************
@@ -1025,8 +1049,8 @@ int libvlc_InternalCleanup( libvlc_int_t *p_libvlc )
 
     bool b_clean = true;
     FOREACH_ARRAY( input_item_t *p_del, priv->input_items )
-        msg_Err( p_libvlc, "input item %p has not been deleted properly: refcount %d, name %s",
-            p_del, p_del->i_gc_refcount, p_del->psz_name ? p_del->psz_name : "(null)" );
+        msg_Err( p_libvlc, "input item %p has not been deleted properly: name %s",
+            p_del, p_del->psz_name ? p_del->psz_name : "(null)" );
         b_clean = false;
     FOREACH_END();
     assert( b_clean );

src/libvlccore.sym

@@ -448,12 +448,11 @@ __vlc_execve
 vlc_fastmem_register
 vlc_freeaddrinfo
 vlc_gai_strerror
-__vlc_gc_decref
-__vlc_gc_incref
-__vlc_gc_init
+vlc_gc_init
 vlc_getaddrinfo
 vlc_getnameinfo
 vlc_gettext
+vlc_hold
 vlc_iconv
 vlc_iconv_close
 vlc_iconv_open
@@ -490,6 +489,7 @@ __vlc_object_yield
 vlc_poll
 vlc_rand_bytes
 vlc_recvmsg
+vlc_release
 vlc_sdp_Start
 vlc_sendmsg
 vlc_strcasestr