misc/events.c: Fix event sending, by properly supporting event_manager edition...

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.

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;

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 )
 {

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
  *****************************************************************************/

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;
 }
+

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
  *****************************************************************************/