1 files changed, 173 insertions, 40 deletions

diff --git a/res/res_musiconhold.c b/res/res_musiconhold.c
index d8fc58d4a..cbc91a902 100644
--- a/res/res_musiconhold.c
+++ b/res/res_musiconhold.c
@@ -71,6 +71,8 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 #include "asterisk/astobj2.h"
 
 #define INITIAL_NUM_FILES   8
+#define HANDLE_REF	1
+#define DONT_UNREF	0
 
 static char *play_moh = "MusicOnHold";
 static char *wait_moh = "WaitMusicOnHold";
@@ -124,11 +126,13 @@ static int respawn_time = 20;
 
 struct moh_files_state {
 	struct mohclass *class;
+	char name[MAX_MUSICCLASS];
 	int origwfmt;
 	int samples;
 	int sample_queue;
 	int pos;
 	int save_pos;
+	int save_total;
 	char *save_pos_filename;
 };
 
@@ -140,6 +144,9 @@ struct moh_files_state {
 
 #define MOH_CACHERTCLASSES      (1 << 5)        /*!< Should we use a separate instance of MOH for each user or not */
 
+/* Custom astobj2 flag */
+#define MOH_NOTDELETED          (1 << 30)       /*!< Find only records that aren't deleted? */
+
 static struct ast_flags global_flags[1] = {{0}};        /*!< global MOH_ flags */
 
 struct mohclass {
@@ -181,6 +188,8 @@ 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"
@@ -216,7 +225,7 @@ static void moh_files_release(struct ast_channel *chan, void *data)
 
 	state->save_pos = state->pos;
 
-	mohclass_unref(state->class);
+	state->class = mohclass_unref(state->class);
 }
 
 static int ast_moh_files_next(struct ast_channel *chan) 
@@ -325,7 +334,12 @@ static void *moh_files_alloc(struct ast_channel *chan, void *params)
 		return NULL;
 	}
 
