From: VMware, Inc <> Date: Tue, 28 Jun 2011 18:47:41 +0000 (-0700) Subject: vmtoolsd: separate loading and initializing plugins. X-Git-Tag: 2011.06.27-437995~28 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=0412e890a7d0e9e3d45fa8ce372e89319628baa2;p=thirdparty%2Fopen-vm-tools.git vmtoolsd: separate loading and initializing plugins. Currently the code that loads the plugins also initializes them right away. This is undesired when using appLoader, since in the process of figuring out dependencies, it may fork the process again and cause the same plugin to be initialized multiple times. This doesn't cause any issues other than a few (one time) memory leaks and wasted work, but it's better to avoid this. Signed-off-by: Marcelo Vanzin --- diff --git a/open-vm-tools/services/vmtoolsd/pluginMgr.c b/open-vm-tools/services/vmtoolsd/pluginMgr.c index 1d24abc26..038dd18ac 100644 --- a/open-vm-tools/services/vmtoolsd/pluginMgr.c +++ b/open-vm-tools/services/vmtoolsd/pluginMgr.c @@ -34,6 +34,15 @@ #include "vmware/tools/utils.h" +/** Defines the internal data about a plugin. */ +typedef struct ToolsPlugin { + gchar *fileName; + GModule *module; + ToolsPluginOnLoad onload; + ToolsPluginData *data; +} ToolsPlugin; + + #ifdef USE_APPLOADER static Bool (*LoadDependencies)(char *libName, Bool useShipped); #endif @@ -163,6 +172,26 @@ ToolsCoreDumpSignal(ToolsAppCtx *ctx, } +/** + * Frees memory associated with a ToolsPlugin instance. If the plugin hasn't + * been initialized yet, this will unload the shared object. + * + * @param[in] plugin ToolsPlugin instance. + */ + +static void +ToolsCoreFreePlugin(ToolsPlugin *plugin) +{ + if (plugin->module != NULL && !g_module_close(plugin->module)) { + g_warning("Error unloading plugin '%s': %s\n", + plugin->fileName, + g_module_error()); + } + g_free(plugin->fileName); + g_free(plugin); +} + + /** * Callback to register applications with the given provider. * @@ -505,7 +534,6 @@ ToolsCoreLoadDirectory(ToolsAppCtx *ctx, gchar *path; GModule *module = NULL; ToolsPlugin *plugin = NULL; - ToolsPluginData *data = NULL; ToolsPluginOnLoad onload; entry = g_ptr_array_index(plugins, i); @@ -534,29 +562,12 @@ ToolsCoreLoadDirectory(ToolsAppCtx *ctx, goto next; } - if (onload != NULL) { - data = onload(ctx); - } - - if (data == NULL) { - g_info("Plugin '%s' didn't provide deployment data, unloading.\n", entry); - goto next; - } - - /* Break early if a plugin has requested the container to quit. */ - if (ctx->errorCode != 0) { - break; - } - - ASSERT(data->name != NULL); - g_module_make_resident(module); plugin = g_malloc(sizeof *plugin); + plugin->fileName = entry; + plugin->data = NULL; plugin->module = module; - plugin->data = data; - VMTools_BindTextDomain(data->name, NULL, NULL); - + plugin->onload = onload; g_ptr_array_add(regs, plugin); - g_debug("Plugin '%s' initialized.\n", plugin->data->name); next: g_free(path); @@ -567,9 +578,6 @@ ToolsCoreLoadDirectory(ToolsAppCtx *ctx, } } - for (i = 0; i < plugins->len; i++) { - g_free(g_ptr_array_index(plugins, i)); - } g_ptr_array_free(plugins, TRUE); ret = TRUE; @@ -611,6 +619,8 @@ ToolsCore_LoadPlugins(ToolsServiceState *state) gboolean pluginDirExists; gboolean ret = FALSE; gchar *pluginRoot; + guint i; + GPtrArray *plugins = NULL; #if defined(sun) && defined(__x86_64__) const char *subdir = "/amd64"; @@ -645,7 +655,7 @@ ToolsCore_LoadPlugins(ToolsServiceState *state) } #endif - state->plugins = g_ptr_array_new(); + plugins = g_ptr_array_new(); /* * First, load plugins from the common directory. The common directory @@ -663,7 +673,7 @@ ToolsCore_LoadPlugins(ToolsServiceState *state) } if (g_file_test(state->commonPath, G_FILE_TEST_IS_DIR) && - !ToolsCoreLoadDirectory(&state->ctx, state->commonPath, state->plugins)) { + !ToolsCoreLoadDirectory(&state->ctx, state->commonPath, plugins)) { goto exit; } @@ -687,10 +697,40 @@ ToolsCore_LoadPlugins(ToolsServiceState *state) } if (pluginDirExists && - !ToolsCoreLoadDirectory(&state->ctx, state->pluginPath, state->plugins)) { + !ToolsCoreLoadDirectory(&state->ctx, state->pluginPath, plugins)) { goto exit; } + + /* + * All plugins are loaded, now initialize them. + */ + + state->plugins = g_ptr_array_new(); + + for (i = 0; i < plugins->len; i++) { + ToolsPlugin *plugin = g_ptr_array_index(plugins, i); + + plugin->data = plugin->onload(&state->ctx); + + if (plugin->data == NULL) { + g_info("Plugin '%s' didn't provide deployment data, unloading.\n", + plugin->fileName); + ToolsCoreFreePlugin(plugin); + } else if (state->ctx.errorCode != 0) { + /* Break early if a plugin has requested the container to quit. */ + ToolsCoreFreePlugin(plugin); + break; + } else { + ASSERT(plugin->data->name != NULL); + g_module_make_resident(plugin->module); + g_ptr_array_add(state->plugins, plugin); + VMTools_BindTextDomain(plugin->data->name, NULL, NULL); + g_debug("Plugin '%s' initialized.\n", plugin->data->name); + } + } + + /* * If there is a debug plugin, see if it exports standard plugin registration * data too. @@ -707,6 +747,9 @@ ToolsCore_LoadPlugins(ToolsServiceState *state) ret = TRUE; exit: + if (plugins != NULL) { + g_ptr_array_free(plugins, TRUE); + } g_free(pluginRoot); return ret; } @@ -874,11 +917,8 @@ ToolsCore_UnloadPlugins(ToolsServiceState *state) } g_ptr_array_remove_index(state->plugins, state->plugins->len - 1); - if (plugin->module != NULL) { - g_module_close(plugin->module); - } - g_free(plugin); - } + ToolsCoreFreePlugin(plugin); + } if (state->providers != NULL) { g_array_free(state->providers, TRUE); diff --git a/open-vm-tools/services/vmtoolsd/toolsCoreInt.h b/open-vm-tools/services/vmtoolsd/toolsCoreInt.h index 90aa16da4..a3a8eb6ab 100644 --- a/open-vm-tools/services/vmtoolsd/toolsCoreInt.h +++ b/open-vm-tools/services/vmtoolsd/toolsCoreInt.h @@ -49,12 +49,6 @@ # define G_MODULE_SUFFIX "dylib" #endif -/** Defines the internal data about a plugin. */ -typedef struct ToolsPlugin { - GModule *module; - ToolsPluginData *data; -} ToolsPlugin; - /** State of app providers. */ typedef enum { TOOLS_PROVIDER_IDLE,