[svn-commits] mjordan: trunk r419563 - in /trunk: main/loader.c res/res_calendar.c
SVN commits to the Digium repositories
svn-commits at lists.digium.com
Fri Jul 25 09:27:56 CDT 2014
Author: mjordan
Date: Fri Jul 25 09:27:52 2014
New Revision: 419563
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=419563
Log:
module loader: Unload modules in reverse order of their start order
When Asterisk starts a module (calling its load_module function), it re-orders
the module list, sorting it alphabetically. Ostensibly, this was done so that
the output of 'module show' listed modules in alphabetic order. This had the
unfortunate side effect of making modules with complex usage patterns
unloadable. A module that has a large number of modules that depend on it is
typically abandoned during the unloading process. This results in its memory
not being reclaimed during exit.
Generally, this isn't harmful - when the process is destroyed, the operating
system will reclaim all memory allocated by the process. Prior to Asterisk 12,
we also didn't have many modules with complex dependencies. However, with
the advent of ARI and PJSIP, this can make make unloading those modules
successfully nearly impossible, and thus tracking memory leaks or ref debug
leaks a real pain.
While this patch is not a complete overhaul of the module loader - such an
effort would be beyond the scope of what could be done for Asterisk 13 -
this does make some marginal improvements to the loader such that modules
like res_pjsip or res_stasis *may* be made properly un-loadable in the future.
1. The linked list of modules has been replaced with a doubly linked list. This
allows traversal of the module list to occur backwards. The module shutdown
routine now walks the global list backwards when it attempts to unload
modules.
2. The alphabetic reorganization of the module list on startup has been
removed. Instead, a started module is placed at the end of the module list.
3. The ast_update_module_list function - which is used by the CLI to display
the modules - now does the sorting alphabetically itself. It creates its own
linked list and inserts the modules into it in alphabetic order. This allows
for the intent of the previous code to be maintained.
This patch also contains a fix for res_calendar. Without calendar.conf, the
calendar modules were improperly bumping the use count of res_calendar, then
failing to load themselves. This patch makes it so that we detect whether or
not calendaring is enabled before altering the use count.
Review: https://reviewboard.asterisk.org/r/3777/
Modified:
trunk/main/loader.c
trunk/res/res_calendar.c
Modified: trunk/main/loader.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/loader.c?view=diff&rev=419563&r1=419562&r2=419563
==============================================================================
--- trunk/main/loader.c (original)
+++ trunk/main/loader.c Fri Jul 25 09:27:52 2014
@@ -39,7 +39,7 @@
#include "asterisk/paths.h" /* use ast_config_AST_MODULE_DIR */
#include <dirent.h>
-#include "asterisk/linkedlists.h"
+#include "asterisk/dlinkedlists.h"
#include "asterisk/module.h"
#include "asterisk/config.h"
#include "asterisk/channel.h"
@@ -103,7 +103,7 @@
AST_LIST_ENTRY(ast_module_user) entry;
};
-AST_LIST_HEAD(module_user_list, ast_module_user);
+AST_DLLIST_HEAD(module_user_list, ast_module_user);
static const unsigned char expected_key[] =
{ 0x87, 0x76, 0x79, 0x35, 0x23, 0xea, 0x3a, 0xd3,
@@ -124,11 +124,12 @@
unsigned int running:1;
unsigned int declined:1;
} flags;
- AST_LIST_ENTRY(ast_module) entry;
+ AST_LIST_ENTRY(ast_module) list_entry;
+ AST_DLLIST_ENTRY(ast_module) entry;
char resource[0];
};
-static AST_LIST_HEAD_STATIC(module_list, ast_module);
+static AST_DLLIST_HEAD_STATIC(module_list, ast_module);
const char *ast_module_name(const struct ast_module *mod)
{
@@ -152,7 +153,7 @@
AST_LIST_ENTRY(loadupdate) entry;
};
-static AST_LIST_HEAD_STATIC(updaters, loadupdate);
+static AST_DLLIST_HEAD_STATIC(updaters, loadupdate);
AST_MUTEX_DEFINE_STATIC(reloadlock);
@@ -163,7 +164,7 @@
static int do_full_reload = 0;
-static AST_LIST_HEAD_STATIC(reload_queue, reload_queue_item);
+static AST_DLLIST_HEAD_STATIC(reload_queue, reload_queue_item);
/* when dynamic modules are being loaded, ast_module_register() will
need to know what filename the module was loaded from while it
@@ -197,16 +198,16 @@
let's avoid it altogether
*/
if (embedding) {
- AST_LIST_INSERT_TAIL(&embedded_module_list, mod, entry);
+ AST_DLLIST_INSERT_TAIL(&embedded_module_list, mod, entry);
} else {
- AST_LIST_LOCK(&module_list);
+ AST_DLLIST_LOCK(&module_list);
/* it is paramount that the new entry be placed at the tail of
the list, otherwise the code that uses dlopen() to load
dynamic modules won't be able to find out if the module it
just opened was registered or failed to load
*/
- AST_LIST_INSERT_TAIL(&module_list, mod, entry);
- AST_LIST_UNLOCK(&module_list);
+ AST_DLLIST_INSERT_TAIL(&module_list, mod, entry);
+ AST_DLLIST_UNLOCK(&module_list);
}
/* give the module a copy of its own handle, for later use in registrations and the like */
@@ -221,15 +222,15 @@
will already be empty, or we cannot have gotten to this
point
*/
- AST_LIST_LOCK(&module_list);
- AST_LIST_TRAVERSE_SAFE_BEGIN(&module_list, mod, entry) {
+ AST_DLLIST_LOCK(&module_list);
+ AST_DLLIST_TRAVERSE_BACKWARDS_SAFE_BEGIN(&module_list, mod, entry) {
if (mod->info == info) {
- AST_LIST_REMOVE_CURRENT(entry);
+ AST_DLLIST_REMOVE_CURRENT(entry);
break;
}
}
- AST_LIST_TRAVERSE_SAFE_END;
- AST_LIST_UNLOCK(&module_list);
+ AST_DLLIST_TRAVERSE_BACKWARDS_SAFE_END;
+ AST_DLLIST_UNLOCK(&module_list);
if (mod) {
ast_debug(5, "Unregistering module %s\n", info->name);
@@ -309,22 +310,22 @@
const char *name;
int (*reload_fn)(void);
} reload_classes[] = { /* list in alpha order, longest match first for cli completion */
- { "cdr", ast_cdr_engine_reload },
- { "dnsmgr", dnsmgr_reload },
- { "extconfig", read_config_maps },
- { "enum", ast_enum_reload },
- { "acl", ast_named_acl_reload },
- { "manager", reload_manager },
- { "http", ast_http_reload },
- { "logger", logger_reload },
- { "features", ast_features_config_reload },
- { "dsp", ast_dsp_reload},
- { "udptl", ast_udptl_reload },
+ { "acl", ast_named_acl_reload },
+ { "cdr", ast_cdr_engine_reload },
+ { "cel", ast_cel_engine_reload },
+ { "dnsmgr", dnsmgr_reload },
+ { "dsp", ast_dsp_reload},
+ { "extconfig", read_config_maps },
+ { "enum", ast_enum_reload },
+ { "features", ast_features_config_reload },
+ { "http", ast_http_reload },
{ "indications", ast_indications_reload },
- { "cel", ast_cel_engine_reload },
- { "plc", ast_plc_reload },
- { "sounds", ast_sounds_reindex },
- { NULL, NULL }
+ { "logger", logger_reload },
+ { "manager", reload_manager },
+ { "plc", ast_plc_reload },
+ { "sounds", ast_sounds_reindex },
+ { "udptl", ast_udptl_reload },
+ { NULL, NULL }
};
static int printdigest(const unsigned char *d)
@@ -391,16 +392,18 @@
{
struct ast_module *cur;
- if (do_lock)
- AST_LIST_LOCK(&module_list);
-
- AST_LIST_TRAVERSE(&module_list, cur, entry) {
+ if (do_lock) {
+ AST_DLLIST_LOCK(&module_list);
+ }
+
+ AST_DLLIST_TRAVERSE(&module_list, cur, entry) {
if (!resource_name_match(resource, cur->resource))
break;
}
- if (do_lock)
- AST_LIST_UNLOCK(&module_list);
+ if (do_lock) {
+ AST_DLLIST_UNLOCK(&module_list);
+ }
return cur;
}
@@ -528,7 +531,7 @@
constructor) places the new module at the tail of the
module_list
*/
- if (resource_being_loaded != (mod = AST_LIST_LAST(&module_list))) {
+ if (resource_being_loaded != (mod = AST_DLLIST_LAST(&module_list))) {
ast_log(LOG_WARNING, "Module '%s' did not register itself during load\n", resource_in);
/* no, it did not, so close it and return */
logged_dlclose(resource_in, lib);
@@ -567,10 +570,10 @@
the previous time we did that, we're going to assume it worked this
time too :) */
- AST_LIST_LAST(&module_list)->lib = lib;
+ AST_DLLIST_LAST(&module_list)->lib = lib;
resource_being_loaded = NULL;
- return AST_LIST_LAST(&module_list);
+ return AST_DLLIST_LAST(&module_list);
}
#endif
@@ -580,7 +583,7 @@
struct ast_module *mod;
int somethingchanged = 1, final = 0;
- AST_LIST_LOCK(&module_list);
+ AST_DLLIST_LOCK(&module_list);
/*!\note Some resources, like timers, are started up dynamically, and thus
* may be still in use, even if all channels are dead. We must therefore
@@ -595,11 +598,13 @@
/* Reset flag before traversing the list */
somethingchanged = 0;
- AST_LIST_TRAVERSE_SAFE_BEGIN(&module_list, mod, entry) {
+ AST_DLLIST_TRAVERSE_BACKWARDS_SAFE_BEGIN(&module_list, mod, entry) {
if (!final && mod->usecount) {
+ ast_debug(1, "Passing on %s: its use count is %d\n",
+ mod->resource, mod->usecount);
continue;
}
- AST_LIST_REMOVE_CURRENT(entry);
+ AST_DLLIST_REMOVE_CURRENT(entry);
if (mod->flags.running && !mod->flags.declined && mod->info->unload) {
ast_verb(1, "Unloading %s\n", mod->resource);
mod->info->unload();
@@ -608,10 +613,10 @@
free(mod);
somethingchanged = 1;
}
- AST_LIST_TRAVERSE_SAFE_END;
+ AST_DLLIST_TRAVERSE_BACKWARDS_SAFE_END;
} while (somethingchanged && !final);
- AST_LIST_UNLOCK(&module_list);
+ AST_DLLIST_UNLOCK(&module_list);
}
int ast_unload_resource(const char *resource_name, enum ast_module_unload_mode force)
@@ -620,10 +625,10 @@
int res = -1;
int error = 0;
- AST_LIST_LOCK(&module_list);
+ AST_DLLIST_LOCK(&module_list);
if (!(mod = find_resource(resource_name, 0))) {
- AST_LIST_UNLOCK(&module_list);
+ AST_DLLIST_UNLOCK(&module_list);
ast_log(LOG_WARNING, "Unload failed, '%s' could not be found\n", resource_name);
return -1;
}
@@ -672,7 +677,7 @@
if (!error)
mod->flags.running = mod->flags.declined = 0;
- AST_LIST_UNLOCK(&module_list);
+ AST_DLLIST_UNLOCK(&module_list);
if (!error && !mod->lib && mod->info && mod->info->restore_globals)
mod->info->restore_globals();
@@ -699,8 +704,8 @@
if (pos != rpos)
return NULL;
- AST_LIST_LOCK(&module_list);
- AST_LIST_TRAVERSE(&module_list, cur, entry) {
+ AST_DLLIST_LOCK(&module_list);
+ AST_DLLIST_TRAVERSE(&module_list, cur, entry) {
if (!strncasecmp(word, cur->resource, l) &&
(cur->info->reload || !needsreload) &&
++which > state) {
@@ -708,7 +713,7 @@
break;
}
}
- AST_LIST_UNLOCK(&module_list);
+ AST_DLLIST_UNLOCK(&module_list);
if (!ret) {
for (i=0; !ret && reload_classes[i].name; i++) {
@@ -879,8 +884,8 @@
goto module_reload_exit;
}
- AST_LIST_LOCK(&module_list);
- AST_LIST_TRAVERSE(&module_list, cur, entry) {
+ AST_DLLIST_LOCK(&module_list);
+ AST_DLLIST_TRAVERSE(&module_list, cur, entry) {
const struct ast_module_info *info = cur->info;
if (name && resource_name_match(name, cur->resource))
@@ -913,7 +918,7 @@
break;
}
}
- AST_LIST_UNLOCK(&module_list);
+ AST_DLLIST_UNLOCK(&module_list);
if (ast_opt_lock_confdir) {
ast_unlock_path(ast_config_AST_CONFIG_DIR);
@@ -990,6 +995,12 @@
case AST_MODULE_LOAD_PRIORITY:
break;
}
+
+ /* Make sure the newly started module is at the end of the list */
+ AST_DLLIST_LOCK(&module_list);
+ AST_DLLIST_REMOVE(&module_list, mod, entry);
+ AST_DLLIST_INSERT_TAIL(&module_list, mod, entry);
+ AST_DLLIST_UNLOCK(&module_list);
return res;
}
@@ -1054,24 +1065,18 @@
res = start_resource(mod);
}
- /* Now make sure that the list is sorted */
- AST_LIST_LOCK(&module_list);
- AST_LIST_REMOVE(&module_list, mod, entry);
- AST_LIST_INSERT_SORTALPHA(&module_list, mod, entry, resource);
- AST_LIST_UNLOCK(&module_list);
-
return res;
}
int ast_load_resource(const char *resource_name)
{
int res;
- AST_LIST_LOCK(&module_list);
+ AST_DLLIST_LOCK(&module_list);
res = load_resource(resource_name, 0, NULL, 0);
if (!res) {
ast_test_suite_event_notify("MODULE_LOAD", "Message: %s", resource_name);
}
- AST_LIST_UNLOCK(&module_list);
+ AST_DLLIST_UNLOCK(&module_list);
return res;
}
@@ -1213,7 +1218,7 @@
AST_LIST_HEAD_INIT_NOLOCK(&load_order);
- AST_LIST_LOCK(&module_list);
+ AST_DLLIST_LOCK(&module_list);
if (embedded_module_list.first) {
module_list.first = embedded_module_list.first;
@@ -1243,7 +1248,7 @@
/* check if 'autoload' is on */
if (!preload_only && ast_true(ast_variable_retrieve(cfg, "modules", "autoload"))) {
/* if so, first add all the embedded modules that are not already running to the load order */
- AST_LIST_TRAVERSE(&module_list, mod, entry) {
+ AST_DLLIST_TRAVERSE(&module_list, mod, entry) {
/* if it's not embedded, skip it */
if (mod->lib)
continue;
@@ -1329,7 +1334,7 @@
ast_free(order);
}
- AST_LIST_UNLOCK(&module_list);
+ AST_DLLIST_UNLOCK(&module_list);
return res;
}
@@ -1351,17 +1356,24 @@
struct ast_module *cur;
int unlock = -1;
int total_mod_loaded = 0;
-
- if (AST_LIST_TRYLOCK(&module_list))
+ AST_LIST_HEAD_NOLOCK(, ast_module) alpha_module_list = AST_LIST_HEAD_NOLOCK_INIT_VALUE;
+
+ if (AST_DLLIST_TRYLOCK(&module_list)) {
unlock = 0;
-
- AST_LIST_TRAVERSE(&module_list, cur, entry) {
+ }
+
+ AST_DLLIST_TRAVERSE(&module_list, cur, entry) {
+ AST_LIST_INSERT_SORTALPHA(&alpha_module_list, cur, list_entry, resource);
+ }
+
+ AST_LIST_TRAVERSE(&alpha_module_list, cur, list_entry) {
total_mod_loaded += modentry(cur->resource, cur->info->description, cur->usecount,
cur->flags.running ? "Running" : "Not Running", like);
}
- if (unlock)
- AST_LIST_UNLOCK(&module_list);
+ if (unlock) {
+ AST_DLLIST_UNLOCK(&module_list);
+ }
return total_mod_loaded;
}
Modified: trunk/res/res_calendar.c
URL: http://svnview.digium.com/svn/asterisk/trunk/res/res_calendar.c?view=diff&rev=419563&r1=419562&r2=419563
==============================================================================
--- trunk/res/res_calendar.c (original)
+++ trunk/res/res_calendar.c Fri Jul 25 09:27:52 2014
@@ -538,6 +538,11 @@
{
struct ast_calendar_tech *iter;
+ if (!calendar_config) {
+ ast_log(LOG_WARNING, "Calendar support disabled, not loading %s calendar module\n", tech->type);
+ return -1;
+ }
+
AST_LIST_LOCK(&techs);
AST_LIST_TRAVERSE(&techs, iter, list) {
if(!strcasecmp(tech->type, iter->type)) {
More information about the svn-commits
mailing list