[vlc-commits] plugin: robustify loading cached plugin
Rémi Denis-Courmont
git at videolan.org
Thu Oct 27 18:46:15 CEST 2016
vlc | branch: master | Rémi Denis-Courmont <remi at remlab.net> | Thu Oct 27 19:20:57 2016 +0300| [ddf09a290ffdc664a3b402d9c0e57b5f79bb51a4] | committer: Rémi Denis-Courmont
plugin: robustify loading cached plugin
So far a cached plugin was loaded in memory, then the plugin
description was uselessly rebuilt from the loaded plugin, and then the
fuzzy "cache merge" was performed.
With this change, the cached plugin is loaded in memory, then the
plugin callbacks are matched by name. The description is not rebuilt
and the cache is not "merged".
> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=ddf09a290ffdc664a3b402d9c0e57b5f79bb51a4
---
src/modules/bank.c | 41 +++++++-----
src/modules/cache.c | 27 --------
src/modules/entry.c | 172 ++++++++++++++++++++++++++++++++++++++++++++++++--
src/modules/modules.h | 5 +-
4 files changed, 196 insertions(+), 49 deletions(-)
diff --git a/src/modules/bank.c b/src/modules/bank.c
index 36f3fcf..a4998f6 100644
--- a/src/modules/bank.c
+++ b/src/modules/bank.c
@@ -103,6 +103,8 @@ static void module_InitStaticModules(void) { }
#endif
#ifdef HAVE_DYNAMIC_PLUGINS
+static const char vlc_entry_name[] = "vlc_entry" MODULE_SUFFIX;
+
/**
* Loads a dynamically-linked plug-in into memory and initialize it.
*
@@ -121,9 +123,8 @@ static vlc_plugin_t *module_InitDynamic(vlc_object_t *obj, const char *path,
return NULL;
/* Try to resolve the symbol */
- static const char entry_name[] = "vlc_entry" MODULE_SUFFIX;
vlc_plugin_cb entry =
- (vlc_plugin_cb) module_Lookup (handle, entry_name);
+ (vlc_plugin_cb) module_Lookup(handle, vlc_entry_name);
if (entry == NULL)
{
msg_Warn (obj, "cannot find plug-in entry point in %s", path);
@@ -408,33 +409,41 @@ int module_Map(vlc_object_t *obj, vlc_plugin_t *plugin)
static vlc_mutex_t lock = VLC_STATIC_MUTEX;
if (atomic_load_explicit(&plugin->loaded, memory_order_acquire))
- return 0;
+ return 0; /* fast path: already loaded */
- vlc_plugin_t *uncache;
+ /* Try to load the plug-in (without locks, so read-only) */
+ module_handle_t handle;
assert(plugin->abspath != NULL);
- uncache = module_InitDynamic(obj, plugin->abspath, false);
- if (uncache == NULL)
+
+ if (module_Load(obj, plugin->abspath, &handle, false))
+ return -1;
+
+ vlc_plugin_cb entry =
+ (vlc_plugin_cb) module_Lookup(handle, vlc_entry_name);
+ if (entry == NULL)
{
- msg_Err(obj, "corrupt module: %s", plugin->abspath);
+ msg_Err(obj, "cannot find plug-in entry point in %s", plugin->abspath);
+ module_Unload(handle);
return -1;
}
- assert(uncache->module != NULL);
-
vlc_mutex_lock(&lock);
if (!atomic_load_explicit(&plugin->loaded, memory_order_relaxed))
- {
- CacheMerge(obj, plugin->module, uncache->module);
+ { /* Lock is held, update the plug-in structure */
+ if (vlc_plugin_resolve(plugin, entry))
+ {
+ vlc_mutex_unlock(&lock);
+ return -1;
+ }
+
+ plugin->handle = handle;
atomic_store_explicit(&plugin->loaded, true, memory_order_release);
}
- else
- uncache = NULL;
+ else /* Another thread won the race to load the plugin */
+ module_Unload(handle);
vlc_mutex_unlock(&lock);
- if (likely(uncache != NULL))
- vlc_plugin_destroy(uncache);
-
return 0;
}
diff --git a/src/modules/cache.c b/src/modules/cache.c
index 8a6c77e..cdeb715 100644
--- a/src/modules/cache.c
+++ b/src/modules/cache.c
@@ -714,33 +714,6 @@ out:
free (tmpname);
}
-/*****************************************************************************
- * CacheMerge: Merge a cache module descriptor with a full module descriptor.
- *****************************************************************************/
-void CacheMerge( vlc_object_t *p_this, module_t *p_cache, module_t *p_module )
-{
- (void)p_this;
-
- p_cache->pf_activate = p_module->pf_activate;
- p_cache->pf_deactivate = p_module->pf_deactivate;
-
- /* FIXME: This looks too simplistic an algorithm to me. What if the module
- * file was altered such that the number of order of submodules was
- * altered... after VLC started -- Courmisch, 09/2008 */
- module_t *p_child = p_module->submodule,
- *p_cchild = p_cache->submodule;
- while( p_child && p_cchild )
- {
- p_cchild->pf_activate = p_child->pf_activate;
- p_cchild->pf_deactivate = p_child->pf_deactivate;
- p_child = p_child->next;
- p_cchild = p_cchild->next;
- }
-
- p_cache->plugin->handle = p_module->plugin->handle;
- atomic_init(&p_module->plugin->loaded, false);
-}
-
/**
* Looks up a plugin file in a table of cached plugins.
*/
diff --git a/src/modules/entry.c b/src/modules/entry.c
index 4166b0c..f9d6d8c 100644
--- a/src/modules/entry.c
+++ b/src/modules/entry.c
@@ -30,6 +30,7 @@
#include <stdarg.h>
#include <limits.h>
#include <float.h>
+#include <search.h>
#include "modules/modules.h"
#include "config/configuration.h"
@@ -164,11 +165,13 @@ static module_config_t *vlc_config_create(vlc_plugin_t *plugin, int type)
return tab + confsize;
}
-
/**
- * Callback for the plugin descriptor functions.
+ * Plug-in descriptor callback.
+ *
+ * This callback populates modules, configuration items and properties of a
+ * plug-in from the plug-in descriptor.
*/
-static int vlc_plugin_setter(void *ctx, void *tgt, int propid, ...)
+static int vlc_plugin_desc_cb(void *ctx, void *tgt, int propid, ...)
{
vlc_plugin_t *plugin = ctx;
module_t *module = tgt;
@@ -457,10 +460,171 @@ vlc_plugin_t *vlc_plugin_describe(vlc_plugin_cb entry)
if (unlikely(plugin == NULL))
return NULL;
- if (entry(vlc_plugin_setter, plugin) != 0)
+ if (entry(vlc_plugin_desc_cb, plugin) != 0)
{
vlc_plugin_destroy(plugin); /* partially initialized plug-in... */
plugin = NULL;
}
return plugin;
}
+
+struct vlc_plugin_symbol
+{
+ const char *name;
+ void *addr;
+};
+
+static int vlc_plugin_symbol_compare(const void *a, const void *b)
+{
+ const struct vlc_plugin_symbol *sa = a , *sb = b;
+
+ return strcmp(sa->name, sb->name);
+}
+
+/**
+ * Plug-in symbols callback.
+ *
+ * This callback generates a mapping of plugin symbol names to symbol
+ * addresses.
+ */
+static int vlc_plugin_gpa_cb(void *ctx, void *tgt, int propid, ...)
+{
+ void **rootp = ctx;
+ const char *name;
+ void *addr;
+
+ (void) tgt;
+
+ switch (propid)
+ {
+ case VLC_MODULE_CB_OPEN:
+ case VLC_MODULE_CB_CLOSE:
+ case VLC_CONFIG_LIST_CB:
+ {
+ va_list ap;
+
+ va_start(ap, propid);
+ name = va_arg(ap, const char *);
+ addr = va_arg(ap, void *);
+ va_end (ap);
+ break;
+ }
+ default:
+ return 0;
+ }
+
+ struct vlc_plugin_symbol *sym = malloc(sizeof (*sym));
+
+ sym->name = name;
+ sym->addr = addr;
+
+ struct vlc_plugin_symbol **symp = tsearch(sym, rootp,
+ vlc_plugin_symbol_compare);
+ if (unlikely(symp == NULL))
+ { /* Memory error */
+ free(sym);
+ return -1;
+ }
+
+ if (*symp != sym)
+ { /* Duplicate symbol */
+ assert((*symp)->addr == sym->addr);
+ free(sym);
+ }
+ return 0;
+}
+
+/**
+ * Gets the symbols of a plugin.
+ *
+ * This function generates a list of symbol names and addresses for a given
+ * plugin descriptor. The result can be used with vlc_plugin_get_symbol()
+ * to resolve a symbol name to an address.
+ *
+ * The result must be freed with vlc_plugin_free_symbols(). The result is only
+ * meaningful until the plugin is unloaded.
+ */
+static void *vlc_plugin_get_symbols(vlc_plugin_cb entry)
+{
+ void *root = NULL;
+
+ if (entry(vlc_plugin_gpa_cb, &root))
+ {
+ tdestroy(root, free);
+ return NULL;
+ }
+
+ return root;
+}
+
+static void vlc_plugin_free_symbols(void *root)
+{
+ tdestroy(root, free);
+}
+
+static int vlc_plugin_get_symbol(void *root, const char *name,
+ void **restrict addrp)
+{
+ if (name == NULL)
+ { /* TODO: use this; do not define "NULL" as a name for NULL? */
+ *addrp = NULL;
+ return 0;
+ }
+
+ const struct vlc_plugin_symbol **symp = tfind(&name, &root,
+ vlc_plugin_symbol_compare);
+
+ if (symp == NULL)
+ return -1;
+
+ *addrp = (*symp)->addr;
+ return 0;
+}
+
+int vlc_plugin_resolve(vlc_plugin_t *plugin, vlc_plugin_cb entry)
+{
+ void *syms = vlc_plugin_get_symbols(entry);
+ int ret = -1;
+
+ /* Resolve modules activate/deactivate callbacks */
+ module_t *module = plugin->module;
+ assert(module != NULL);
+
+ if (vlc_plugin_get_symbol(syms, module->activate_name,
+ &module->pf_activate)
+ || vlc_plugin_get_symbol(syms, module->deactivate_name,
+ &module->pf_deactivate))
+ goto error;
+
+ for (module = module->submodule; module != NULL; module = module->next)
+ {
+ if (vlc_plugin_get_symbol(syms, module->activate_name,
+ &module->pf_activate)
+ || vlc_plugin_get_symbol(syms, module->deactivate_name,
+ &module->pf_deactivate))
+ goto error;
+ }
+
+ /* Resolve configuration callbacks */
+ for (size_t i = 0; i < plugin->conf.size; i++)
+ {
+ module_config_t *item = plugin->conf.items + i;
+ void *cb;
+
+ if (item->list_cb_name == NULL)
+ continue;
+ if (vlc_plugin_get_symbol(syms, item->list_cb_name, &cb))
+ goto error;
+
+ if (IsConfigIntegerType (item->i_type))
+ item->list.i_cb = cb;
+ else
+ if (IsConfigStringType (item->i_type))
+ item->list.psz_cb = cb;
+ }
+
+ ret = 0;
+error:
+ vlc_plugin_free_symbols(syms);
+ return ret;
+}
diff --git a/src/modules/modules.h b/src/modules/modules.h
index 9bd717e..2742e4f 100644
--- a/src/modules/modules.h
+++ b/src/modules/modules.h
@@ -104,11 +104,13 @@ struct module_t
};
vlc_plugin_t *vlc_plugin_create(void);
-vlc_plugin_t *vlc_plugin_describe(vlc_plugin_cb);
void vlc_plugin_destroy(vlc_plugin_t *);
module_t *vlc_module_create(vlc_plugin_t *);
void vlc_module_destroy (module_t *);
+vlc_plugin_t *vlc_plugin_describe(vlc_plugin_cb);
+int vlc_plugin_resolve(vlc_plugin_t *, vlc_plugin_cb);
+
void module_InitBank (void);
size_t module_LoadPlugins( vlc_object_t * );
#define module_LoadPlugins(a) module_LoadPlugins(VLC_OBJECT(a))
@@ -125,7 +127,6 @@ void *module_Lookup (module_handle_t, const char *);
void module_Unload (module_handle_t);
/* Plugins cache */
-void CacheMerge (vlc_object_t *, module_t *, module_t *);
vlc_plugin_t *vlc_cache_load(vlc_object_t *, const char *, block_t **);
struct stat;
More information about the vlc-commits
mailing list