[vlc-commits] Fix #5706

[Rafaël Carré]

vlc | branch: master | Rafaël Carré <funman at videolan.org> | Fri Nov 11 14:40:21 2011 -0500| [673d65624392724baaac130601a64bd624a6738f] | committer: Rafaël Carré

Fix #5706

Properly take the recursive event_sending_lock when looking for listener to
send the event.
Ensure that there will be no event dispatch after vlc_event_detach().

Also, fix a bug in libvlc_event where removing an event handler from an event
callback would lead to a crash.

> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=673d65624392724baaac130601a64bd624a6738f
---

 lib/event.c       |   14 +++++++++-----
 src/misc/events.c |   20 +++++++++++++-------
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/lib/event.c b/lib/event.c
index 2ce0a23..753480e 100644
--- a/lib/event.c
+++ b/lib/event.c
@@ -157,8 +157,7 @@ void libvlc_event_send( libvlc_event_manager_t * p_em,
     /* Fill event with the sending object now */
     p_event->p_obj = p_em->p_obj;
 
-    /* Here a read/write lock would be nice */
-
+    vlc_mutex_lock( &p_em->event_sending_lock );
     vlc_mutex_lock( &p_em->object_lock );
     for( i = 0; i < vlc_array_count(&p_em->listeners_groups); i++)
     {
@@ -175,6 +174,7 @@ void libvlc_event_send( libvlc_event_manager_t * p_em,
             if( !array_listeners_cached )
             {
                 vlc_mutex_unlock( &p_em->object_lock );
+                vlc_mutex_unlock( &p_em->event_sending_lock );
                 fprintf(stderr, "Can't alloc memory in libvlc_event_send" );
                 return;
             }
@@ -194,14 +194,18 @@ void libvlc_event_send( libvlc_event_manager_t * p_em,
     {
         free( array_listeners_cached );
         vlc_mutex_unlock( &p_em->object_lock );
+        vlc_mutex_unlock( &p_em->event_sending_lock );
         return;
     }
 
+    /* Track item removed from *this* thread, with a simple flag. Indeed
+     * event_sending_lock is a recursive lock. This has the advantage of
+     * allowing to remove an event listener from within a callback */
+    listeners_group->b_sublistener_removed = false;
+
     vlc_mutex_unlock( &p_em->object_lock );
 
-    vlc_mutex_lock( &p_em->event_sending_lock );
     listener_cached = array_listeners_cached;
-    listeners_group->b_sublistener_removed = false;
     for( i = 0; i < i_cached_listeners; i++ )
     {
         if(listener_cached->is_asynchronous)
@@ -424,7 +428,7 @@ void libvlc_event_detach( libvlc_event_manager_t *p_event_manager,
 
                     /* Mark this group as edited so that libvlc_event_send
                      * will recheck what listener to call */
-                    listeners_group->b_sublistener_removed = false;
+                    listeners_group->b_sublistener_removed = true;
 
                     free( listener );
                     vlc_array_remove( &listeners_group->listeners, j );
diff --git a/src/misc/events.c b/src/misc/events.c
index e3b1ee0..9b63d65 100644
--- a/src/misc/events.c
+++ b/src/misc/events.c
@@ -170,7 +170,9 @@ void vlc_event_send( vlc_event_manager_t * p_em,
     /* Fill event with the sending object now */
     p_event->p_obj = p_em->p_obj;
 
+    vlc_mutex_lock( &p_em->event_sending_lock ) ;
     vlc_mutex_lock( &p_em->object_lock );
+
     FOREACH_ARRAY( listeners_group, p_em->listeners_groups )
         if( listeners_group->event_type == p_event->type )
         {
@@ -184,6 +186,7 @@ void vlc_event_send( vlc_event_manager_t * p_em,
             if( !array_of_cached_listeners )
             {
                 vlc_mutex_unlock( &p_em->object_lock );
+                vlc_mutex_unlock( &p_em->event_sending_lock ) ;
                 return;
             }
 
@@ -196,28 +199,31 @@ void vlc_event_send( vlc_event_manager_t * p_em,
             break;
         }
     FOREACH_END()
+
+    /* Track item removed from *this* thread, with a simple flag. Indeed
+     * event_sending_lock is a recursive lock. This has the advantage of
+     * allowing to remove an event listener from within a callback */
+    listeners_group->b_sublistener_removed = false;
+
     vlc_mutex_unlock( &p_em->object_lock );
- 
+
     /* Call the function attached */
     cached_listener = array_of_cached_listeners;
 
     if( !listeners_group || !array_of_cached_listeners )
     {
         free( array_of_cached_listeners );
+        vlc_mutex_unlock( &p_em->event_sending_lock );
         return;
     }
 
-    vlc_mutex_lock( &p_em->event_sending_lock ) ;
-
-    /* Track item removed from *this* thread, with a simple flag */
-    listeners_group->b_sublistener_removed = false;
 
     for( i = 0; i < i_cached_listeners; i++ )
     {
-        /* 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 */
+            /* If a callback was removed inside one of our callback, this gets
+	     * called */
             bool valid_listener;
             vlc_mutex_lock( &p_em->object_lock );
             valid_listener = group_contains_listener( listeners_group, cached_listener );