From: Pierre d'Herbemont Date: Fri, 1 Jun 2007 20:23:10 +0000 (+0000) Subject: libvlc event: Make event function thread safe. (And fix a mutex leak) X-Git-Tag: 0.9.0-test0~7173 X-Git-Url: https://git.sesse.net/?a=commitdiff_plain;h=593aeb2b2a5d78c502cc3aa1c27d9c65559e6efd;p=vlc libvlc event: Make event function thread safe. (And fix a mutex leak) --- diff --git a/src/control/core.c b/src/control/core.c index 23bfa0629c..48614cd7e3 100644 --- a/src/control/core.c +++ b/src/control/core.c @@ -101,6 +101,7 @@ libvlc_instance_t * libvlc_new( int argc, char **argv, p_new->b_playlist_locked = 0; p_new->p_callback_list = NULL; vlc_mutex_init(p_libvlc_int, &p_new->instance_lock); + vlc_mutex_init(p_libvlc_int, &p_new->event_callback_lock); return p_new; } @@ -108,6 +109,8 @@ libvlc_instance_t * libvlc_new( int argc, char **argv, void libvlc_destroy( libvlc_instance_t *p_instance, libvlc_exception_t *p_e ) { libvlc_event_remove_all_callbacks( p_instance, p_e /* current implementation never triggers it */); + vlc_mutex_destroy( &p_instance->instance_lock ); + vlc_mutex_destroy( &p_instance->event_callback_lock); libvlc_InternalCleanup( p_instance->p_libvlc_int ); libvlc_InternalDestroy( p_instance->p_libvlc_int, VLC_FALSE ); } diff --git a/src/control/event.c b/src/control/event.c index 770348fcac..08685ab68f 100644 --- a/src/control/event.c +++ b/src/control/event.c @@ -33,7 +33,9 @@ static int handle_event( vlc_object_t *p_this, char const *psz_cmd, vlc_value_t oldval, vlc_value_t newval, void *p_data ) { - struct libvlc_callback_entry_t *entry = p_data; /* FIXME: we need some locking here */ + /* This is thread safe, as the var_*Callback already provide the locking + * facility for p_data */ + struct libvlc_callback_entry_t *entry = p_data; libvlc_event_t event; event.type = entry->i_event_type; switch ( event.type ) @@ -55,8 +57,8 @@ static int handle_event( vlc_object_t *p_this, char const *psz_cmd, return VLC_SUCCESS; } -static inline void add_callback_entry( struct libvlc_callback_entry_t *entry, - struct libvlc_callback_entry_list_t **list ) +static inline void add_callback_to_list( struct libvlc_callback_entry_t *entry, + struct libvlc_callback_entry_list_t **list ) { struct libvlc_callback_entry_list_t *new_listitem; @@ -77,6 +79,30 @@ static inline void add_callback_entry( struct libvlc_callback_entry_t *entry, *list = new_listitem; } +static int remove_variable_callback( libvlc_instance_t *p_instance, + struct libvlc_callback_entry_t * p_entry ) +{ + const char * callback_name = NULL; + + /* Note: Appropriate lock should be held by the caller */ + + switch ( p_entry->i_event_type ) + { + case VOLUME_CHANGED: + callback_name = "volume-change"; + break; + case INPUT_POSITION_CHANGED: + break; + } + + if (!callback_name) + return VLC_EGENERIC; + + return var_DelCallback( p_instance->p_libvlc_int, + callback_name, handle_event, + p_entry ); +} + /* * Public libvlc functions */ @@ -88,7 +114,7 @@ void libvlc_event_add_callback( libvlc_instance_t *p_instance, libvlc_exception_t *p_e ) { struct libvlc_callback_entry_t *entry; - const char *callback_name = NULL; + const char * callback_name = NULL; int res; if ( !f_callback ) @@ -129,7 +155,9 @@ void libvlc_event_add_callback( libvlc_instance_t *p_instance, RAISEVOID("Internal callback registration was not successful. Callback not registered."); } - add_callback_entry( entry, &p_instance->p_callback_list ); + vlc_mutex_lock( &p_instance->instance_lock ); + add_callback_to_list( entry, &p_instance->p_callback_list ); + vlc_mutex_unlock( &p_instance->instance_lock ); return; } @@ -137,18 +165,21 @@ void libvlc_event_add_callback( libvlc_instance_t *p_instance, void libvlc_event_remove_all_callbacks( libvlc_instance_t *p_instance, libvlc_exception_t *p_e ) { - struct libvlc_callback_entry_list_t *p_listitem = p_instance->p_callback_list; + struct libvlc_callback_entry_list_t *p_listitem; + + vlc_mutex_lock( &p_instance->instance_lock ); + + p_listitem = p_instance->p_callback_list; while( p_listitem ) { - libvlc_event_remove_callback( p_instance, - p_listitem->elmt->i_event_type, - p_listitem->elmt->f_callback, - p_listitem->elmt->p_user_data, - p_e); - /* libvlc_event_remove_callback will reset the p_callback_list */ - p_listitem = p_instance->p_callback_list; + remove_variable_callback( p_instance, p_listitem->elmt ); /* FIXME: We could warn on error */ + p_listitem = p_listitem->next; + } + p_instance->p_callback_list = NULL; + + vlc_mutex_unlock( &p_instance->instance_lock ); } void libvlc_event_remove_callback( libvlc_instance_t *p_instance, @@ -157,7 +188,11 @@ void libvlc_event_remove_callback( libvlc_instance_t *p_instance, void *p_user_data, libvlc_exception_t *p_e ) { - struct libvlc_callback_entry_list_t *p_listitem = p_instance->p_callback_list; + struct libvlc_callback_entry_list_t *p_listitem; + + vlc_mutex_lock( &p_instance->instance_lock ); + + p_listitem = p_instance->p_callback_list; while( p_listitem ) { @@ -167,40 +202,23 @@ void libvlc_event_remove_callback( libvlc_instance_t *p_instance, ) { - const char * callback_name = NULL; - int res; + remove_variable_callback( p_instance, p_listitem->elmt ); /* FIXME: We should warn on error */ if( p_listitem->prev ) p_listitem->prev->next = p_listitem->next; else p_instance->p_callback_list = p_listitem->next; + p_listitem->next->prev = p_listitem->prev; - switch ( i_event_type ) - { - case VOLUME_CHANGED: - callback_name = "volume-change"; - break; - case INPUT_POSITION_CHANGED: - break; - default: - RAISEVOID( "Unsupported event." ); - } - - res = var_DelCallback( p_instance->p_libvlc_int, - callback_name, handle_event, - p_listitem->elmt ); - - if (res != VLC_SUCCESS) - { - RAISEVOID("Internal callback unregistration was not successful. Callback not unregistered."); - } - - free( p_listitem->elmt ); /* FIXME: need some locking here */ + free( p_listitem->elmt ); + free( p_listitem ); break; } + p_listitem = p_listitem->next; } + vlc_mutex_unlock( &p_instance->instance_lock ); } diff --git a/src/control/libvlc_internal.h b/src/control/libvlc_internal.h index 309e920938..d762d71b89 100644 --- a/src/control/libvlc_internal.h +++ b/src/control/libvlc_internal.h @@ -68,6 +68,7 @@ struct libvlc_instance_t vlm_t *p_vlm; int b_playlist_locked; vlc_mutex_t instance_lock; + vlc_mutex_t event_callback_lock; struct libvlc_callback_entry_list_t *p_callback_list; };