From 4178688b4a7c77cc5ca296fd6bc0b91fea0d0f2a Mon Sep 17 00:00:00 2001 From: Anthony Minessale Date: Wed, 9 Apr 2014 23:26:41 +0500 Subject: [PATCH] add switch_hashtable_insert_destructor so you can insert a pointer into a hash with a custom destructor and use it in spandsp to fix a leak on reloadxml with the tone_descriptor tables and fix a bunch of random tiny leaks etc --- .../private/switch_hashtable_private.h | 1 + src/include/switch_core.h | 4 ++- src/include/switch_hashtable.h | 3 +- src/include/switch_types.h | 2 ++ src/include/switch_utils.h | 2 +- src/mod/applications/mod_httapi/mod_httapi.c | 22 +++++++------- .../applications/mod_spandsp/mod_spandsp.c | 11 ++++++- .../mod_spandsp/mod_spandsp_fax.c | 1 - src/mod/languages/mod_v8/mod_v8.cpp | 1 + src/mod/loggers/mod_logfile/mod_logfile.c | 30 ++++++++----------- src/switch_core_hash.c | 4 +-- src/switch_hashtable.c | 12 +++++++- 12 files changed, 56 insertions(+), 37 deletions(-) diff --git a/src/include/private/switch_hashtable_private.h b/src/include/private/switch_hashtable_private.h index a68e7f3798..fbd08f11d5 100644 --- a/src/include/private/switch_hashtable_private.h +++ b/src/include/private/switch_hashtable_private.h @@ -22,6 +22,7 @@ struct entry void *k, *v; unsigned int h; hashtable_flag_t flags; + hashtable_destructor_t destructor; struct entry *next; }; diff --git a/src/include/switch_core.h b/src/include/switch_core.h index d636542bbd..b4e3e09353 100644 --- a/src/include/switch_core.h +++ b/src/include/switch_core.h @@ -1370,7 +1370,9 @@ SWITCH_DECLARE(switch_status_t) switch_core_hash_destroy(_Inout_ switch_hash_t * \return SWITCH_STATUS_SUCCESS if the data is added \note the string key must be a constant or a dynamic string */ -SWITCH_DECLARE(switch_status_t) switch_core_hash_insert(_In_ switch_hash_t *hash, _In_z_ const char *key, _In_opt_ const void *data); +SWITCH_DECLARE(switch_status_t) switch_core_hash_insert_destructor(_In_ switch_hash_t *hash, _In_z_ const char *key, _In_opt_ const void *data, hashtable_destructor_t destructor); +#define switch_core_hash_insert(_h, _k, _d) switch_core_hash_insert_destructor(_h, _k, _d, NULL) + /*! \brief Insert data into a hash diff --git a/src/include/switch_hashtable.h b/src/include/switch_hashtable.h index 4a588586a0..dddc89f286 100644 --- a/src/include/switch_hashtable.h +++ b/src/include/switch_hashtable.h @@ -123,7 +123,8 @@ typedef enum { } hashtable_flag_t; SWITCH_DECLARE(int) -switch_hashtable_insert(switch_hashtable_t *h, void *k, void *v, hashtable_flag_t flags); +switch_hashtable_insert_destructor(switch_hashtable_t *h, void *k, void *v, hashtable_flag_t flags, hashtable_destructor_t destructor); +#define switch_hashtable_insert(_h, _k, _v, _f) switch_hashtable_insert_destructor(_h, _k, _v, _f, NULL) #define DEFINE_HASHTABLE_INSERT(fnname, keytype, valuetype) \ int fnname (switch_hashtable_t *h, keytype *k, valuetype *v) \ diff --git a/src/include/switch_types.h b/src/include/switch_types.h index 1be1a96a00..949958835b 100644 --- a/src/include/switch_types.h +++ b/src/include/switch_types.h @@ -2079,6 +2079,8 @@ typedef struct switch_core_port_allocator switch_core_port_allocator_t; typedef struct switch_media_bug switch_media_bug_t; typedef struct switch_limit_interface switch_limit_interface_t; +typedef void (*hashtable_destructor_t)(void *ptr); + struct switch_console_callback_match_node { char *val; struct switch_console_callback_match_node *next; diff --git a/src/include/switch_utils.h b/src/include/switch_utils.h index 751b94632c..852c13009d 100644 --- a/src/include/switch_utils.h +++ b/src/include/switch_utils.h @@ -875,7 +875,7 @@ SWITCH_DECLARE(switch_time_t) switch_str_time(const char *in); \param vname the name of the global pointer to modify with the new function */ #define SWITCH_DECLARE_GLOBAL_STRING_FUNC(fname, vname) static void fname(const char *string) { if (!string) return;\ - if (vname) {free(vname); vname = NULL;}vname = strdup(string);} static void fname(const char *string) + if (vname) {vname = NULL;}vname = switch_core_permanent_strdup(string);} static void fname(const char *string) /*! \brief Separate a string into an array based on a character delimiter diff --git a/src/mod/applications/mod_httapi/mod_httapi.c b/src/mod/applications/mod_httapi/mod_httapi.c index c6f42120e6..b33a7ef7fe 100644 --- a/src/mod/applications/mod_httapi/mod_httapi.c +++ b/src/mod/applications/mod_httapi/mod_httapi.c @@ -2071,49 +2071,49 @@ static switch_status_t do_config(void) profile->auth_scheme = auth_scheme; profile->timeout = timeout; - profile->url = strdup(url); + profile->url = switch_core_strdup(globals.pool, url); switch_assert(profile->url); if (bind_local != NULL) { - profile->bind_local = strdup(bind_local); + profile->bind_local = switch_core_strdup(globals.pool, bind_local); } if (method != NULL) { - profile->method = strdup(method); + profile->method = switch_core_strdup(globals.pool, method); } else { profile->method = NULL; } if (bind_cred) { - profile->cred = strdup(bind_cred); + profile->cred = switch_core_strdup(globals.pool, bind_cred); } profile->disable100continue = disable100continue; profile->enable_cacert_check = enable_cacert_check; if (ssl_cert_file) { - profile->ssl_cert_file = strdup(ssl_cert_file); + profile->ssl_cert_file = switch_core_strdup(globals.pool, ssl_cert_file); } if (ssl_key_file) { - profile->ssl_key_file = strdup(ssl_key_file); + profile->ssl_key_file = switch_core_strdup(globals.pool, ssl_key_file); } if (ssl_key_password) { - profile->ssl_key_password = strdup(ssl_key_password); + profile->ssl_key_password = switch_core_strdup(globals.pool, ssl_key_password); } if (ssl_version) { - profile->ssl_version = strdup(ssl_version); + profile->ssl_version = switch_core_strdup(globals.pool, ssl_version); } if (ssl_cacert_file) { - profile->ssl_cacert_file = strdup(ssl_cacert_file); + profile->ssl_cacert_file = switch_core_strdup(globals.pool, ssl_cacert_file); } profile->enable_ssl_verifyhost = enable_ssl_verifyhost; if (cookie_file) { - profile->cookie_file = strdup(cookie_file); + profile->cookie_file = switch_core_strdup(globals.pool, cookie_file); } profile->vars_map = vars_map; @@ -2138,7 +2138,7 @@ static switch_status_t do_config(void) switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_NOTICE, "Profile [%s] JSON Function [%s]\n", zstr(bname) ? "N/A" : bname, profile->url); - profile->name = strdup(bname); + profile->name = switch_core_strdup(globals.pool, bname); switch_core_hash_insert(globals.profile_hash, bname, profile); diff --git a/src/mod/applications/mod_spandsp/mod_spandsp.c b/src/mod/applications/mod_spandsp/mod_spandsp.c index be7711aeb4..b3a4eeec8c 100644 --- a/src/mod/applications/mod_spandsp/mod_spandsp.c +++ b/src/mod/applications/mod_spandsp/mod_spandsp.c @@ -476,6 +476,12 @@ void mod_spandsp_indicate_data(switch_core_session_t *session, switch_bool_t sel /* ************************************************************************** CONFIGURATION ************************************************************************* */ +static void destroy_descriptor(void *ptr) +{ + tone_descriptor_t *d = (tone_descriptor_t *) ptr; + + super_tone_rx_free_descriptor(d->spandsp_tone_descriptor); +} switch_status_t load_configuration(switch_bool_t reload) { @@ -657,7 +663,8 @@ switch_status_t load_configuration(switch_bool_t reload) switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_CRIT, "Unable to allocate tone_descriptor: %s\n", name); switch_goto_status(SWITCH_STATUS_FALSE, done); } - switch_core_hash_insert(spandsp_globals.tones, name, descriptor); + + switch_core_hash_insert_destructor(spandsp_globals.tones, name, descriptor, destroy_descriptor); /* add tones to descriptor */ for (tone = switch_xml_child(xdescriptor, "tone"); tone; tone = switch_xml_next(tone)) { @@ -830,6 +837,8 @@ SWITCH_MODULE_SHUTDOWN_FUNCTION(mod_spandsp_shutdown) switch_core_destroy_memory_pool(&spandsp_globals.config_pool); } + memset(&spandsp_globals, 0, sizeof(spandsp_globals)); + return SWITCH_STATUS_UNLOAD; } diff --git a/src/mod/applications/mod_spandsp/mod_spandsp_fax.c b/src/mod/applications/mod_spandsp/mod_spandsp_fax.c index 82edadbbf4..9c97996a17 100644 --- a/src/mod/applications/mod_spandsp/mod_spandsp_fax.c +++ b/src/mod/applications/mod_spandsp/mod_spandsp_fax.c @@ -1645,7 +1645,6 @@ void mod_spandsp_fax_shutdown(void) t38_state_list.thread_running = 0; wake_thread(1); switch_thread_join(&tstatus, t38_state_list.thread); - memset(&spandsp_globals, 0, sizeof(spandsp_globals)); } static const switch_state_handler_table_t t38_gateway_state_handlers; diff --git a/src/mod/languages/mod_v8/mod_v8.cpp b/src/mod/languages/mod_v8/mod_v8.cpp index 1dfb6bc83e..4590b96867 100644 --- a/src/mod/languages/mod_v8/mod_v8.cpp +++ b/src/mod/languages/mod_v8/mod_v8.cpp @@ -872,6 +872,7 @@ SWITCH_MODULE_SHUTDOWN_FUNCTION(mod_v8_shutdown) switch_mutex_destroy(globals.event_mutex); switch_core_hash_destroy(&module_manager.load_hash); + switch_core_destroy_memory_pool(&module_manager.pool); return SWITCH_STATUS_SUCCESS; } diff --git a/src/mod/loggers/mod_logfile/mod_logfile.c b/src/mod/loggers/mod_logfile/mod_logfile.c index d7024a07e7..15d972ba7c 100644 --- a/src/mod/loggers/mod_logfile/mod_logfile.c +++ b/src/mod/loggers/mod_logfile/mod_logfile.c @@ -313,6 +313,17 @@ static switch_status_t mod_logfile_logger(const switch_log_node_t *node, switch_ return process_node(node, level); } +static void cleanup_profile(void *ptr) +{ + logfile_profile_t *profile = (logfile_profile_t *) ptr; + + switch_core_hash_destroy(&profile->log_hash); + switch_file_close(profile->log_afd); + switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_INFO, "Closing %s\n", profile->logfile); + switch_safe_free(profile->logfile); + +} + static switch_status_t load_profile(switch_xml_t xml) { switch_xml_t param, settings; @@ -365,7 +376,7 @@ static switch_status_t load_profile(switch_xml_t xml) return SWITCH_STATUS_GENERR; } - switch_core_hash_insert(profile_hash, new_profile->name, (void *) new_profile); + switch_core_hash_insert_destructor(profile_hash, new_profile->name, (void *) new_profile, cleanup_profile); return SWITCH_STATUS_SUCCESS; } @@ -454,26 +465,9 @@ SWITCH_MODULE_LOAD_FUNCTION(mod_logfile_load) SWITCH_MODULE_SHUTDOWN_FUNCTION(mod_logfile_shutdown) { - switch_hash_index_t *hi; - const void *var; - void *val; - switch_log_unbind_logger(mod_logfile_logger); switch_event_unbind(&globals.node); - - for (hi = switch_core_hash_first(profile_hash); hi; hi = switch_core_hash_next(&hi)) { - logfile_profile_t *profile; - switch_core_hash_this(hi, &var, NULL, &val); - if ((profile = (logfile_profile_t *) val)) { - switch_file_close(profile->log_afd); - switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_INFO, "Closing %s\n", profile->logfile); - switch_safe_free(profile->logfile); - } - } - switch_core_hash_destroy(&profile_hash); - - return SWITCH_STATUS_SUCCESS; } diff --git a/src/switch_core_hash.c b/src/switch_core_hash.c index 343f43e6b3..e997ac9bcc 100644 --- a/src/switch_core_hash.c +++ b/src/switch_core_hash.c @@ -55,9 +55,9 @@ SWITCH_DECLARE(switch_status_t) switch_core_hash_destroy(switch_hash_t **hash) return SWITCH_STATUS_SUCCESS; } -SWITCH_DECLARE(switch_status_t) switch_core_hash_insert(switch_hash_t *hash, const char *key, const void *data) +SWITCH_DECLARE(switch_status_t) switch_core_hash_insert_destructor(switch_hash_t *hash, const char *key, const void *data, hashtable_destructor_t destructor) { - switch_hashtable_insert(hash, strdup(key), (void *)data, HASHTABLE_FLAG_FREE_KEY); + switch_hashtable_insert_destructor(hash, strdup(key), (void *)data, HASHTABLE_FLAG_FREE_KEY, destructor); return SWITCH_STATUS_SUCCESS; } diff --git a/src/switch_hashtable.c b/src/switch_hashtable.c index ab56b1ebac..4155c7c335 100644 --- a/src/switch_hashtable.c +++ b/src/switch_hashtable.c @@ -155,7 +155,7 @@ switch_hashtable_count(switch_hashtable_t *h) /*****************************************************************************/ SWITCH_DECLARE(int) -switch_hashtable_insert(switch_hashtable_t *h, void *k, void *v, hashtable_flag_t flags) +switch_hashtable_insert_destructor(switch_hashtable_t *h, void *k, void *v, hashtable_flag_t flags, hashtable_destructor_t destructor) { /* This method allows duplicate keys - but they shouldn't be used */ unsigned int index; @@ -175,6 +175,7 @@ switch_hashtable_insert(switch_hashtable_t *h, void *k, void *v, hashtable_flag_ e->k = k; e->v = v; e->flags = flags; + e->destructor = destructor; e->next = h->table[index]; h->table[index] = e; return -1; @@ -222,6 +223,12 @@ switch_hashtable_remove(switch_hashtable_t *h, void *k) if (e->flags & HASHTABLE_FLAG_FREE_KEY) { freekey(e->k); } + if (e->flags & HASHTABLE_FLAG_FREE_VALUE) { + switch_safe_free(e->v); + } else if (e->destructor) { + e->destructor(e->v); + e->v = NULL; + } switch_safe_free(e); return v; } @@ -251,6 +258,9 @@ switch_hashtable_destroy(switch_hashtable_t **h) if (f->flags & HASHTABLE_FLAG_FREE_VALUE) { switch_safe_free(f->v); + } else if (f->destructor) { + f->destructor(f->v); + f->v = NULL; } switch_safe_free(f); }