From 358c28c444e3063246c6d966b0834b1078018fdf Mon Sep 17 00:00:00 2001 From: =?utf8?q?R=C3=A9mi=20Denis-Courmont?= Date: Wed, 28 May 2008 20:56:15 +0300 Subject: [PATCH] intf_Destroy(): use vlc_object_release() and a destructor instead While reading this, you will find a bunch of: while (find (VLC_OBJECT_INTERFACE)) release; release; These are of course plain BUGS (which are neither introduced nor fixed by this commit). Imagine, for instance, what happens if two threads run the code above at the same time... they end up releasing the interface once too many. --- include/vlc_interface.h | 1 - modules/access/vcdx/demux.c | 2 +- modules/codec/cmml/cmml.c | 2 +- modules/control/ntservice.c | 4 ++-- modules/control/rc.c | 2 +- src/interface/interface.c | 46 ++++++++++++++++++------------------- src/libvlc-common.c | 12 ++++------ src/libvlccore.sym | 1 - 8 files changed, 32 insertions(+), 38 deletions(-) diff --git a/include/vlc_interface.h b/include/vlc_interface.h index 8040ab8405..04c7c19db6 100644 --- a/include/vlc_interface.h +++ b/include/vlc_interface.h @@ -120,7 +120,6 @@ struct intf_dialog_args_t VLC_EXPORT( intf_thread_t *, __intf_Create, ( vlc_object_t *, const char *, int, const char *const * ) ); VLC_EXPORT( int, intf_RunThread, ( intf_thread_t * ) ); VLC_EXPORT( void, intf_StopThread, ( intf_thread_t * ) ); -VLC_EXPORT( void, intf_Destroy, ( intf_thread_t * ) ); /* If the interface is in the main thread, it should listen both to * p_intf->b_die and p_libvlc->b_die */ diff --git a/modules/access/vcdx/demux.c b/modules/access/vcdx/demux.c index c7c62e2d0f..8d1b674f85 100644 --- a/modules/access/vcdx/demux.c +++ b/modules/access/vcdx/demux.c @@ -129,7 +129,7 @@ void VCDEnd ( vlc_object_t *p_this ) intf_StopThread( p_intf ); vlc_object_detach( p_intf ); vlc_object_release( p_intf ); - intf_Destroy( p_intf ); + vlc_object_release( p_intf ); } p_vcd->p_intf = NULL; diff --git a/modules/codec/cmml/cmml.c b/modules/codec/cmml/cmml.c index 373ca49181..ef66d4a821 100644 --- a/modules/codec/cmml/cmml.c +++ b/modules/codec/cmml/cmml.c @@ -189,7 +189,7 @@ static void CloseDecoder( vlc_object_t *p_this ) intf_StopThread( p_intf ); vlc_object_detach( p_intf ); vlc_object_release( p_intf ); - intf_Destroy( p_intf ); + vlc_object_release( p_intf ); } p_sys->p_intf = NULL; diff --git a/modules/control/ntservice.c b/modules/control/ntservice.c index 99c0d86024..7eed573b3a 100644 --- a/modules/control/ntservice.c +++ b/modules/control/ntservice.c @@ -164,7 +164,7 @@ static void Run( intf_thread_t *p_intf ) intf_StopThread( p_extraintf ); vlc_object_detach( p_extraintf ); vlc_object_release( p_extraintf ); - intf_Destroy( p_extraintf ); + vlc_object_release( p_extraintf ); } /* Make sure we exit (In case other interfaces have been spawned) */ @@ -332,7 +332,7 @@ static void WINAPI ServiceDispatch( DWORD numArgs, char **args ) if( intf_RunThread( p_new_intf ) ) { vlc_object_detach( p_new_intf ); - intf_Destroy( p_new_intf ); + vlc_object_release( p_new_intf ); msg_Err( p_intf, "interface \"%s\" cannot run", psz_temp ); } diff --git a/modules/control/rc.c b/modules/control/rc.c index 3bfd0a62ba..05aa08a37f 100644 --- a/modules/control/rc.c +++ b/modules/control/rc.c @@ -1543,7 +1543,7 @@ static int Intf( vlc_object_t *p_this, char const *psz_cmd, if( intf_RunThread( p_newintf ) ) { vlc_object_detach( p_newintf ); - intf_Destroy( p_newintf ); + vlc_object_release( p_newintf ); } } diff --git a/src/interface/interface.c b/src/interface/interface.c index 3cdd819b21..7f95e01598 100644 --- a/src/interface/interface.c +++ b/src/interface/interface.c @@ -54,6 +54,24 @@ static void RunInterface( intf_thread_t *p_intf ); static int AddIntfCallback( vlc_object_t *, char const *, vlc_value_t , vlc_value_t , void * ); +/** + * \brief Destroy the interface after the main loop endeed. + * + * \param p_intf the interface thread + * \return nothing + */ +static void intf_Destroy( vlc_object_t *obj ) +{ + intf_thread_t *p_intf = (intf_thread_t *)obj; + + /* Unlock module if present (a switch may have failed) */ + if( p_intf->p_module ) + module_Unneed( p_intf, p_intf->p_module ); + + free( p_intf->psz_intf ); + vlc_mutex_destroy( &p_intf->change_lock ); +} + /***************************************************************************** * intf_Create: prepare interface before main loop ***************************************************************************** @@ -114,6 +132,7 @@ intf_thread_t* __intf_Create( vlc_object_t *p_this, const char *psz_module, /* Attach interface to its parent object */ vlc_object_attach( p_intf, p_this ); + vlc_object_set_destructor( p_intf, intf_Destroy ); return p_intf; } @@ -140,6 +159,8 @@ int intf_RunThread( intf_thread_t *p_intf ) if( p_intf->b_should_run_on_first_thread ) { RunInterface( p_intf ); + vlc_object_detach( p_intf ); + vlc_object_release( p_intf ); return VLC_SUCCESS; } @@ -172,29 +193,6 @@ void intf_StopThread( intf_thread_t *p_intf ) } } -/** - * \brief Destroy the interface after the main loop endeed. - * - * Destroys interfaces and closes output devices - * \param p_intf the interface thread - * \return nothing - */ -void intf_Destroy( intf_thread_t *p_intf ) -{ - /* Unlock module if present (a switch may have failed) */ - if( p_intf->p_module ) - { - module_Unneed( p_intf, p_intf->p_module ); - } - free( p_intf->psz_intf ); - - vlc_mutex_destroy( &p_intf->change_lock ); - - /* Free structure */ - vlc_object_release( p_intf ); -} - - /* Following functions are local */ /***************************************************************************** @@ -283,7 +281,7 @@ static int AddIntfCallback( vlc_object_t *p_this, char const *psz_cmd, if( intf_RunThread( p_intf ) != VLC_SUCCESS ) { vlc_object_detach( p_intf ); - intf_Destroy( p_intf ); + vlc_object_release( p_intf ); return VLC_EGENERIC; } diff --git a/src/libvlc-common.c b/src/libvlc-common.c index fdb1d8b531..7be0f85c41 100644 --- a/src/libvlc-common.c +++ b/src/libvlc-common.c @@ -936,9 +936,8 @@ int libvlc_InternalCleanup( libvlc_int_t *p_libvlc ) { intf_StopThread( p_intf ); vlc_object_detach( p_intf ); - vlc_object_release( p_intf ); - intf_Destroy( p_intf ); - p_intf = NULL; + vlc_object_release( p_intf ); /* for intf_Create() */ + vlc_object_release( p_intf ); /* for vlc_object_find() */ } /* Free video outputs */ @@ -1144,11 +1143,10 @@ int libvlc_InternalAddIntf( libvlc_int_t *p_libvlc, /* Try to run the interface */ p_intf->b_play = b_play; i_err = intf_RunThread( p_intf ); - if( i_err || p_intf->b_should_run_on_first_thread ) + if( i_err ) { vlc_object_detach( p_intf ); - intf_Destroy( p_intf ); - p_intf = NULL; + vlc_object_release( p_intf ); return i_err; } @@ -1161,7 +1159,7 @@ int libvlc_InternalAddIntf( libvlc_int_t *p_libvlc, while( vlc_object_lock_and_wait( p_intf ) == 0 ); vlc_object_detach( p_intf ); - intf_Destroy( p_intf ); + vlc_object_release( p_intf ); } return VLC_SUCCESS; diff --git a/src/libvlccore.sym b/src/libvlccore.sym index d4283706f2..bd77501ad4 100644 --- a/src/libvlccore.sym +++ b/src/libvlccore.sym @@ -147,7 +147,6 @@ __input_Read input_StopThread input_vaControl __intf_Create -intf_Destroy __intf_Eject __intf_Progress __intf_ProgressUpdate -- 2.39.2