Merged revisions 233718 via svnmerge from

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

........
  r233718 | tilghman | 2009-12-08 12:22:44 -0600 (Tue, 08 Dec 2009) | 8 lines
  
  Find another ref leak and change how we manage module references.
  (closes issue #16388)
   Reported by: parisioa
   Patches: 
         20091208__issue16388.diff.txt uploaded by tilghman (license 14)
   Tested by: parisioa, tilghman
   Review: https://reviewboard.asterisk.org/r/442/
........


git-svn-id: http://svn.digium.com/svn/asterisk/branches/1.6.2@233731 f38db490-d61c-443f-a65b-d21fe96a405b

1 files changed, 40 insertions, 75 deletions

diff --git a/res/res_musiconhold.c b/res/res_musiconhold.c
index 33ae02413..0fd0efc74 100644
--- a/res/res_musiconhold.c
+++ b/res/res_musiconhold.c
@@ -186,8 +186,6 @@ struct mohdata {
 };
 
 static struct ao2_container *mohclasses;
-static struct ao2_container *deleted_classes;
-static pthread_t deleted_thread;
 
 #define LOCAL_MPG_123 "/usr/local/bin/mpg123"
 #define MPG_123 "/usr/bin/mpg123"
@@ -196,7 +194,32 @@ static pthread_t deleted_thread;
 static int reload(void);
 
 #define mohclass_ref(class,string)   (ao2_t_ref((class), +1, (string)), class)
+
+#ifndef REF_DEBUG
 #define mohclass_unref(class,string) (ao2_t_ref((class), -1, (string)), (struct mohclass *) NULL)
+#else
+#define mohclass_unref(class,string) _mohclass_unref(class, string, __FILE__,__LINE__,__PRETTY_FUNCTION__)
+static struct mohclass *_mohclass_unref(struct mohclass *class, const char *tag, const char *file, int line, const char *funcname)
+{
+	struct mohclass *dup;
+	if ((dup = ao2_find(mohclasses, class, OBJ_POINTER))) {
+		if (_ao2_ref_debug(dup, -1, (char *) tag, (char *) file, line, funcname) == 2) {
+			FILE *ref = fopen("/tmp/refs", "a");
+			if (ref) {
+				fprintf(ref, "%p =1   %s:%d:%s (%s) BAD ATTEMPT!\n", class, file, line, funcname, tag);
+				fclose(ref);
+			}
+			ast_log(LOG_WARNING, "Attempt to unref mohclass %p (%s) when only 1 ref remained, and class is still in a container! (at %s:%d (%s))\n",
+				class, class->name, file, line, funcname);
+		} else {
+			ao2_ref(class, -1);
+		}
+	} else {
+		ao2_t_ref(class, -1, (char *) tag);
+	}
+	return NULL;
+}
+#endif
 
 static void moh_files_release(struct ast_channel *chan, void *data)
 {
@@ -324,6 +347,7 @@ static void *moh_files_alloc(struct ast_channel *chan, void *params)
 
 	if (!chan->music_state && (state = ast_calloc(1, sizeof(*state)))) {
 		chan->music_state = state;
+		ast_module_ref(ast_module_info->self);
 	} else {
 		state = chan->music_state;
 	}
@@ -532,43 +556,6 @@ static int spawn_mp3(struct mohclass *class)
 	return fds[0];
 }
 
-static void *deleted_monitor(void *data)
-{
-	struct ao2_iterator iter;
-	struct mohclass *class;
-	struct ast_module *mod = NULL;
-
-	for (;;) {
-		pthread_testcancel();
-		if (ao2_container_count(deleted_classes) == 0) {
-			if (mod) {
-				ast_module_unref(mod);
-				mod = NULL;
-			}
-			poll(NULL, 0, -1);
-		} else {
-			/* While deleted classes are still in use, prohibit unloading */
-			mod = ast_module_ref(ast_module_info->self);
-		}
-		pthread_testcancel();
-		iter = ao2_iterator_init(deleted_classes, 0);
-		while ((class = ao2_iterator_next(&iter))) {
-			if (ao2_ref(class, 0) == 2) {
-				ao2_unlink(deleted_classes, class);
-			}
-			ao2_ref(class, -1);
-		}
-		ao2_iterator_destroy(&iter);
-		if (ao2_container_count(deleted_classes) == 0 && mod) {
-			ast_module_unref(mod);
-			mod = NULL;
-		}
-		pthread_testcancel();
-		sleep(60);
-	}
-	return NULL;
-}
-
 static void *monmp3thread(void *data)
 {
 #define	MOH_MS_INTERVAL		100
@@ -871,7 +858,8 @@ static void *moh_alloc(struct ast_channel *chan, void *params)
 	/* Initiating music_state for current channel. Channel should know name of moh class */
 	if (!chan->music_state && (state = ast_calloc(1, sizeof(*state)))) {
 		chan->music_state = state;
-		state->class = class;
+		state->class = mohclass_ref(class, "Copying reference into state container");
+		ast_module_ref(ast_module_info->self);
 	} else
 		state = chan->music_state;
 	if (state && state->class != class) {
@@ -1156,19 +1144,25 @@ static int _moh_register(struct mohclass *moh, int reload, int unref, const char
 	
 	if (!strcasecmp(moh->mode, "files")) {
 		if (init_files_class(moh)) {
-			moh = mohclass_unref(moh, "unreffing potential new moh class (init_files_class failed)");
+			if (unref) {
+				moh = mohclass_unref(moh, "unreffing potential new moh class (init_files_class failed)");
+			}
 			return -1;
 		}
 	} else if (!strcasecmp(moh->mode, "mp3") || !strcasecmp(moh->mode, "mp3nb") || 
 			!strcasecmp(moh->mode, "quietmp3") || !strcasecmp(moh->mode, "quietmp3nb") || 
 			!strcasecmp(moh->mode, "httpmp3") || !strcasecmp(moh->mode, "custom")) {
 		if (init_app_class(moh)) {
-			moh = mohclass_unref(moh, "unreffing potential new moh class (init_app_class_failed)");
+			if (unref) {
+				moh = mohclass_unref(moh, "unreffing potential new moh class (init_app_class_failed)");
+			}
 			return -1;
 		}
 	} else {
 		ast_log(LOG_WARNING, "Don't know how to do a mode '%s' music on hold\n", moh->mode);
-		moh = mohclass_unref(moh, "unreffing potential new moh class (unknown mode)");
+		if (unref) {
+			moh = mohclass_unref(moh, "unreffing potential new moh class (unknown mode)");
+		}
 		return -1;
 	}
 
@@ -1191,6 +1185,8 @@ static void local_ast_moh_cleanup(struct ast_channel *chan)
 		}
 		ast_free(chan->music_state);
 		chan->music_state = NULL;
+		/* Only held a module reference if we had a music state */
+		ast_module_unref(ast_module_info->self);
 	}
 }
 
@@ -1540,12 +1536,7 @@ static int moh_classes_delete_marked(void *obj, void *arg, int flags)
 {
 	struct mohclass *class = obj;
 
-	if (class->delete) {
-		ao2_link(deleted_classes, obj);
-		pthread_kill(deleted_thread, SIGURG);
-		return CMP_MATCH;
-	}
-	return 0;
+	return class->delete ? CMP_MATCH : 0;
 }
 
 static int load_moh_classes(int reload)
@@ -1748,20 +1739,6 @@ static char *handle_cli_moh_show_classes(struct ast_cli_entry *e, int cmd, struc
 		}
 	}
 	ao2_iterator_destroy(&i);
