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 0f8ff4d33c29a37464bf6ce8d0d3e47409a4dbb9..4cee110f786c1a8227785bf0f05cd3e505f74234 100644 (file)
--- 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 38e320e4cc4bfabdd851406ec31d57d618c719dc..b91a2b9f2621cb7743c3a1cb8b41a95895cb1d52 100644 (file)
--- 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 **)&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

diff --git a/lib/ns/hooks.c b/lib/ns/hooks.c
index f79ef1b47bbdf7d9c25483b15024e80e4929400c..4e9bc9e632b0be7e05b4394baddd7d5ea71f1045 100644 (file)
--- 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 **)&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;
 
-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));
 }