-	if (state->class != class) {
+	/* LOGIC: Comparing an unrefcounted pointer is a really bad idea, because
+	 * malloc may allocate a different class to the same memory block.  This
+	 * might only happen when two reloads are generated in a short period of
+	 * time, but it's still important to protect against.
+	 * PROG: Compare the quick operation first, to save CPU. */
+	if (state->save_total != class->total_files || strcmp(state->name, class->name) != 0) {
 		memset(state, 0, sizeof(*state));
 		if (ast_test_flag(class, MOH_RANDOMIZE) && class->total_files) {
 			state->pos = ast_random() % class->total_files;
@@ -334,6 +348,9 @@ static void *moh_files_alloc(struct ast_channel *chan, void *params)
 
 	state->class = mohclass_ref(class);
 	state->origwfmt = chan->writeformat;
+	/* For comparison on restart of MOH (see above) */
+	ast_copy_string(state->name, class->name, sizeof(state->name));
+	state->save_total = class->total_files;
 
 	ast_verb(3, "Started music on hold, class '%s', on %s\n", class->name, chan->name);
 	
@@ -542,6 +559,43 @@ 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
@@ -602,11 +656,25 @@ static void *monmp3thread(void *data)
 				class->srcfd = -1;
 				pthread_testcancel();
 				if (class->pid > 1) {
-					killpg(class->pid, SIGHUP);
-					usleep(100000);
-					killpg(class->pid, SIGTERM);
-					usleep(100000);
-					killpg(class->pid, SIGKILL);
+					do {
+						if (killpg(class->pid, SIGHUP) < 0) {
+							ast_log(LOG_WARNING, "Unable to send a SIGHUP to MOH process?!!: %s\n", strerror(errno));
+						}
+						usleep(100000);
+						if (killpg(class->pid, SIGTERM) < 0) {
+							if (errno == ESRCH) {
+								break;
+							}
+							ast_log(LOG_WARNING, "Unable to terminate MOH process?!!: %s\n", strerror(errno));
+						}
+						usleep(100000);
+						if (killpg(class->pid, SIGKILL) < 0) {
+							if (errno == ESRCH) {
+								break;
+							}
+							ast_log(LOG_WARNING, "Unable to kill MOH process?!!: %s\n", strerror(errno));
+						}
+					} while (0);
 					class->pid = 0;
 				}
 			} else {
@@ -735,7 +803,9 @@ static int stop_moh_exec(struct ast_channel *chan, void *data)
 	return 0;
 }
 
-static struct mohclass *get_mohbyname(const char *name, int warn)
+#define get_mohbyname(a,b,c)	_get_mohbyname(a,b,c,__FILE__,__LINE__,__PRETTY_FUNCTION__)
+
+static struct mohclass *_get_mohbyname(const char *name, int warn, int flags, const char *file, int lineno, const char *funcname)
 {
 	struct mohclass *moh = NULL;
 	struct mohclass tmp_class = {
@@ -744,7 +814,7 @@ static struct mohclass *get_mohbyname(const char *name, int warn)
 
 	ast_copy_string(tmp_class.name, name, sizeof(tmp_class.name));
 
-	moh = ao2_find(mohclasses, &tmp_class, 0);
+	moh = ao2_find(mohclasses, &tmp_class, flags);
 
 	if (!moh && warn) {
 		ast_log(LOG_DEBUG, "Music on Hold class '%s' not found in memory\n", name);
@@ -1076,27 +1146,28 @@ static int init_app_class(struct mohclass *class)
 }
 
 /*!
- * \note This function owns the reference it gets to moh
+ * \note This function owns the reference it gets to moh if unref is true
  */
-static int moh_register(struct mohclass *moh, int reload, int unref)
+#define moh_register(a,b,c)	_moh_register(a,b,c,__FILE__,__LINE__,__PRETTY_FUNCTION__)
+static int _moh_register(struct mohclass *moh, int reload, int unref, const char *file, int line, const char *funcname)
 {
 	struct mohclass *mohclass = NULL;
 
-	if ((mohclass = get_mohbyname(moh->name, 0)) && !moh_diff(mohclass, moh)) {
-		if (!mohclass->delete) {
- 			ast_log(LOG_WARNING, "Music on Hold class '%s' already exists\n", moh->name);
-			mohclass = mohclass_unref(mohclass);
-			if (unref) {
-				moh = mohclass_unref(moh);
-			}
-			return -1;
+	if ((mohclass = _get_mohbyname(moh->name, 0, MOH_NOTDELETED, file, line, funcname)) && !moh_diff(mohclass, moh)) {
+		ast_log(LOG_WARNING, "Music on Hold class '%s' already exists\n", moh->name);
+		mohclass = mohclass_unref(mohclass);
+		if (unref) {
+			moh = mohclass_unref(moh);
 		}
+		return -1;
+	} else if (mohclass) {
+		/* Found a class, but it's different from the one being registered */
 		mohclass = mohclass_unref(mohclass);
 	}
 
 	time(&moh->start);
 	moh->start -= respawn_time;
-	
+
 	if (!strcasecmp(moh->mode, "files")) {
 		if (init_files_class(moh)) {
 			moh = mohclass_unref(moh);
@@ -1129,6 +1200,9 @@ static void local_ast_moh_cleanup(struct ast_channel *chan)
 	struct moh_files_state *state = chan->music_state;
 
 	if (state) {
+		if (state->class) {
+			state->class = mohclass_unref(state->class);
+		}
 		ast_free(chan->music_state);
 		chan->music_state = NULL;
 	}
@@ -1136,7 +1210,9 @@ static void local_ast_moh_cleanup(struct ast_channel *chan)
 
 static void moh_class_destructor(void *obj);
 
-static struct mohclass *moh_class_malloc(void)
+#define moh_class_malloc()	_moh_class_malloc(__FILE__,__LINE__,__PRETTY_FUNCTION__)
+
+static struct mohclass *_moh_class_malloc(const char *file, int line, const char *funcname)
 {
 	struct mohclass *class;
 
@@ -1167,26 +1243,26 @@ static int local_ast_moh_start(struct ast_channel *chan, const char *mclass, con
 	 * 4) The default class.
 	 */
 	if (!ast_strlen_zero(chan->musicclass)) {
-		mohclass = get_mohbyname(chan->musicclass, 1);
+		mohclass = get_mohbyname(chan->musicclass, 1, 0);
 		if (!mohclass && realtime_possible) {
 			var = ast_load_realtime("musiconhold", "name", chan->musicclass, NULL);
 		}
 	}
 	if (!mohclass && !var && !ast_strlen_zero(mclass)) {
-		mohclass = get_mohbyname(mclass, 1);
+		mohclass = get_mohbyname(mclass, 1, 0);
 		if (!mohclass && realtime_possible) {
 			var = ast_load_realtime("musiconhold", "name", mclass, NULL);
 		}
 	}
 	if (!mohclass && !var && !ast_strlen_zero(interpclass)) {
-		mohclass = get_mohbyname(interpclass, 1);
+		mohclass = get_mohbyname(interpclass, 1, 0);
 		if (!mohclass && realtime_possible) {
 			var = ast_load_realtime("musiconhold", "name", interpclass, NULL);
 		}
 	}
 
 	if (!mohclass && !var) {
-		mohclass = get_mohbyname("default", 1);
+		mohclass = get_mohbyname("default", 1, 0);
 		if (!mohclass && realtime_possible) {
 			var = ast_load_realtime("musiconhold", "name", "default", NULL);
 		}
@@ -1263,7 +1339,7 @@ static int local_ast_moh_start(struct ast_channel *chan, const char *mclass, con
 				 * has a pointer to a freed mohclass, so any operations involving the mohclass container would result in reading
 				 * invalid memory.
 				 */
-				moh_register(mohclass, 0, 0);
+				moh_register(mohclass, 0, DONT_UNREF);
 			} else {
 				/* We don't register RT moh class, so let's init it manualy */
 
@@ -1368,9 +1444,20 @@ static void moh_class_destructor(void *obj)
 {
 	struct mohclass *class = obj;
 	struct mohdata *member;
+	pthread_t tid = 0;
 
 	ast_debug(1, "Destroying MOH class '%s'\n", class->name);
 
+	/* Kill the thread first, so it cannot restart the child process while the
+	 * class is being destroyed */
+	if (class->thread != AST_PTHREADT_NULL && class->thread != 0) {
+		tid = class->thread;
+		class->thread = AST_PTHREADT_NULL;
+		pthread_cancel(tid);
+		/* We'll collect the exit status later, after we ensure all the readers
+		 * are dead. */
+	}
+
 	if (class->pid > 1) {
 		char buff[8192];
 		int bytes, tbytes = 0, stime = 0, pid = 0;
@@ -1384,11 +1471,25 @@ static void moh_class_destructor(void *obj)
 		/* Back when this was just mpg123, SIGKILL was fine.  Now we need
 		 * to give the process a reason and time enough to kill off its
 		 * children. */
-		killpg(pid, SIGHUP);
-		usleep(100000);
-		killpg(pid, SIGTERM);
-		usleep(100000);
-		killpg(pid, SIGKILL);
+		do {
+			if (killpg(pid, SIGHUP) < 0) {
+				ast_log(LOG_WARNING, "Unable to send a SIGHUP to MOH process?!!: %s\n", strerror(errno));
+			}
+			usleep(100000);
+			if (killpg(pid, SIGTERM) < 0) {
+				if (errno == ESRCH) {
+					break;
+				}
+				ast_log(LOG_WARNING, "Unable to terminate MOH process?!!: %s\n", strerror(errno));
+			}
+			usleep(100000);
+			if (killpg(pid, SIGKILL) < 0) {
+				if (errno == ESRCH) {
+					break;
+				}
+				ast_log(LOG_WARNING, "Unable to kill MOH process?!!: %s\n", strerror(errno));
+			}
+		} while (0);
 
 		while ((ast_wait_for_input(class->srcfd, 100) > 0) && 
 				(bytes = read(class->srcfd, buff, 8192)) && time(NULL) < stime) {
@@ -1404,12 +1505,6 @@ static void moh_class_destructor(void *obj)
 		free(member);
 	}
 
-	if (class->thread) {
-		pthread_cancel(class->thread);
-		pthread_join(class->thread, NULL);
-		class->thread = AST_PTHREADT_NULL;
-	}
-
 	if (class->filearray) {
 		int i;
 		for (i = 0; i < class->total_files; i++) {
@@ -1418,6 +1513,11 @@ static void moh_class_destructor(void *obj)
 		free(class->filearray);
 		class->filearray = NULL;
 	}
+
+	/* Finally, collect the exit status of the monitor thread */
+	if (tid > 0) {
+		pthread_join(tid, NULL);
+	}
 }
 
 static int moh_class_mark(void *obj, void *arg, int flags)
@@ -1433,7 +1533,12 @@ static int moh_classes_delete_marked(void *obj, void *arg, int flags)
 {
 	struct mohclass *class = obj;
 
-	return class->delete ? CMP_MATCH : 0;
+	if (class->delete) {
+		ao2_link(deleted_classes, obj);
+		pthread_kill(deleted_thread, SIGURG);
+		return CMP_MATCH;
+	}
+	return 0;
 }
 
 static int load_moh_classes(int reload)
@@ -1524,7 +1629,7 @@ static int load_moh_classes(int reload)
 		}
 
 		/* Don't leak a class when it's already registered */
-		if (!moh_register(class, reload, 1)) {
+		if (!moh_register(class, reload, HANDLE_REF)) {
 			numclasses++;
 		}
 	}
@@ -1635,6 +1740,20 @@ 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_iterator_next(&i)); mohclass_unref(class)) {
+		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;
 }
@@ -1656,7 +1775,9 @@ static int moh_class_cmp(void *obj, void *arg, int flags)
 {
 	struct mohclass *class = obj, *class2 = arg;
 
-	return strcasecmp(class->name, class2->name) ? 0 : CMP_MATCH | CMP_STOP;
+	return strcasecmp(class->name, class2->name) ? 0 :
+		(flags & MOH_NOTDELETED) && (class->delete || class2->delete) ? 0 :
+		CMP_MATCH | CMP_STOP;
 }
 
 static int load_module(void)
@@ -1667,6 +1788,14 @@ static int load_module(void)
 		return AST_MODULE_LOAD_DECLINE;
 	}
 
+	if (!(deleted_classes = ao2_container_alloc(53, moh_class_hash, moh_class_cmp))) {
+		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");
@@ -1734,6 +1863,10 @@ static int unload_module(void)
 	res |= ast_unregister_application(stop_moh);
 	ast_cli_unregister_multiple(cli_moh, sizeof(cli_moh) / sizeof(struct ast_cli_entry));
 
+	pthread_cancel(deleted_thread);
+	pthread_kill(deleted_thread, SIGURG);
+	pthread_join(deleted_thread, NULL);
+
 	return res;
 }