[vlc-devel] commit: Keep the bank lock until plugins are loaded. ( Rémi Denis-Courmont )
git version control
git at videolan.org
Tue Feb 17 22:36:21 CET 2009
vlc | branch: master | Rémi Denis-Courmont <rdenis at simphalempin.com> | Tue Feb 17 23:28:40 2009 +0200| [247685e30aacb74925c486928844aff52d46874e] | committer: Rémi Denis-Courmont
Keep the bank lock until plugins are loaded.
This is a bit ugly but it fixes two race conditions:
- loading plugins while another thread is initializing,
- using the bank when the first thread has not completed loading plugins.
Unfortunately, there is still a small race when module_need() calls
AllocatePlugin(). It really should not -need to- do that, but the fix would
be quite invasive. We would basically need to store plugin callbacks by names
rather than function pointers. Then the module descriptors would be fully
serializable, so we would not need to re-describe plugins when loading their
shared object. That would also fix the last known corruption bug in the plugins
cache.
> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=247685e30aacb74925c486928844aff52d46874e
---
src/libvlc.c | 14 +++++-----
src/modules/modules.c | 61 ++++++++++++++++++++++++++++---------------------
src/modules/modules.h | 4 +-
3 files changed, 44 insertions(+), 35 deletions(-)
diff --git a/src/libvlc.c b/src/libvlc.c
index 3591a90..fb7aa52 100644
--- a/src/libvlc.c
+++ b/src/libvlc.c
@@ -329,7 +329,7 @@ int libvlc_InternalInit( libvlc_int_t *p_libvlc, int i_argc,
if( config_LoadCmdLine( p_libvlc, &i_argc, ppsz_argv, true ) )
{
- module_EndBank( p_libvlc );
+ module_EndBank( p_libvlc, false );
return VLC_EGENERIC;
}
@@ -429,7 +429,7 @@ int libvlc_InternalInit( libvlc_int_t *p_libvlc, int i_argc,
if( b_exit )
{
free( priv->psz_configfile );
- module_EndBank( p_libvlc );
+ module_EndBank( p_libvlc, false );
return i_ret;
}
@@ -455,7 +455,7 @@ int libvlc_InternalInit( libvlc_int_t *p_libvlc, int i_argc,
/* Translate "C" to the language code: "fr", "en_GB", "nl", "ru"... */
msg_Dbg( p_libvlc, "translation test: code is \"%s\"", _("C") );
- module_EndBank( p_libvlc );
+ module_EndBank( p_libvlc, false );
module_InitBank( p_libvlc );
if( !config_GetInt( p_libvlc, "ignore-config" ) )
config_LoadConfigFile( p_libvlc, "main" );
@@ -541,7 +541,7 @@ int libvlc_InternalInit( libvlc_int_t *p_libvlc, int i_argc,
if( b_exit )
{
free( priv->psz_configfile );
- module_EndBank( p_libvlc );
+ module_EndBank( p_libvlc, true );
return i_ret;
}
@@ -569,7 +569,7 @@ int libvlc_InternalInit( libvlc_int_t *p_libvlc, int i_argc,
PauseConsole();
#endif
free( priv->psz_configfile );
- module_EndBank( p_libvlc );
+ module_EndBank( p_libvlc, true );
return VLC_EGENERIC;
}
priv->i_verbose = config_GetInt( p_libvlc, "verbose" );
@@ -826,7 +826,7 @@ int libvlc_InternalInit( libvlc_int_t *p_libvlc, int i_argc,
{
module_unneed( p_libvlc, priv->p_memcpy_module );
}
- module_EndBank( p_libvlc );
+ module_EndBank( p_libvlc, true );
free( priv->psz_configfile );
return VLC_EGENERIC;
}
@@ -1109,7 +1109,7 @@ void libvlc_InternalCleanup( libvlc_int_t *p_libvlc )
}
/* Free module bank. It is refcounted, so we call this each time */
- module_EndBank( p_libvlc );
+ module_EndBank( p_libvlc, true );
FREENULL( priv->psz_configfile );
var_DelCallback( p_libvlc, "key-pressed", vlc_key_to_action,
diff --git a/src/modules/modules.c b/src/modules/modules.c
index dd026d0..805ea21 100644
--- a/src/modules/modules.c
+++ b/src/modules/modules.c
@@ -148,28 +148,39 @@ void __module_InitBank( vlc_object_t *p_this )
else
p_module_bank->i_usage++;
- vlc_mutex_unlock( &module_lock );
+ /* We do retain the module bank lock until the plugins are loaded as well.
+ * This is ugly, this staged loading approach is needed: LibVLC gets
+ * some configuration parameters relevant to loading the plugins from
+ * the main (builtin) module. The module bank becomes shared read-only data
+ * once it is ready, so we need to fully serialize initialization.
+ * DO NOT UNCOMMENT the following line unless you managed to squeeze
+ * module_LoadPlugins() before you unlock the mutex. */
+ /*vlc_mutex_unlock( &module_lock );*/
}
-
+#undef module_EndBank
/**
- * End bank
- *
* Unloads all unused plugin modules and empties the module
* bank in case of success.
* \param p_this vlc object structure
* \return nothing
*/
-void __module_EndBank( vlc_object_t *p_this )
+void module_EndBank( vlc_object_t *p_this, bool b_plugins )
{
- module_bank_t *p_bank;
+ module_bank_t *p_bank = p_module_bank;
+
+ assert (p_bank != NULL);
/* Save the configuration */
config_AutoSaveConfigFile( p_this );
- vlc_mutex_lock( &module_lock );
- p_bank = p_module_bank;
- assert (p_bank != NULL);
+ /* If plugins were _not_ loaded, then the caller still has the bank lock
+ * from module_InitBank(). */
+ if( b_plugins )
+ vlc_mutex_lock( &module_lock );
+ /*else
+ vlc_assert_locked( &module_lock ); not for static mutexes :( */
+
if( --p_bank->i_usage > 0 )
{
vlc_mutex_unlock( &module_lock );
@@ -218,34 +229,32 @@ void __module_EndBank( vlc_object_t *p_this )
#undef module_LoadPlugins
/**
- * Load all plugins
- *
- * Load all plugin modules we can find.
+ * Loads module descriptions for all available plugins.
* Fills the module bank structure with the plugin modules.
+ *
* \param p_this vlc object structure
* \return nothing
*/
void module_LoadPlugins( vlc_object_t * p_this, bool b_cache_delete )
{
+ module_bank_t *p_bank = p_module_bank;
+
+ assert( p_bank );
+ /*vlc_assert_locked( &module_lock ); not for static mutexes :( */
+
#ifdef HAVE_DYNAMIC_PLUGINS
- vlc_mutex_lock( &module_lock );
- if( p_module_bank->b_plugins )
+ if( p_bank->i_usage == 1 )
{
- vlc_mutex_unlock( &module_lock );
- return;
+ msg_Dbg( p_this, "checking plugin modules" );
+ p_module_bank->b_cache = config_GetInt( p_this, "plugins-cache" ) > 0;
+
+ if( p_module_bank->b_cache || b_cache_delete )
+ CacheLoad( p_this, p_module_bank, b_cache_delete );
+ AllocateAllPlugins( p_this, p_module_bank );
}
+#endif
p_module_bank->b_plugins = true;
vlc_mutex_unlock( &module_lock );
-
- msg_Dbg( p_this, "checking plugin modules" );
- p_module_bank->b_cache = config_GetInt( p_this, "plugins-cache" ) > 0;
-
- if( p_module_bank->b_cache || b_cache_delete )
- CacheLoad( p_this, p_module_bank, b_cache_delete );
-
- /* FIXME: race - do this under the lock */
- AllocateAllPlugins( p_this, p_module_bank );
-#endif
}
/**
diff --git a/src/modules/modules.h b/src/modules/modules.h
index 3e2aef7..00836dd 100644
--- a/src/modules/modules.h
+++ b/src/modules/modules.h
@@ -152,8 +152,8 @@ module_t *vlc_submodule_create (module_t *module);
void __module_InitBank ( vlc_object_t * );
void module_LoadPlugins( vlc_object_t *, bool );
#define module_LoadPlugins(a,b) module_LoadPlugins(VLC_OBJECT(a),b)
-#define module_EndBank(a) __module_EndBank(VLC_OBJECT(a))
-void __module_EndBank ( vlc_object_t * );
+void module_EndBank( vlc_object_t *, bool );
+#define module_EndBank(a,b) module_EndBank(VLC_OBJECT(a), b)
/* Low-level OS-dependent handler */
int module_Load (vlc_object_t *, const char *, module_handle_t *);
More information about the vlc-devel
mailing list