From cffcf64f36b2112eefeb5ddb3cf8de93e69c3cdf Mon Sep 17 00:00:00 2001 From: =?utf8?q?R=C3=A9mi=20Denis-Courmont?= Date: Sun, 11 Jan 2009 19:05:00 +0200 Subject: [PATCH] Cleanup interaction-capable interface registration * Make registration thread-safe, and independent on vlc_list_find(). * Fix use-after-free race condition when destroying the interface (interaction will be lost instead). --- include/vlc_interface.h | 6 +-- modules/gui/macosx/intf.m | 4 +- modules/gui/qt4/main_interface.cpp | 4 +- modules/gui/skins2/src/vlcproc.cpp | 4 +- src/interface/interaction.c | 72 +++++++++++++++++++----------- src/interface/interface.c | 1 - src/libvlc.h | 1 + src/libvlccore.sym | 2 + 8 files changed, 58 insertions(+), 36 deletions(-) diff --git a/include/vlc_interface.h b/include/vlc_interface.h index edd012c32e..feb738acd6 100644 --- a/include/vlc_interface.h +++ b/include/vlc_interface.h @@ -67,9 +67,6 @@ struct intf_thread_t void ( *pf_show_dialog ) ( intf_thread_t *, int, int, intf_dialog_args_t * ); - /** Interaction stuff */ - bool b_interaction; - vlc_mutex_t change_lock; }; @@ -108,6 +105,9 @@ VLC_EXPORT( void, intf_StopThread, ( intf_thread_t * ) ); #define intf_Eject(a,b) __intf_Eject(VLC_OBJECT(a),b) VLC_EXPORT( int, __intf_Eject, ( vlc_object_t *, const char * ) ); +VLC_EXPORT( int, interaction_Register, ( intf_thread_t * ) ); +VLC_EXPORT( int, interaction_Unregister, ( intf_thread_t * ) ); + /*@}*/ /***************************************************************************** diff --git a/modules/gui/macosx/intf.m b/modules/gui/macosx/intf.m index ce0043224b..902af53d84 100644 --- a/modules/gui/macosx/intf.m +++ b/modules/gui/macosx/intf.m @@ -409,7 +409,7 @@ static VLCMain *_o_sharedMainInstance = nil; var_Create( p_intf, "interaction", VLC_VAR_ADDRESS ); var_AddCallback( p_intf, "interaction", InteractCallback, self ); - p_intf->b_interaction = true; + interaction_Register( p_intf ); /* update the playmode stuff */ p_intf->p_sys->b_playmode_update = true; @@ -685,7 +685,7 @@ static VLCMain *_o_sharedMainInstance = nil; [o_extended savePrefs]; } - p_intf->b_interaction = false; + interaction_Unregister( p_intf ); var_DelCallback( p_intf, "interaction", InteractCallback, self ); /* remove global observer watching for vout device changes correctly */ diff --git a/modules/gui/qt4/main_interface.cpp b/modules/gui/qt4/main_interface.cpp index 747bd623dd..97cf1e185f 100644 --- a/modules/gui/qt4/main_interface.cpp +++ b/modules/gui/qt4/main_interface.cpp @@ -186,7 +186,7 @@ MainInterface::MainInterface( intf_thread_t *_p_intf ) : QVLCMW( _p_intf ) ************/ var_Create( p_intf, "interaction", VLC_VAR_ADDRESS ); var_AddCallback( p_intf, "interaction", InteractCallback, this ); - p_intf->b_interaction = true; + interaction_Register( p_intf ); var_AddCallback( p_intf->p_libvlc, "intf-show", IntfShowCB, p_intf ); @@ -267,7 +267,7 @@ MainInterface::~MainInterface() /* Unregister callback for the intf-popupmenu variable */ var_DelCallback( p_intf->p_libvlc, "intf-popupmenu", PopupMenuCB, p_intf ); - p_intf->b_interaction = false; + interaction_Unregister( p_intf ); var_DelCallback( p_intf, "interaction", InteractCallback, this ); p_intf->p_sys->p_mi = NULL; diff --git a/modules/gui/skins2/src/vlcproc.cpp b/modules/gui/skins2/src/vlcproc.cpp index 4d9e862c9e..451e7be48c 100644 --- a/modules/gui/skins2/src/vlcproc.cpp +++ b/modules/gui/skins2/src/vlcproc.cpp @@ -164,7 +164,7 @@ VlcProc::VlcProc( intf_thread_t *pIntf ): SkinObject( pIntf ), // Called when we have an interaction dialog to display var_Create( pIntf, "interaction", VLC_VAR_ADDRESS ); var_AddCallback( pIntf, "interaction", onInteraction, this ); - pIntf->b_interaction = true; + interaction_Register( pIntf ); getIntf()->p_sys->p_input = NULL; } @@ -179,6 +179,8 @@ VlcProc::~VlcProc() vlc_object_release( getIntf()->p_sys->p_input ); } + interaction_Unregister( getIntf() ); + var_DelCallback( getIntf()->p_sys->p_playlist, "intf-change", onIntfChange, this ); var_DelCallback( getIntf()->p_sys->p_playlist, "item-append", diff --git a/src/interface/interaction.c b/src/interface/interaction.c index 2be3317600..cd617b8d68 100644 --- a/src/interface/interaction.c +++ b/src/interface/interaction.c @@ -47,7 +47,7 @@ * Local prototypes *****************************************************************************/ static interaction_t * InteractionGet( vlc_object_t * ); -static void InteractionSearchInterface( interaction_t * ); +static intf_thread_t * SearchInterface( interaction_t * ); static void* InteractionLoop( vlc_object_t * ); static void InteractionManage( interaction_t * ); @@ -401,6 +401,39 @@ void interaction_Destroy( interaction_t *p_interaction ) vlc_object_release( p_interaction ); } +static vlc_mutex_t intf_lock = VLC_STATIC_MUTEX; + +int interaction_Register( intf_thread_t *intf ) +{ + libvlc_priv_t *priv = libvlc_priv( intf->p_libvlc ); + int ret = VLC_EGENERIC; + + vlc_mutex_lock( &intf_lock ); + if( priv->p_interaction_intf == NULL ) + { /* Since the interface is responsible for unregistering itself before + * it terminates, an object reference is not needed. */ + priv->p_interaction_intf = intf; + ret = VLC_SUCCESS; + } + vlc_mutex_unlock( &intf_lock ); + return ret; +} + +int interaction_Unregister( intf_thread_t *intf ) +{ + libvlc_priv_t *priv = libvlc_priv( intf->p_libvlc ); + int ret = VLC_EGENERIC; + + vlc_mutex_lock( &intf_lock ); + if( priv->p_interaction_intf == intf ) + { + priv->p_interaction_intf = NULL; + ret = VLC_SUCCESS; + } + vlc_mutex_unlock( &intf_lock ); + return ret; +} + /********************************************************************** * The following functions are local **********************************************************************/ @@ -415,32 +448,19 @@ static interaction_t * InteractionGet( vlc_object_t *p_this ) } -/* Look for an interface suitable for interaction */ -static void InteractionSearchInterface( interaction_t *p_interaction ) +/* Look for an interface suitable for interaction, and hold it. */ +static intf_thread_t *SearchInterface( interaction_t *p_interaction ) { - vlc_list_t *p_list; - int i_index; + libvlc_priv_t *priv = libvlc_priv( p_interaction->p_libvlc ); + intf_thread_t *intf; - p_interaction->p_intf = NULL; - - p_list = vlc_list_find( p_interaction, VLC_OBJECT_INTF, FIND_ANYWHERE ); - if( !p_list ) - { - msg_Err( p_interaction, "unable to create module list" ); - return; - } + vlc_mutex_lock( &intf_lock ); + intf = priv->p_interaction_intf; + if( intf != NULL ) + vlc_object_hold( intf ); + vlc_mutex_unlock( &intf_lock ); - for( i_index = 0; i_index < p_list->i_count; i_index ++ ) - { - intf_thread_t *p_intf = (intf_thread_t *) - p_list->p_values[i_index].p_object; - if( p_intf->b_interaction ) - { - p_interaction->p_intf = p_intf; - break; - } - } - vlc_list_release ( p_list ); + return intf; } /* Find an interaction dialog by its id */ @@ -595,7 +615,7 @@ static void InteractionManage( interaction_t *p_interaction ) /* Nothing to do */ if( p_interaction->i_dialogs == 0 ) return; - InteractionSearchInterface( p_interaction ); + p_interaction->p_intf = SearchInterface( p_interaction ); if( !p_interaction->p_intf ) { /* We mark all dialogs as answered with their "default" answer */ @@ -611,8 +631,6 @@ static void InteractionManage( interaction_t *p_interaction ) p_dialog->i_status = HIDING_DIALOG; } } - else - vlc_object_hold( p_interaction->p_intf ); for( i_index = 0 ; i_index < p_interaction->i_dialogs; i_index ++ ) { diff --git a/src/interface/interface.c b/src/interface/interface.c index edef41e83b..a3fb41a6fa 100644 --- a/src/interface/interface.c +++ b/src/interface/interface.c @@ -89,7 +89,6 @@ intf_thread_t* __intf_Create( vlc_object_t *p_this, const char *psz_module ) p_intf = vlc_object_create( p_this, VLC_OBJECT_INTF ); if( !p_intf ) return NULL; - p_intf->b_interaction = false; #if defined( __APPLE__ ) || defined( WIN32 ) p_intf->b_should_run_on_first_thread = false; #endif diff --git a/src/libvlc.h b/src/libvlc.h index 32218d8c08..35801b82f3 100644 --- a/src/libvlc.h +++ b/src/libvlc.h @@ -226,6 +226,7 @@ typedef struct libvlc_priv_t playlist_t *p_playlist; //< the playlist singleton vlm_t *p_vlm; ///< the VLM singleton (or NULL) interaction_t *p_interaction; ///< interface interaction object + intf_thread_t *p_interaction_intf; ///< XXX interface for interaction httpd_t *p_httpd; ///< HTTP daemon (src/network/httpd.c) #ifdef ENABLE_SOUT sap_handler_t *p_sap; ///< SAP SDP advertiser diff --git a/src/libvlccore.sym b/src/libvlccore.sym index 3658d2db92..e5bb2cf359 100644 --- a/src/libvlccore.sym +++ b/src/libvlccore.sym @@ -188,6 +188,8 @@ __input_Read input_SplitMRL input_StopThread input_vaControl +interaction_Register +interaction_Unregister __intf_Create __intf_Eject __intf_Progress -- 2.39.2