Refactor the cleanup code in lt_dl code

The cleanup code that would clean the object after plugin/dlz/dyndb
loading has failed was duplicating the destructor for the object, so
instead of the extra code, we just use the destructor instead.
This commit is contained in:
Ondřej Surý 2020-10-20 23:51:08 +02:00 committed by Michał Kępień
parent 4e9a58a3e6
commit e2436159ab
3 changed files with 73 additions and 106 deletions

View file

@ -197,6 +197,9 @@ dl_load_symbol(dlopen_data_t *cd, const char *symbol, bool mandatory) {
return (ptr);
}
static void
dlopen_dlz_destroy(void *driverarg, void *dbdata);
/*
* Called at startup for each dlopen zone in named.conf
*/
@ -324,15 +327,8 @@ dlopen_dlz_create(const char *dlzname, unsigned int argc, char *argv[],
failed:
dlopen_log(ISC_LOG_ERROR, "dlz_dlopen of '%s' failed", dlzname);
isc_mem_free(mctx, cd->dl_path);
isc_mem_free(mctx, cd->dlzname);
dlopen_dlz_destroy(NULL, cd);
isc_mutex_destroy(&cd->lock);
if (cd->dl_handle) {
(void)lt_dlclose(cd->dl_handle);
}
isc_mem_put(mctx, cd, sizeof(*cd));
isc_mem_destroy(&mctx);
return (result);
}
@ -342,7 +338,6 @@ failed:
static void
dlopen_dlz_destroy(void *driverarg, void *dbdata) {
dlopen_data_t *cd = (dlopen_data_t *)dbdata;
isc_mem_t *mctx;
UNUSED(driverarg);
@ -352,22 +347,13 @@ dlopen_dlz_destroy(void *driverarg, void *dbdata) {
MAYBE_UNLOCK(cd);
}
if (cd->dl_path) {
isc_mem_free(cd->mctx, cd->dl_path);
}
if (cd->dlzname) {
isc_mem_free(cd->mctx, cd->dlzname);
}
if (cd->dl_handle) {
lt_dlclose(cd->dl_handle);
}
isc_mutex_destroy(&cd->lock);
mctx = cd->mctx;
isc_mem_put(mctx, cd, sizeof(*cd));
isc_mem_destroy(&mctx);
isc_mem_free(cd->mctx, cd->dl_path);
isc_mem_free(cd->mctx, cd->dlzname);
isc_mem_putanddetach(&cd->mctx, cd, sizeof(*cd));
}
/*

View file

@ -105,14 +105,14 @@ load_symbol(lt_dlhandle handle, const char *filename, const char *symbol_name,
return (ISC_R_SUCCESS);
}
static void
unload_library(dyndb_implementation_t **impp);
static isc_result_t
load_library(isc_mem_t *mctx, const char *filename, const char *instname,
dyndb_implementation_t **impp) {
isc_result_t result;
lt_dlhandle handle = NULL;
dyndb_implementation_t *imp = NULL;
dns_dyndb_register_t *register_func = NULL;
dns_dyndb_destroy_t *destroy_func = NULL;
dns_dyndb_version_t *version_func = NULL;
int version;
@ -122,12 +122,20 @@ load_library(isc_mem_t *mctx, const char *filename, const char *instname,
ISC_LOG_INFO, "loading DynDB instance '%s' driver '%s'",
instname, filename);
imp = isc_mem_get(mctx, sizeof(*imp));
memset(imp, 0, sizeof(*imp));
isc_mem_attach(mctx, &imp->mctx);
imp->name = isc_mem_strdup(imp->mctx, instname);
INIT_LINK(imp, link);
if (lt_dlinit() != 0) {
CHECK(ISC_R_FAILURE);
}
handle = lt_dlopen(filename);
if (handle == NULL) {
imp->handle = lt_dlopen(filename);
if (imp->handle == NULL) {
const char *errmsg = lt_dlerror();
if (errmsg == NULL) {
errmsg = "unknown error";
@ -140,7 +148,7 @@ load_library(isc_mem_t *mctx, const char *filename, const char *instname,
CHECK(ISC_R_FAILURE);
}
CHECK(load_symbol(handle, filename, "dyndb_version",
CHECK(load_symbol(imp->handle, filename, "dyndb_version",
(void **)&version_func));
version = version_func(NULL);
@ -154,42 +162,23 @@ load_library(isc_mem_t *mctx, const char *filename, const char *instname,
CHECK(ISC_R_FAILURE);
}
CHECK(load_symbol(handle, filename, "dyndb_init",
(void **)&register_func));
CHECK(load_symbol(handle, filename, "dyndb_destroy",
(void **)&destroy_func));
imp = isc_mem_get(mctx, sizeof(dyndb_implementation_t));
imp->mctx = NULL;
isc_mem_attach(mctx, &imp->mctx);
imp->handle = handle;
imp->register_func = register_func;
imp->destroy_func = destroy_func;
imp->name = isc_mem_strdup(mctx, instname);
imp->inst = NULL;
INIT_LINK(imp, link);
CHECK(load_symbol(imp->handle, filename, "dyndb_init",
(void **)&imp->register_func));
CHECK(load_symbol(imp->handle, filename, "dyndb_destroy",
(void **)&imp->destroy_func));
*impp = imp;
imp = NULL;
return (ISC_R_SUCCESS);
cleanup:
if (result != ISC_R_SUCCESS) {
isc_log_write(dns_lctx, DNS_LOGCATEGORY_DATABASE,
DNS_LOGMODULE_DYNDB, ISC_LOG_ERROR,
"failed to dynamically load instance '%s' "
"driver '%s': %s (%s)",
instname, filename, lt_dlerror(),
isc_result_totext(result));
}
if (imp != NULL) {
isc_mem_putanddetach(&imp->mctx, imp,
sizeof(dyndb_implementation_t));
}
if (result != ISC_R_SUCCESS && handle != NULL) {
(void)lt_dlclose(handle);
}
isc_log_write(dns_lctx, DNS_LOGCATEGORY_DATABASE, DNS_LOGMODULE_DYNDB,
ISC_LOG_ERROR,
"failed to dynamically load DynDB instance '%s' driver "
"'%s': %s",
instname, filename, isc_result_totext(result));
unload_library(&imp);
return (result);
}
@ -203,8 +192,15 @@ unload_library(dyndb_implementation_t **impp) {
imp = *impp;
*impp = NULL;
/*
* This is a resource leak, but there is nothing we can currently do
* about it due to how configuration loading/reloading is designed.
*/
if (imp->handle != NULL) {
(void)lt_dlclose(imp->handle);
}
isc_mem_free(imp->mctx, imp->name);
isc_mem_putanddetach(&imp->mctx, imp, sizeof(dyndb_implementation_t));
isc_mem_putanddetach(&imp->mctx, imp, sizeof(*imp));
}
isc_result_t

