diff --git a/include/variables.h b/include/variables.h index 7f5cb5ff17..f2ba2ec817 100644 --- a/include/variables.h +++ b/include/variables.h @@ -2,7 +2,7 @@ * variables.h: variables handling ***************************************************************************** * Copyright (C) 2002 VideoLAN - * $Id: variables.h,v 1.4 2002/10/16 19:39:42 sam Exp $ + * $Id: variables.h,v 1.5 2002/10/17 13:15:30 sam Exp $ * * Authors: Samuel Hocevar * @@ -37,9 +37,13 @@ struct variable_t u32 i_hash; int i_type; - /* Lots of other things that can be added */ + /* Creation count: we only destroy the variable if it reaches 0 */ int i_usage; + /* Set to TRUE if the variable is in a callback */ + vlc_bool_t b_incallback; + + /* Registered callbacks */ int i_entries; callback_entry_t * p_entries; }; diff --git a/include/vlc/vlc.h b/include/vlc/vlc.h index e6f1de424c..769acc894f 100644 --- a/include/vlc/vlc.h +++ b/include/vlc/vlc.h @@ -2,7 +2,7 @@ * vlc.h: global header for vlc ***************************************************************************** * Copyright (C) 1998, 1999, 2000 VideoLAN - * $Id: vlc.h,v 1.16 2002/10/14 16:46:55 sam Exp $ + * $Id: vlc.h,v 1.17 2002/10/17 13:15:30 sam Exp $ * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -50,6 +50,7 @@ typedef union #define VLC_SUCCESS -0 /* No error */ #define VLC_ENOMEM -1 /* Not enough memory */ #define VLC_ETHREAD -2 /* Thread error */ +#define VLC_ETIMEOUT -3 /* Timeout */ #define VLC_ENOMOD -10 /* Module not found */ diff --git a/modules/misc/testsuite/test4.c b/modules/misc/testsuite/test4.c index 53d8d2ae13..7b7fa227dc 100644 --- a/modules/misc/testsuite/test4.c +++ b/modules/misc/testsuite/test4.c @@ -2,7 +2,7 @@ * test4.c : Miscellaneous stress tests module for vlc ***************************************************************************** * Copyright (C) 2002 VideoLAN - * $Id: test4.c,v 1.2 2002/10/14 19:04:51 sam Exp $ + * $Id: test4.c,v 1.3 2002/10/17 13:15:30 sam Exp $ * * Authors: Samuel Hocevar * @@ -41,6 +41,11 @@ /***************************************************************************** * Local prototypes. *****************************************************************************/ +static int Callback ( vlc_object_t *, char *, char * ); +static int MyCallback( vlc_object_t *, char const *, + vlc_value_t, vlc_value_t, void * ); +static void * MyThread ( vlc_object_t * ); + static int Stress ( vlc_object_t *, char *, char * ); static void * Dummy ( vlc_object_t * ); @@ -51,14 +56,139 @@ static int Signal ( vlc_object_t *, char *, char * ); *****************************************************************************/ vlc_module_begin(); set_description( _("Miscellaneous stress tests") ); - var_Create( p_module->p_libvlc, "stress", VLC_VAR_COMMAND ); - var_Set( p_module->p_libvlc, "stress", (vlc_value_t)(void*)Stress ); + var_Create( p_module->p_libvlc, "callback-test", VLC_VAR_COMMAND ); + var_Set( p_module->p_libvlc, "callback-test", (vlc_value_t)(void*)Callback ); + var_Create( p_module->p_libvlc, "stress-test", VLC_VAR_COMMAND ); + var_Set( p_module->p_libvlc, "stress-test", (vlc_value_t)(void*)Stress ); var_Create( p_module->p_libvlc, "signal", VLC_VAR_COMMAND ); var_Set( p_module->p_libvlc, "signal", (vlc_value_t)(void*)Signal ); vlc_module_end(); /***************************************************************************** - * Stress: perform various tests + * Callback: test callback functions + *****************************************************************************/ +static int Callback( vlc_object_t *p_this, char *psz_cmd, char *psz_arg ) +{ + int i; + char psz_var[20]; + vlc_object_t *pp_objects[10]; + vlc_value_t val; + + /* Allocate a few variables */ + for( i = 0; i < 1000; i++ ) + { + sprintf( psz_var, "blork-%i", i ); + var_Create( p_this, psz_var, VLC_VAR_INTEGER ); + var_AddCallback( p_this, psz_var, MyCallback, NULL ); + } + + /* + * Test #1: callback loop detection + */ + printf( "Test #1: callback loop detection\n" ); + + printf( " - without boundary check (vlc should print an error)\n" ); + val.i_int = 1234567; + var_Set( p_this, "blork-12", val ); + + printf( " - with boundary check\n" ); + val.i_int = 12; + var_Set( p_this, "blork-12", val ); + + /* + * Test #2: concurrent access + */ + printf( "Test #2: concurrent access\n" ); + + printf( " - launch worker threads\n" ); + + for( i = 0; i < 10; i++ ) + { + pp_objects[i] = vlc_object_create( p_this, VLC_OBJECT_GENERIC ); + vlc_object_attach( pp_objects[i], p_this ); + vlc_thread_create( pp_objects[i], "foo", MyThread, 0, VLC_TRUE ); + } + + msleep( 3000000 ); + + printf( " - kill worker threads\n" ); + + for( i = 0; i < 10; i++ ) + { + pp_objects[i]->b_die = VLC_TRUE; + vlc_thread_join( pp_objects[i] ); + vlc_object_detach( pp_objects[i] ); + vlc_object_destroy( pp_objects[i] ); + } + + /* Clean our mess */ + for( i = 0; i < 1000; i++ ) + { + sprintf( psz_var, "blork-%i", i ); + var_DelCallback( p_this, psz_var, MyCallback, NULL ); + var_Destroy( p_this, psz_var ); + } + + return VLC_SUCCESS; +} + +/***************************************************************************** + * MyCallback: used by callback-test + *****************************************************************************/ +static int MyCallback( vlc_object_t *p_this, char const *psz_var, + vlc_value_t oldval, vlc_value_t newval, void *p_data ) +{ + int i_var = 1 + atoi( psz_var + strlen("blork-") ); + char psz_newvar[20]; + + if( i_var == 1000 ) + { + i_var = 0; + } + + /* If we are requested to stop, then stop. */ + if( i_var == newval.i_int ) + { + return VLC_SUCCESS; + } + + /* If we're the blork-3 callback, set blork-4, and so on. */ + sprintf( psz_newvar, "blork-%i", i_var ); + var_Set( p_this, psz_newvar, newval ); + + return VLC_SUCCESS; +} + +/***************************************************************************** + * MyThread: used by callback-test, creates objects and then do nothing. + *****************************************************************************/ +static void * MyThread( vlc_object_t *p_this ) +{ + char psz_var[20]; + vlc_value_t val; + vlc_object_t *p_parent = p_this->p_parent; + + vlc_thread_ready( p_this ); + + val.i_int = 42; + + while( !p_this->b_die ) + { + int i = (int) (100.0 * rand() / (RAND_MAX)); + + sprintf( psz_var, "blork-%i", i ); + val.i_int = i + 200; + var_Set( p_parent, psz_var, val ); + + /* This is quite heavy, but we only have 10 threads. Keep cool. */ + msleep( 1000 ); + } + + return NULL; +} + +/***************************************************************************** + * Stress: perform various stress tests *****************************************************************************/ static int Stress( vlc_object_t *p_this, char *psz_cmd, char *psz_arg ) { @@ -212,7 +342,7 @@ static int Stress( vlc_object_t *p_this, char *psz_cmd, char *psz_arg ) } /***************************************************************************** - * Dummy: used by test, creates objects and then do nothing. + * Dummy: used by stress-test, creates objects and then do nothing. *****************************************************************************/ static void * Dummy( vlc_object_t *p_this ) { diff --git a/src/misc/error.c b/src/misc/error.c index 86656b17cd..a71ed7581d 100644 --- a/src/misc/error.c +++ b/src/misc/error.c @@ -2,7 +2,7 @@ * error.c: error handling routine ***************************************************************************** * Copyright (C) 2002 VideoLAN - * $Id: error.c,v 1.1 2002/10/14 16:35:18 sam Exp $ + * $Id: error.c,v 1.2 2002/10/17 13:15:31 sam Exp $ * * Authors: Samuel Hocevar * @@ -43,6 +43,8 @@ char const * vlc_error ( int i_err ) return "not enough memory"; case VLC_ETHREAD: return "thread error"; + case VLC_ETIMEOUT: + return "timeout"; case VLC_ENOMOD: return "module not found"; diff --git a/src/misc/objects.c b/src/misc/objects.c index f0be50c2bf..5b5878dc87 100644 --- a/src/misc/objects.c +++ b/src/misc/objects.c @@ -2,7 +2,7 @@ * objects.c: vlc_object_t handling ***************************************************************************** * Copyright (C) 2002 VideoLAN - * $Id: objects.c,v 1.25 2002/10/14 16:46:56 sam Exp $ + * $Id: objects.c,v 1.26 2002/10/17 13:15:31 sam Exp $ * * Authors: Samuel Hocevar * @@ -532,8 +532,6 @@ vlc_list_t * __vlc_list_find( vlc_object_t *p_this, int i_type, int i_mode ) *****************************************************************************/ static int DumpCommand( vlc_object_t *p_this, char *psz_cmd, char *psz_arg ) { - vlc_mutex_lock( &structure_lock ); - if( *psz_cmd == 't' ) { char psz_foo[2 * MAX_DUMPSTRUCTURE_DEPTH + 1]; @@ -553,13 +551,19 @@ static int DumpCommand( vlc_object_t *p_this, char *psz_cmd, char *psz_arg ) p_object = p_this->p_vlc ? VLC_OBJECT(p_this->p_vlc) : p_this; } + vlc_mutex_lock( &structure_lock ); + psz_foo[0] = '|'; DumpStructure( p_object, 0, psz_foo ); + + vlc_mutex_unlock( &structure_lock ); } else if( *psz_cmd == 'l' ) { vlc_object_t **pp_current, **pp_end; + vlc_mutex_lock( &structure_lock ); + pp_current = p_this->p_libvlc->pp_objects; pp_end = pp_current + p_this->p_libvlc->i_objects; @@ -576,9 +580,9 @@ static int DumpCommand( vlc_object_t *p_this, char *psz_cmd, char *psz_arg ) (*pp_current)->psz_object_type ); } } - } - vlc_mutex_unlock( &structure_lock ); + vlc_mutex_unlock( &structure_lock ); + } return VLC_SUCCESS; } diff --git a/src/misc/variables.c b/src/misc/variables.c index 3c73184ffb..1b499151a6 100644 --- a/src/misc/variables.c +++ b/src/misc/variables.c @@ -2,7 +2,7 @@ * variables.c: routines for object variables handling ***************************************************************************** * Copyright (C) 2002 VideoLAN - * $Id: variables.c,v 1.7 2002/10/16 19:39:42 sam Exp $ + * $Id: variables.c,v 1.8 2002/10/17 13:15:31 sam Exp $ * * Authors: Samuel Hocevar * @@ -42,6 +42,7 @@ struct callback_entry_t /***************************************************************************** * Local prototypes *****************************************************************************/ +static int GetUnused ( vlc_object_t *, const char * ); static u32 HashString ( const char * ); static int Insert ( variable_t *, int, const char * ); static int InsertInner ( variable_t *, int, u32 ); @@ -106,6 +107,8 @@ int __var_Create( vlc_object_t *p_this, const char *psz_name, int i_type ) p_var->i_usage = 1; + p_var->b_incallback = VLC_FALSE; + p_var->i_entries = 0; p_var->p_entries = NULL; @@ -152,20 +155,19 @@ int __var_Create( vlc_object_t *p_this, const char *psz_name, int i_type ) *****************************************************************************/ int __var_Destroy( vlc_object_t *p_this, const char *psz_name ) { - int i_del; + int i_var; variable_t *p_var; vlc_mutex_lock( &p_this->var_lock ); - i_del = Lookup( p_this->p_vars, p_this->i_vars, psz_name ); - - if( i_del < 0 ) + i_var = GetUnused( p_this, psz_name ); + if( i_var < 0 ) { vlc_mutex_unlock( &p_this->var_lock ); - return VLC_ENOVAR; + return i_var; } - p_var = &p_this->p_vars[i_del]; + p_var = &p_this->p_vars[i_var]; if( p_var->i_usage > 1 ) { @@ -197,9 +199,9 @@ int __var_Destroy( vlc_object_t *p_this, const char *psz_name ) free( p_var->psz_name ); - memmove( p_this->p_vars + i_del, - p_this->p_vars + i_del + 1, - (p_this->i_vars - i_del - 1) * sizeof(variable_t) ); + memmove( p_this->p_vars + i_var, + p_this->p_vars + i_var + 1, + (p_this->i_vars - i_var - 1) * sizeof(variable_t) ); if( (p_this->i_vars & 15) == 0 ) { @@ -253,12 +255,11 @@ int __var_Set( vlc_object_t *p_this, const char *psz_name, vlc_value_t val ) vlc_mutex_lock( &p_this->var_lock ); - i_var = Lookup( p_this->p_vars, p_this->i_vars, psz_name ); - + i_var = GetUnused( p_this, psz_name ); if( i_var < 0 ) { vlc_mutex_unlock( &p_this->var_lock ); - return VLC_ENOVAR; + return i_var; } p_var = &p_this->p_vars[i_var]; @@ -276,23 +277,41 @@ int __var_Set( vlc_object_t *p_this, const char *psz_name, vlc_value_t val ) /* Backup needed stuff */ oldval = p_var->val; - /* Set the variable */ - p_var->val = val; - - /* Deal with callbacks */ + /* Deal with callbacks. Tell we're in a callback, release the lock, + * call stored functions, retake the lock. */ if( p_var->i_entries ) { - int i; + int i_var; + int i_entries = p_var->i_entries; + callback_entry_t *p_entries = p_var->p_entries; - for( i = p_var->i_entries ; i-- ; ) + p_var->b_incallback = VLC_TRUE; + vlc_mutex_unlock( &p_this->var_lock ); + + /* The real calls */ + for( ; i_entries-- ; ) + { + p_entries[i_entries].pf_callback( p_this, psz_name, oldval, val, + p_entries[i_entries].p_data ); + } + + vlc_mutex_lock( &p_this->var_lock ); + + i_var = Lookup( p_this->p_vars, p_this->i_vars, psz_name ); + if( i_var < 0 ) { - /* FIXME: oooh, see the deadlocks! see the race conditions! */ - p_var->p_entries[i].pf_callback( p_this, p_var->psz_name, - oldval, val, - p_var->p_entries[i].p_data ); + msg_Err( p_this, "variable %s has disappeared" ); + vlc_mutex_unlock( &p_this->var_lock ); + return VLC_ENOVAR; } + + p_var = &p_this->p_vars[i_var]; + p_var->b_incallback = VLC_FALSE; } + /* Set the variable */ + p_var->val = val; + /* Free data if needed */ switch( p_var->i_type ) { @@ -385,20 +404,19 @@ int __var_Get( vlc_object_t *p_this, const char *psz_name, vlc_value_t *p_val ) int __var_AddCallback( vlc_object_t *p_this, const char *psz_name, vlc_callback_t pf_callback, void *p_data ) { - int i_var, i_entry; + int i_entry, i_var; variable_t *p_var; vlc_mutex_lock( &p_this->var_lock ); - i_var = Lookup( p_this->p_vars, p_this->i_vars, psz_name ); + i_var = GetUnused( p_this, psz_name ); if( i_var < 0 ) { vlc_mutex_unlock( &p_this->var_lock ); - return VLC_ENOVAR; + return i_var; } p_var = &p_this->p_vars[i_var]; - i_entry = p_var->i_entries++; if( i_entry ) @@ -428,16 +446,16 @@ int __var_AddCallback( vlc_object_t *p_this, const char *psz_name, int __var_DelCallback( vlc_object_t *p_this, const char *psz_name, vlc_callback_t pf_callback, void *p_data ) { - int i_var, i_entry; + int i_entry, i_var; variable_t *p_var; vlc_mutex_lock( &p_this->var_lock ); - i_var = Lookup( p_this->p_vars, p_this->i_vars, psz_name ); + i_var = GetUnused( p_this, psz_name ); if( i_var < 0 ) { vlc_mutex_unlock( &p_this->var_lock ); - return VLC_ENOVAR; + return i_var; } p_var = &p_this->p_vars[i_var]; @@ -481,6 +499,41 @@ int __var_DelCallback( vlc_object_t *p_this, const char *psz_name, /* Following functions are local */ +/***************************************************************************** + * GetUnused: find an unused variable from its name + ***************************************************************************** + * We do i_tries tries before giving up, just in case the variable is being + * modified and called from a callback. + *****************************************************************************/ +static int GetUnused( vlc_object_t *p_this, const char *psz_name ) +{ + int i_var, i_tries = 0; + + while( VLC_TRUE ) + { + i_var = Lookup( p_this->p_vars, p_this->i_vars, psz_name ); + if( i_var < 0 ) + { + return VLC_ENOVAR; + } + + if( ! p_this->p_vars[i_var].b_incallback ) + { + return i_var; + } + + if( i_tries++ > 100 ) + { + msg_Err( p_this, "caught in a callback deadlock?" ); + return VLC_ETIMEOUT; + } + + vlc_mutex_unlock( &p_this->var_lock ); + msleep( THREAD_SLEEP ); + vlc_mutex_lock( &p_this->var_lock ); + } +} + /***************************************************************************** * HashString: our cool hash function *****************************************************************************