-	i = ao2_iterator_init(deleted_classes, 0);
-	for (; (class = ao2_t_iterator_next(&i, "Show deleted classes iterator")); mohclass_unref(class, "Unref iterator in moh show classes")) {
-		ast_cli(a->fd, "(Deleted) Class: %s (%d)\n", class->name, ao2_ref(class, 0) - 2);
-		ast_cli(a->fd, "\tMode: %s\n", S_OR(class->mode, "<none>"));
-		ast_cli(a->fd, "\tDirectory: %s\n", S_OR(class->dir, "<none>"));
-		ast_cli(a->fd, "\tRealtime: %s\n", class->realtime ? "yes" : "no");
-		if (ast_test_flag(class, MOH_CUSTOM)) {
-			ast_cli(a->fd, "\tApplication: %s\n", S_OR(class->args, "<none>"));
-		}
-		if (strcasecmp(class->mode, "files")) {
-			ast_cli(a->fd, "\tFormat: %s\n", ast_getformatname(class->format));
-		}
-	}
-	ao2_iterator_destroy(&i);
 
 	return CLI_SUCCESS;
 }
@@ -1796,14 +1773,6 @@ static int load_module(void)
 		return AST_MODULE_LOAD_DECLINE;
 	}
 
-	if (!(deleted_classes = ao2_t_container_alloc(53, moh_class_hash, moh_class_cmp, "Moh deleted class container"))) {
-		return AST_MODULE_LOAD_DECLINE;
-	}
-
-	if (ast_pthread_create_background(&deleted_thread, NULL, deleted_monitor, NULL)) {
-		return AST_MODULE_LOAD_DECLINE;
-	}
-
 	if (!load_moh_classes(0)) { 	/* No music classes configured, so skip it */
 		ast_log(LOG_WARNING, "No music on hold classes configured, "
 				"disabling music on hold.\n");
@@ -1872,10 +1841,6 @@ static int unload_module(void)
 	ast_cli_unregister_multiple(cli_moh, ARRAY_LEN(cli_moh));
 	ast_unregister_atexit(ast_moh_destroy);
 
-	pthread_cancel(deleted_thread);
-	pthread_kill(deleted_thread, SIGURG);
-	pthread_join(deleted_thread, NULL);
-
 	return res;
 }