Merged revisions 228798 via svnmerge from

https://origsvn.digium.com/svn/asterisk/trunk

(closes issue #16470)
 Reported by: kjotte

........
  r228798 | tilghman | 2009-11-09 01:37:52 -0600 (Mon, 09 Nov 2009) | 14 lines

  Fix various problems detected with Valgrind.
   * chan_console accessed pvts after deallocation.
   * The module loader did not check usecount on shutdown, which led to chan_iax2
   reading a timer that was already unloaded.
  (closes issue #16062)
   Reported by: alexanderheinz
   Patches:
         20091109__issue16062.diff.txt uploaded by tilghman (license 14)
   Tested by: tilghman
........

git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.6.0@248008 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/channels/chan_console.c b/channels/chan_console.c
index c125a66d709fba54f6903f1476300fd788ecd227..0d2d6360b9604a4b1e01b44fc08ee3fb365e8ee7 100644 (file)
--- a/channels/chan_console.c
+++ b/channels/chan_console.c
@@ -1496,6 +1496,7 @@ return_error_pa_init:
 return_error:
        if (pvts)
                ao2_ref(pvts, -1);
+       pvts = NULL;
        pvt_destructor(&globals);
 
        return AST_MODULE_LOAD_DECLINE;

diff --git a/main/loader.c b/main/loader.c
index 34a887e14d866c8ee00c1e464a0be65c4b8c0e21..0c8f306c1f52fd9488043f7e538b394bdb11c707 100644 (file)
--- a/main/loader.c
+++ b/main/loader.c
@@ -462,26 +462,39 @@ static struct ast_module *load_dynamic_module(const char *resource_in, enum modu
 void ast_module_shutdown(void)
 {
        struct ast_module *mod;
-       AST_LIST_HEAD_NOLOCK_STATIC(local_module_list, ast_module);
-
-       /* We have to call the unload() callbacks in reverse order that the modules
-        * exist in the module list so it is the reverse order of how they were
-        * loaded. */
+       int somethingchanged = 1, final = 0;
 
        AST_LIST_LOCK(&module_list);
-       while ((mod = AST_LIST_REMOVE_HEAD(&module_list, entry)))
-               AST_LIST_INSERT_HEAD(&local_module_list, mod, entry);
-       AST_LIST_UNLOCK(&module_list);
 
-       while ((mod = AST_LIST_REMOVE_HEAD(&local_module_list, entry))) {
-               if (mod->info->unload)
-                       mod->info->unload();
-               /* Since this should only be called when shutting down "gracefully",
-                * all channels should be down before we get to this point, meaning
-                * there will be no module users left. */
-               AST_LIST_HEAD_DESTROY(&mod->users);
-               free(mod);
-       }
+       /*!\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
+        * check the usecount before asking modules to unload. */
+       do {
+               if (!somethingchanged) {
+                       /*!\note If we go through the entire list without changing
+                        * anything, ignore the usecounts and unload, then exit. */
+                       final = 1;
+               }
+
+               /* Reset flag before traversing the list */
+               somethingchanged = 0;
+
+               AST_LIST_TRAVERSE_SAFE_BEGIN(&module_list, mod, entry) {
+                       if (!final && mod->usecount) {
+                               continue;
+                       }
+                       AST_LIST_REMOVE_CURRENT(entry);
+                       if (mod->info->unload) {
+                               mod->info->unload();
+                       }
+                       AST_LIST_HEAD_DESTROY(&mod->users);
+                       free(mod);
+                       somethingchanged = 1;
+               }
+               AST_LIST_TRAVERSE_SAFE_END;
+       } while (somethingchanged && !final);
+
+       AST_LIST_UNLOCK(&module_list);
 }
 
 int ast_unload_resource(const char *resource_name, enum ast_module_unload_mode force)