From b2c266cd066e2ca40e117c86a47710387e6beaf7 Mon Sep 17 00:00:00 2001 From: =?utf8?q?R=C3=A9mi=20Denis-Courmont?= Date: Sat, 23 Jan 2010 21:31:19 +0200 Subject: [PATCH] Use a global R/W lock for configuration Previously, we had one configuration mutex per module. With a global read/write lock, resetting, loading, saving and auto-saving the configuration becomes atomic (and use only one lock & unlock pair). Also, multiple threads can now read the configuration item of the same module at the same time. Note that, as the earlier configuration mutex, only configuration item values are protected. The list of items and their meta-data cannot change while VLC runs (they're hard-coded in the plugin descriptors). --- src/config/configuration.h | 2 ++ src/config/core.c | 14 ++++++++------ src/config/file.c | 33 ++++++++++++++++++++++----------- src/modules/modules.c | 2 ++ 4 files changed, 34 insertions(+), 17 deletions(-) diff --git a/src/config/configuration.h b/src/config/configuration.h index bd2b123c1e..a419552e84 100644 --- a/src/config/configuration.h +++ b/src/config/configuration.h @@ -54,6 +54,8 @@ static inline int IsConfigFloatType (int type) uint_fast32_t ConfigStringToKey( const char * ); char *ConfigKeyToString( uint_fast32_t ); +extern vlc_rwlock_t config_lock; + /* The configuration file */ #define CONFIG_FILE "vlcrc" diff --git a/src/config/core.c b/src/config/core.c index 97fd1e106b..d6a3f2ad78 100644 --- a/src/config/core.c +++ b/src/config/core.c @@ -35,6 +35,8 @@ #include "configuration.h" #include "modules/modules.h" +vlc_rwlock_t config_lock; + static inline char *strdupnull (const char *src) { return src ? strdup (src) : NULL; @@ -220,9 +222,9 @@ char * __config_GetPsz( vlc_object_t *p_this, const char *psz_name ) } /* return a copy of the string */ - vlc_mutex_lock( p_config->p_lock ); + vlc_rwlock_rdlock (&config_lock); char *psz_value = strdupnull (p_config->value.psz); - vlc_mutex_unlock( p_config->p_lock ); + vlc_rwlock_unlock (&config_lock); return psz_value; } @@ -256,7 +258,7 @@ void __config_PutPsz( vlc_object_t *p_this, return; } - vlc_mutex_lock( p_config->p_lock ); + vlc_rwlock_wrlock (&config_lock); /* backup old value */ oldval.psz_string = (char *)p_config->value.psz; @@ -270,7 +272,7 @@ void __config_PutPsz( vlc_object_t *p_this, val.psz_string = (char *)p_config->value.psz; - vlc_mutex_unlock( p_config->p_lock ); + vlc_rwlock_unlock (&config_lock); if( p_config->pf_callback ) { @@ -506,6 +508,7 @@ void __config_ResetAll( vlc_object_t *p_this ) module_t *p_module; module_t **list = module_list_get (NULL); + vlc_rwlock_wrlock (&config_lock); for (size_t j = 0; (p_module = list[j]) != NULL; j++) { if( p_module->b_submodule ) continue; @@ -514,7 +517,6 @@ void __config_ResetAll( vlc_object_t *p_this ) { module_config_t *p_config = p_module->p_config + i; - vlc_mutex_lock (p_config->p_lock); if (IsConfigIntegerType (p_config->i_type)) p_config->value.i = p_config->orig.i; else @@ -527,9 +529,9 @@ void __config_ResetAll( vlc_object_t *p_this ) p_config->value.psz = strdupnull (p_config->orig.psz); } - vlc_mutex_unlock (p_config->p_lock); } } + vlc_rwlock_unlock (&config_lock); module_list_free (list); } diff --git a/src/config/file.c b/src/config/file.c index db1128dc26..e1fa786b76 100644 --- a/src/config/file.c +++ b/src/config/file.c @@ -189,6 +189,7 @@ int __config_LoadConfigFile( vlc_object_t *p_this, const char *psz_module_name ) locale_t loc = newlocale (LC_NUMERIC_MASK, "C", NULL); locale_t baseloc = uselocale (loc); + vlc_rwlock_wrlock (&config_lock); while (fgets (line, 1024, file) != NULL) { /* Ignore comments and empty lines */ @@ -263,7 +264,6 @@ int __config_LoadConfigFile( vlc_object_t *p_this, const char *psz_module_name ) /* We found it */ errno = 0; - vlc_mutex_lock( p_item->p_lock ); switch( p_item->i_type ) { case CONFIG_ITEM_BOOL: @@ -301,10 +301,10 @@ int __config_LoadConfigFile( vlc_object_t *p_this, const char *psz_module_name ) p_item->saved.psz = strdupnull (p_item->value.psz); break; } - vlc_mutex_unlock( p_item->p_lock ); break; } } + vlc_rwlock_unlock (&config_lock); if (ferror (file)) { @@ -532,6 +532,9 @@ static int SaveConfigFile( vlc_object_t *p_this, const char *psz_module_name, goto error; } + /* Configuration lock must be taken before vlcrc serializer below. */ + vlc_rwlock_rdlock (&config_lock); + /* The temporary configuration file is per-PID. Therefore SaveConfigFile() * should be serialized against itself within a given process. */ static vlc_mutex_t lock = VLC_STATIC_MUTEX; @@ -540,6 +543,7 @@ static int SaveConfigFile( vlc_object_t *p_this, const char *psz_module_name, int fd = utf8_open (temporary, O_CREAT|O_WRONLY|O_TRUNC, S_IRUSR|S_IWUSR); if (fd == -1) { + vlc_rwlock_unlock (&config_lock); vlc_mutex_unlock (&lock); module_list_free (list); goto error; @@ -547,6 +551,7 @@ static int SaveConfigFile( vlc_object_t *p_this, const char *psz_module_name, file = fdopen (fd, "wt"); if (file == NULL) { + vlc_rwlock_unlock (&config_lock); close (fd); vlc_mutex_unlock (&lock); module_list_free (list); @@ -560,6 +565,10 @@ static int SaveConfigFile( vlc_object_t *p_this, const char *psz_module_name, locale_t loc = newlocale (LC_NUMERIC_MASK, "C", NULL); locale_t baseloc = uselocale (loc); + /* We would take the config lock here. But this would cause a lock + * inversion with the serializer above and config_AutoSaveConfigFile(). + vlc_rwlock_rdlock (&config_lock);*/ + /* Look for the selected module, if NULL then save everything */ for( i_index = 0; (p_parser = list[i_index]) != NULL; i_index++ ) { @@ -591,8 +600,6 @@ static int SaveConfigFile( vlc_object_t *p_this, const char *psz_module_name, || p_item->b_unsaveable) /* ignore volatile option */ continue; - vlc_mutex_lock (p_item->p_lock); - /* Do not save the new value in the configuration file * if doing an autosave, and the item is not an "autosaved" one. */ bool b_retain = b_autosave && !p_item->b_autosave; @@ -663,9 +670,9 @@ static int SaveConfigFile( vlc_object_t *p_this, const char *psz_module_name, if (!b_retain) p_item->b_dirty = false; - vlc_mutex_unlock (p_item->p_lock); } } + vlc_rwlock_unlock (&config_lock); module_list_free (list); if (loc != (locale_t)0) @@ -721,14 +728,15 @@ error: int config_AutoSaveConfigFile( vlc_object_t *p_this ) { - size_t i_index; + int ret = VLC_SUCCESS; bool save = false; assert( p_this ); /* Check if there's anything to save */ module_t **list = module_list_get (NULL); - for( i_index = 0; list[i_index] && !save; i_index++ ) + vlc_rwlock_rdlock (&config_lock); + for (size_t i_index = 0; list[i_index] && !save; i_index++) { module_t *p_parser = list[i_index]; module_config_t *p_item, *p_end; @@ -739,14 +747,17 @@ int config_AutoSaveConfigFile( vlc_object_t *p_this ) p_item < p_end && !save; p_item++ ) { - vlc_mutex_lock (p_item->p_lock); save = p_item->b_autosave && p_item->b_dirty; - vlc_mutex_unlock (p_item->p_lock); } } - module_list_free (list); - return save ? VLC_SUCCESS : SaveConfigFile( p_this, NULL, true ); + if (save) + /* Note: this will get the read lock recursively. Ok. */ + ret = SaveConfigFile (p_this, NULL, true); + vlc_rwlock_unlock (&config_lock); + + module_list_free (list); + return ret; } int __config_SaveConfigFile( vlc_object_t *p_this, const char *psz_module_name ) diff --git a/src/modules/modules.c b/src/modules/modules.c index b6592024e6..8f20a8ebbb 100644 --- a/src/modules/modules.c +++ b/src/modules/modules.c @@ -137,6 +137,7 @@ void __module_InitBank( vlc_object_t *p_this ) * options of main will be available in the module bank structure just * as for every other module. */ AllocateBuiltinModule( p_this, vlc_entry__main ); + vlc_rwlock_init (&config_lock); } else p_module_bank->i_usage++; @@ -180,6 +181,7 @@ void module_EndBank( vlc_object_t *p_this, bool b_plugins ) vlc_mutex_unlock( &module_lock ); return; } + vlc_rwlock_destroy (&config_lock); p_module_bank = NULL; vlc_mutex_unlock( &module_lock ); -- 2.39.2