[asterisk-commits] tilghman: branch 1.6.1 r248009 - in /branches/1.6.1: ./ channels/ main/

[SVN commits to the Asterisk project]

Author: tilghman
Date: Fri Feb 19 13:05:34 2010
New Revision: 248009

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=248009
Log:
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
........

Modified:
    branches/1.6.1/   (props changed)
    branches/1.6.1/channels/chan_console.c
    branches/1.6.1/main/loader.c

Propchange: branches/1.6.1/
------------------------------------------------------------------------------
Binary property 'trunk-merged' - no diff available.

Modified: branches/1.6.1/channels/chan_console.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.6.1/channels/chan_console.c?view=diff&rev=248009&r1=248008&r2=248009
==============================================================================
--- branches/1.6.1/channels/chan_console.c (original)
+++ branches/1.6.1/channels/chan_console.c Fri Feb 19 13:05:34 2010
@@ -1502,6 +1502,7 @@
 return_error:
 	if (pvts)
 		ao2_ref(pvts, -1);
+	pvts = NULL;
 	pvt_destructor(&globals);
 
 	return AST_MODULE_LOAD_DECLINE;

Modified: branches/1.6.1/main/loader.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.6.1/main/loader.c?view=diff&rev=248009&r1=248008&r2=248009
==============================================================================
--- branches/1.6.1/main/loader.c (original)
+++ branches/1.6.1/main/loader.c Fri Feb 19 13:05:34 2010
@@ -443,26 +443,39 @@
 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);
+
+	/*!\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);
-
-	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);
-	}
 }
 
 int ast_unload_resource(const char *resource_name, enum ast_module_unload_mode force)