From: Ondřej Surý Date: Tue, 20 Oct 2020 21:51:08 +0000 (+0200) Subject: Refactor the cleanup code in lt_dl code X-Git-Tag: v9.17.7~38^2~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e2436159aba301d63de60a4840ade8684c29af96;p=thirdparty%2Fbind9.git 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. --- diff --git a/bin/named/unix/dlz_dlopen_driver.c b/bin/named/unix/dlz_dlopen_driver.c index 0f8ff4d33c2..4cee110f786 100644 --- a/bin/named/unix/dlz_dlopen_driver.c +++ b/bin/named/unix/dlz_dlopen_driver.c @@ -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)); } /* diff --git a/lib/dns/dyndb.c b/lib/dns/dyndb.c index 38e320e4cc4..b91a2b9f262 100644 --- a/lib/dns/dyndb.c +++ b/lib/dns/dyndb.c @@ -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 **)®ister_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 diff --git a/lib/ns/hooks.c b/lib/ns/hooks.c index f79ef1b47bb..4e9bc9e632b 100644 --- a/lib/ns/hooks.c +++ b/lib/ns/hooks.c @@ -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 **)®ister_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; -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)); + return (ISC_R_SUCCESS); - if (plugin != NULL) { - isc_mem_putanddetach(&plugin->mctx, plugin, - sizeof(*plugin)); - } +cleanup: + 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 (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)); }