Commit 5c2c82ed authored by Rémi Denis-Courmont's avatar Rémi Denis-Courmont

event: remove recursive deletion

In theory, vlc_event_detach() can be called from within the event
handler. In practice, callers of vlc_event_detach() expect that the
event handler is not pending after the function returns. This would not
work if recursion actually occurred, it would lead to use-after-free.

This removes recursion, including memory allocation, copying and missing
error handling in event sending.
parent 5e407d5f
......@@ -111,20 +111,13 @@ typedef enum vlc_event_type_t {
typedef struct vlc_event_listeners_group_t
{
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 */
bool b_sublistener_removed;
} vlc_event_listeners_group_t;
/* Event manager type */
typedef struct vlc_event_manager_t
{
void * p_obj;
vlc_mutex_t object_lock;
vlc_mutex_t event_sending_lock;
vlc_mutex_t lock;
vlc_event_listeners_group_t events[vlc_InputItemPreparseEnded + 1];
} vlc_event_manager_t;
......
......@@ -52,26 +52,6 @@ typedef struct vlc_event_listener_t
vlc_event_callback_t pf_callback;
} vlc_event_listener_t;
static bool
listeners_are_equal( vlc_event_listener_t * listener1,
vlc_event_listener_t * listener2 )
{
return listener1->pf_callback == listener2->pf_callback &&
listener1->p_user_data == listener2->p_user_data;
}
static bool
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( listeners_are_equal(searched_listener, listener) )
return true;
FOREACH_END()
return false;
}
/*****************************************************************************
*
*****************************************************************************/
......@@ -86,15 +66,7 @@ group_contains_listener( vlc_event_listeners_group_t * group,
void vlc_event_manager_init( vlc_event_manager_t * p_em, void * p_obj )
{
p_em->p_obj = p_obj;
vlc_mutex_init( &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_em->event_sending_lock );
vlc_mutex_init( &p_em->lock );
for( size_t i = 0; i < ARRAY_SIZE(p_em->events); i++ )
ARRAY_INIT( p_em->events[i].listeners );
......@@ -107,8 +79,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 );
vlc_mutex_destroy( &p_em->lock );
for( size_t i = 0; i < ARRAY_SIZE(p_em->events); i++ )
{
......@@ -129,69 +100,17 @@ void vlc_event_send( vlc_event_manager_t * p_em,
{
vlc_event_listeners_group_t *slot = &p_em->events[p_event->type];
vlc_event_listener_t * listener;
vlc_event_listener_t * array_of_cached_listeners = NULL;
vlc_event_listener_t * cached_listener;
int i, i_cached_listeners = 0;
/* 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 );
vlc_mutex_lock( &p_em->lock ) ;
if( slot->listeners.i_size <= 0 )
{
vlc_mutex_unlock( &p_em->object_lock );
vlc_mutex_unlock( &p_em->event_sending_lock ) ;
return;
}
/* Save the function to call */
i_cached_listeners = slot->listeners.i_size;
array_of_cached_listeners = malloc(
sizeof(vlc_event_listener_t)*i_cached_listeners );
if( unlikely(!array_of_cached_listeners) )
abort();
cached_listener = array_of_cached_listeners;
FOREACH_ARRAY( listener, slot->listeners )
memcpy( cached_listener, listener, sizeof(vlc_event_listener_t) );
cached_listener++;
listener->pf_callback( p_event, listener->p_user_data );
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 */
slot->b_sublistener_removed = false;
vlc_mutex_unlock( &p_em->object_lock );
/* Call the function attached */
cached_listener = array_of_cached_listeners;
for( i = 0; i < i_cached_listeners; i++ )
{
if( slot->b_sublistener_removed )
{
/* 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( slot, cached_listener );
vlc_mutex_unlock( &p_em->object_lock );
if( !valid_listener )
{
cached_listener++;
continue;
}
}
cached_listener->pf_callback( p_event, cached_listener->p_user_data );
cached_listener++;
}
vlc_mutex_unlock( &p_em->event_sending_lock );
free( array_of_cached_listeners );
vlc_mutex_unlock( &p_em->lock );
}
#undef vlc_event_attach
......@@ -213,9 +132,9 @@ int vlc_event_attach( vlc_event_manager_t * p_em,
listener->p_user_data = p_user_data;
listener->pf_callback = pf_callback;
vlc_mutex_lock( &p_em->object_lock );
vlc_mutex_lock( &p_em->lock );
ARRAY_APPEND( slot->listeners, listener );
vlc_mutex_unlock( &p_em->object_lock );
vlc_mutex_unlock( &p_em->lock );
return VLC_SUCCESS;
}
......@@ -231,24 +150,18 @@ void vlc_event_detach( vlc_event_manager_t *p_em,
vlc_event_listeners_group_t *slot = &p_em->events[event_type];
struct vlc_event_listener_t * listener;
vlc_mutex_lock( &p_em->event_sending_lock );
vlc_mutex_lock( &p_em->object_lock );
vlc_mutex_lock( &p_em->lock );
FOREACH_ARRAY( listener, slot->listeners )
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 */
slot->b_sublistener_removed = true;
/* that's our listener */
ARRAY_REMOVE( slot->listeners,
fe_idx /* This comes from the macro (and that's why
I hate macro) */ );
vlc_mutex_unlock( &p_em->lock );
free( listener );
vlc_mutex_unlock( &p_em->event_sending_lock );
vlc_mutex_unlock( &p_em->object_lock );
return;
}
FOREACH_END()
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment