From: Pierre d'Herbemont Date: Tue, 1 Jan 2008 14:33:18 +0000 (+0000) Subject: misc/events.c: Fix event sending, by properly supporting event_manager edition (event... X-Git-Tag: 0.9.0-test0~3777 X-Git-Url: https://git.sesse.net/?a=commitdiff_plain;h=192a2076ddccdb60053ae5be615ae1303353bbe4;p=vlc misc/events.c: Fix event sending, by properly supporting event_manager edition (event_detach) while executing a callback (vlc_event_send). This is done through a recursive lock. --- diff --git a/include/vlc_events.h b/include/vlc_events.h index ec80b17414..29a89149b1 100644 --- a/include/vlc_events.h +++ b/include/vlc_events.h @@ -104,6 +104,7 @@ typedef struct vlc_event_manager_t { void * p_obj; vlc_mutex_t object_lock; + vlc_mutex_t event_sending_lock; vlc_object_t *p_parent_object; DECL_ARRAY(struct vlc_event_listeners_group_t *) listeners_groups; } vlc_event_manager_t; diff --git a/include/vlc_input.h b/include/vlc_input.h index 314543f276..08d13d8b88 100644 --- a/include/vlc_input.h +++ b/include/vlc_input.h @@ -198,21 +198,7 @@ static inline void input_ItemClean( input_item_t *p_i ) vlc_mutex_destroy( &p_i->lock ); } -static inline void input_item_SetMeta( input_item_t *p_i, vlc_meta_type_t meta_type, const char *psz_val ) -{ - vlc_event_t event; - - vlc_mutex_lock( &p_i->lock ); - if( !p_i->p_meta ) - p_i->p_meta = vlc_meta_New(); - vlc_meta_Set( p_i->p_meta, meta_type, psz_val ); - vlc_mutex_unlock( &p_i->lock ); - - /* Notify interested third parties */ - event.type = vlc_InputItemMetaChanged; - event.u.input_item_meta_changed.meta_type = meta_type; - vlc_event_send( &p_i->event_manager, &event ); -} +VLC_EXPORT( void, input_item_SetMeta, ( input_item_t *p_i, vlc_meta_type_t meta_type, const char *psz_val )); static inline vlc_bool_t input_item_MetaMatch( input_item_t *p_i, vlc_meta_type_t meta_type, const char *psz ) { diff --git a/include/vlc_threads_funcs.h b/include/vlc_threads_funcs.h index d77e29ac46..2823aa8385 100644 --- a/include/vlc_threads_funcs.h +++ b/include/vlc_threads_funcs.h @@ -37,6 +37,7 @@ *****************************************************************************/ VLC_EXPORT( void, __vlc_threads_error, ( vlc_object_t *) ); VLC_EXPORT( int, __vlc_mutex_init, ( vlc_object_t *, vlc_mutex_t * ) ); +VLC_EXPORT( int, __vlc_mutex_init_recursive, ( vlc_object_t *, vlc_mutex_t * ) ); VLC_EXPORT( int, __vlc_mutex_destroy, ( const char *, int, vlc_mutex_t * ) ); VLC_EXPORT( int, __vlc_cond_init, ( vlc_object_t *, vlc_cond_t * ) ); VLC_EXPORT( int, __vlc_cond_destroy, ( const char *, int, vlc_cond_t * ) ); @@ -70,6 +71,12 @@ VLC_EXPORT( void, __vlc_thread_join, ( vlc_object_t *, const char *, int ) ); #define vlc_mutex_init( P_THIS, P_MUTEX ) \ __vlc_mutex_init( VLC_OBJECT(P_THIS), P_MUTEX ) +/***************************************************************************** + * vlc_mutex_init: initialize a recursive mutex (Don't use it) + *****************************************************************************/ +#define vlc_mutex_init_recursive( P_THIS, P_MUTEX ) \ + __vlc_mutex_init_recursive( VLC_OBJECT(P_THIS), P_MUTEX ) + /***************************************************************************** * vlc_mutex_lock: lock a mutex *****************************************************************************/ diff --git a/src/misc/events.c b/src/misc/events.c index 6e056f7ff7..67ffb3fee5 100644 --- a/src/misc/events.c +++ b/src/misc/events.c @@ -58,6 +58,12 @@ typedef struct vlc_event_listeners_group_t { vlc_event_type_t event_type; DECL_ARRAY(struct vlc_event_listener_t *) listeners; + + /* Used in vlc_event_send() to make sure to behave + Correctly when vlc_event_detach was called during + a callback */ + vlc_bool_t b_sublistener_removed; + } vlc_event_listeners_group_t; #ifdef DEBUG_EVENT @@ -70,6 +76,18 @@ static const char * ppsz_event_type_to_name[] = }; #endif +static vlc_bool_t +group_contains_listener( vlc_event_listeners_group_t * group, + vlc_event_listener_t * searched_listener ) +{ + vlc_event_listener_t * listener; + FOREACH_ARRAY( listener, group->listeners ) + if( searched_listener == listener ) + return VLC_TRUE; + FOREACH_END() + return VLC_FALSE; +} + /***************************************************************************** * *****************************************************************************/ @@ -87,6 +105,14 @@ int vlc_event_manager_init( vlc_event_manager_t * p_em, void * p_obj, p_em->p_obj = p_obj; p_em->p_parent_object = p_parent_obj; vlc_mutex_init( p_parent_obj, &p_em->object_lock ); + + /* We need a recursive lock here, because we need to be able + * to call libvlc_event_detach even if vlc_event_send is in + * the call frame. + * This ensures that after libvlc_event_detach, the callback + * will never gets triggered. + * */ + vlc_mutex_init_recursive( p_parent_obj, &p_em->event_sending_lock ); ARRAY_INIT( p_em->listeners_groups ); return VLC_SUCCESS; } @@ -100,6 +126,7 @@ void vlc_event_manager_fini( vlc_event_manager_t * p_em ) struct vlc_event_listener_t * listener; vlc_mutex_destroy( &p_em->object_lock ); + vlc_mutex_destroy( &p_em->event_sending_lock ); FOREACH_ARRAY( listeners_group, p_em->listeners_groups ) FOREACH_ARRAY( listener, listeners_group->listeners ) @@ -183,6 +210,11 @@ void vlc_event_send( vlc_event_manager_t * p_em, /* Call the function attached */ cached_listener = array_of_cached_listeners; + vlc_mutex_lock( &p_em->event_sending_lock ); + + /* Track item removed from *this* thread, with a simple flag */ + listeners_group->b_sublistener_removed = VLC_FALSE; + for( i = 0; i < i_cached_listeners; i++ ) { #ifdef DEBUG_EVENT @@ -193,12 +225,29 @@ void vlc_event_send( vlc_event_manager_t * p_em, cached_listener->p_user_data ); free(cached_listener->psz_debug_name); #endif - + /* No need to lock on listeners_group, a listener group can't be removed */ + if( listeners_group->b_sublistener_removed ) + { + /* If a callback was removed, this gets called */ + vlc_bool_t valid_listener; + vlc_mutex_lock( &p_em->object_lock ); + valid_listener = group_contains_listener( listeners_group, cached_listener ); + vlc_mutex_unlock( &p_em->object_lock ); + if( !valid_listener ) + { +#ifdef DEBUG_EVENT + msg_Dbg( p_em->p_parent_object, "Callback was removed during execution" ); +#endif + cached_listener++; + continue; + } + } cached_listener->pf_callback( p_event, cached_listener->p_user_data ); cached_listener++; } - free( array_of_cached_listeners ); + vlc_mutex_unlock( &p_em->event_sending_lock ); + free( array_of_cached_listeners ); } /** @@ -250,6 +299,7 @@ int __vlc_event_attach( vlc_event_manager_t * p_em, /** * Remove a callback for an event. */ + int vlc_event_detach( vlc_event_manager_t *p_em, vlc_event_type_t event_type, vlc_event_callback_t pf_callback, @@ -259,6 +309,7 @@ int vlc_event_detach( vlc_event_manager_t *p_em, struct vlc_event_listener_t * listener; vlc_mutex_lock( &p_em->object_lock ); + vlc_mutex_lock( &p_em->event_sending_lock ); FOREACH_ARRAY( listeners_group, p_em->listeners_groups ) if( listeners_group->event_type == event_type ) { @@ -266,6 +317,10 @@ int vlc_event_detach( vlc_event_manager_t *p_em, if( listener->pf_callback == pf_callback && listener->p_user_data == p_user_data ) { + /* Tell vlc_event_send, we did remove an item from that group, + in case vlc_event_send is in our caller stack */ + listeners_group->b_sublistener_removed = VLC_TRUE; + /* that's our listener */ ARRAY_REMOVE( listeners_group->listeners, fe_idx /* This comes from the macro (and that's why @@ -280,15 +335,18 @@ int vlc_event_detach( vlc_event_manager_t *p_em, free( listener->psz_debug_name ); #endif free( listener ); + vlc_mutex_unlock( &p_em->event_sending_lock ); vlc_mutex_unlock( &p_em->object_lock ); return VLC_SUCCESS; } FOREACH_END() } FOREACH_END() + vlc_mutex_unlock( &p_em->event_sending_lock ); vlc_mutex_unlock( &p_em->object_lock ); msg_Warn( p_em->p_parent_object, "Can't detach to an object event manager event" ); return VLC_EGENERIC; } + diff --git a/src/misc/threads.c b/src/misc/threads.c index f210b287da..908f510bc4 100644 --- a/src/misc/threads.c +++ b/src/misc/threads.c @@ -86,7 +86,7 @@ vlc_threadvar_t msg_context_global_key; void __vlc_threads_error( vlc_object_t *p_this ) { - msg_Err( p_this, "Error detected. Put a breakpoint in '%s' to debug.", + msg_Err( p_this, "Error detected. Put a breakpoint in '%s' to debug.", __func__ ); } @@ -321,6 +321,7 @@ int __vlc_mutex_init( vlc_object_t *p_this, vlc_mutex_t *p_mutex ) # else pthread_mutexattr_settype( &attr, PTHREAD_MUTEX_ERRORCHECK ); # endif + i_result = pthread_mutex_init( &p_mutex->mutex, &attr ); pthread_mutexattr_destroy( &attr ); return( i_result ); @@ -335,6 +336,40 @@ int __vlc_mutex_init( vlc_object_t *p_this, vlc_mutex_t *p_mutex ) #endif } +/***************************************************************************** + * vlc_mutex_init: initialize a recursive mutex (Do not use) + *****************************************************************************/ +int __vlc_mutex_init_recursive( vlc_object_t *p_this, vlc_mutex_t *p_mutex ) +{ +#if defined( WIN32 ) + /* Create mutex returns a recursive mutex */ + p_mutex->mutex = CreateMutex( 0, FALSE, 0 ); + return ( p_mutex->mutex != NULL ? 0 : 1 ); +#elif defined( PTHREAD_COND_T_IN_PTHREAD_H ) + pthread_mutexattr_t attr; + int i_result; + + pthread_mutexattr_init( &attr ); +# if defined(DEBUG) + /* Create error-checking mutex to detect problems more easily. */ +# if defined(SYS_LINUX) + pthread_mutexattr_setkind_np( &attr, PTHREAD_MUTEX_ERRORCHECK_NP ); +# else + pthread_mutexattr_settype( &attr, PTHREAD_MUTEX_ERRORCHECK ); +# endif +# endif + pthread_mutexattr_settype( &attr, PTHREAD_MUTEX_RECURSIVE ); + i_result = pthread_mutex_init( &p_mutex->mutex, &attr ); + pthread_mutexattr_destroy( &attr ); + return( i_result ); +#else + msg_Err(p_this, "no recursive mutex found. Falling back to regular mutex.\n" + "Expect hangs\n") + return __vlc_mutex_init( p_this, p_mutex ); +#endif +} + + /***************************************************************************** * vlc_mutex_destroy: destroy a mutex, inner version *****************************************************************************/