[vlc-devel] commit: Use a global R/W lock for configuration ( Rémi Denis-Courmont )
git version control
git at videolan.org
Sat Jan 23 20:51:58 CET 2010
vlc | branch: master | Rémi Denis-Courmont <remi at remlab.net> | Sat Jan 23 21:31:19 2010 +0200| [b2c266cd066e2ca40e117c86a47710387e6beaf7] | committer: Rémi Denis-Courmont
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).
> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=b2c266cd066e2ca40e117c86a47710387e6beaf7
---
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 bd2b123..a419552 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 97fd1e1..d6a3f2a 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 db1128d..e1fa786 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 b659202..8f20a8e 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 );
More information about the vlc-devel
mailing list