View file

@ -117,25 +117,32 @@ load_symbol(void *handle, const char *modpath, const char *symbol_name,
return (ISC_R_SUCCESS);
}
static void
unload_plugin(ns_plugin_t **pluginp);
static isc_result_t
load_plugin(isc_mem_t *mctx, const char *modpath, ns_plugin_t **pluginp) {
isc_result_t result;
lt_dlhandle handle = NULL;
ns_plugin_t *plugin = NULL;
ns_plugin_check_t *check_func = NULL;
ns_plugin_register_t *register_func = NULL;
ns_plugin_destroy_t *destroy_func = NULL;
ns_plugin_version_t *version_func = NULL;
int version;
REQUIRE(pluginp != NULL && *pluginp == NULL);
plugin = isc_mem_get(mctx, sizeof(*plugin));
memset(plugin, 0, sizeof(*plugin));
isc_mem_attach(mctx, &plugin->mctx);
plugin->modpath = isc_mem_strdup(plugin->mctx, modpath);
ISC_LINK_INIT(plugin, link);
if (lt_dlinit() != 0) {
return (ISC_R_FAILURE);
CHECK(ISC_R_FAILURE);
}
handle = lt_dlopen(modpath);
if (handle == NULL) {
plugin->handle = lt_dlopen(modpath);
if (plugin->handle == NULL) {
const char *errmsg = lt_dlerror();
if (errmsg == NULL) {
errmsg = "unknown error";
@ -144,10 +151,10 @@ load_plugin(isc_mem_t *mctx, const char *modpath, ns_plugin_t **pluginp) {
NS_LOGMODULE_HOOKS, ISC_LOG_ERROR,
"failed to dlopen() plugin '%s': %s", modpath,
errmsg);
return (ISC_R_FAILURE);
CHECK(ISC_R_FAILURE);
}
CHECK(load_symbol(handle, modpath, "plugin_version",
CHECK(load_symbol(plugin->handle, modpath, "plugin_version",
(void **)&version_func));
version = version_func();
@ -161,44 +168,24 @@ load_plugin(isc_mem_t *mctx, const char *modpath, ns_plugin_t **pluginp) {
CHECK(ISC_R_FAILURE);
}
CHECK(load_symbol(handle, modpath, "plugin_check",
(void **)&check_func));
CHECK(load_symbol(handle, modpath, "plugin_register",
(void **)&register_func));
CHECK(load_symbol(handle, modpath, "plugin_destroy",
(void **)&destroy_func));
plugin = isc_mem_get(mctx, sizeof(*plugin));
memset(plugin, 0, sizeof(*plugin));
isc_mem_attach(mctx, &plugin->mctx);
plugin->handle = handle;
plugin->modpath = isc_mem_strdup(plugin->mctx, modpath);
plugin->check_func = check_func;
plugin->register_func = register_func;
plugin->destroy_func = destroy_func;
ISC_LINK_INIT(plugin, link);
CHECK(load_symbol(plugin->handle, modpath, "plugin_check",
(void **)&plugin->check_func));
CHECK(load_symbol(plugin->handle, modpath, "plugin_register",
(void **)&plugin->register_func));
CHECK(load_symbol(plugin->handle, modpath, "plugin_destroy",
(void **)&plugin->destroy_func));
*pluginp = plugin;
plugin = NULL;
return (ISC_R_SUCCESS);
cleanup:
if (result != ISC_R_SUCCESS) {
isc_log_write(ns_lctx, NS_LOGCATEGORY_GENERAL,
NS_LOGMODULE_HOOKS, ISC_LOG_ERROR,
"failed to dynamically load "
"plugin '%s': %s",
modpath, isc_result_totext(result));
isc_log_write(ns_lctx, NS_LOGCATEGORY_GENERAL, NS_LOGMODULE_HOOKS,
ISC_LOG_ERROR,
"failed to dynamically load plugin '%s': %s", modpath,
isc_result_totext(result));
if (plugin != NULL) {
isc_mem_putanddetach(&plugin->mctx, plugin,
sizeof(*plugin));
}
if (handle != NULL) {
(void)lt_dlclose(handle);
}
}
unload_plugin(&plugin);
return (result);
}
@ -219,13 +206,11 @@ unload_plugin(ns_plugin_t **pluginp) {
if (plugin->inst != NULL) {
plugin->destroy_func(&plugin->inst);
}
if (plugin->handle != NULL) {
(void)lt_dlclose(plugin->handle);
}
if (plugin->modpath != NULL) {
isc_mem_free(plugin->mctx, plugin->modpath);
}
isc_mem_free(plugin->mctx, plugin->modpath);
isc_mem_putanddetach(&plugin->mctx, plugin, sizeof(*plugin));
}