Re-work ref count handling of MoH classes using astobj2 to resolve crashes.

(closes issue #13566)
Reported by: igorcarneiro
Tested by: russell
Review: http://reviewboard.digium.com/r/106/


git-svn-id: http://svn.digium.com/svn/asterisk/trunk@166273 f38db490-d61c-443f-a65b-d21fe96a405b

1 files changed, 419 insertions, 371 deletions

diff --git a/res/res_musiconhold.c b/res/res_musiconhold.c
index d2a6f9f7e..64db250c5 100644
--- a/res/res_musiconhold.c
+++ b/res/res_musiconhold.c
@@ -46,7 +46,7 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 #include <thread.h>
 #endif
 
-#if defined(HAVE_DAHDI)
+#ifdef HAVE_DAHDI
 #include <dahdi/user.h>
 #endif
 
@@ -66,6 +66,7 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 #include "asterisk/linkedlists.h"
 #include "asterisk/manager.h"
 #include "asterisk/paths.h"
+#include "asterisk/astobj2.h"
 
 #define INITIAL_NUM_FILES   8
 
@@ -162,8 +163,6 @@ struct mohclass {
 	int srcfd;
 	/*! FD for timing source */
 	int pseudofd;
-	/*! Number of users */
-	int inuse;
 	/*! Created on the fly, from RT engine */
 	int realtime;
 	unsigned int delete:1;
@@ -179,63 +178,44 @@ struct mohdata {
 	AST_LIST_ENTRY(mohdata) list;
 };
 
-AST_RWLIST_HEAD_STATIC(mohclasses, mohclass);
+static struct ao2_container *mohclasses;
 
 #define LOCAL_MPG_123 "/usr/local/bin/mpg123"
 #define MPG_123 "/usr/bin/mpg123"
 #define MAX_MP3S 256
 
-static int ast_moh_destroy_one(struct mohclass *moh);
 static int reload(void);
 
-static void ast_moh_free_class(struct mohclass **mohclass) 
-{
-	struct mohdata *member;
-	struct mohclass *class = *mohclass;
-	int i;
-	
-	while ((member = AST_LIST_REMOVE_HEAD(&class->members, list)))
-		ast_free(member);
-	
-	if (class->thread) {
-		pthread_cancel(class->thread);
-		class->thread = 0;
-	}
-
-	if (class->filearray) {
-		for (i = 0; i < class->total_files; i++)
-			ast_free(class->filearray[i]);
-		ast_free(class->filearray);
-	}
-
-	ast_free(class);
-	*mohclass = NULL;
-}
-
+#define mohclass_ref(class)   (ao2_ref((class), +1), class)
+#define mohclass_unref(class) (ao2_ref((class), -1), NULL)
 
 static void moh_files_release(struct ast_channel *chan, void *data)
 {
 	struct moh_files_state *state;
 
-	if (chan) {
-		if ((state = chan->music_state)) {
-			if (chan->stream) {
-	                        ast_closestream(chan->stream);
-	                        chan->stream = NULL;
-	                }
-			ast_verb(3, "Stopped music on hold on %s\n", chan->name);
+	if (!chan || !chan->music_state) {
+		return;
+	}
+
+	state = chan->music_state;
+
+	if (chan->stream) {
+		ast_closestream(chan->stream);
+		chan->stream = NULL;
+	}
 	
-			if (state->origwfmt && ast_set_write_format(chan, state->origwfmt)) {
-				ast_log(LOG_WARNING, "Unable to restore channel '%s' to format '%d'\n", chan->name, state->origwfmt);
-			}
-			state->save_pos = state->pos;
+	if (option_verbose > 2) {
+		ast_verbose(VERBOSE_PREFIX_3 "Stopped music on hold on %s\n", chan->name);
+	}
 
-			if (ast_atomic_dec_and_test(&state->class->inuse) && state->class->delete)
-				ast_moh_destroy_one(state->class);
-		}
+	if (state->origwfmt && ast_set_write_format(chan, state->origwfmt)) {
+		ast_log(LOG_WARNING, "Unable to restore channel '%s' to format '%d'\n", chan->name, state->origwfmt);
 	}
-}
 
+	state->save_pos = state->pos;
+
+	state->class = mohclass_unref(state->class);
+}
 
 static int ast_moh_files_next(struct ast_channel *chan) 
 {
@@ -292,7 +272,6 @@ static int ast_moh_files_next(struct ast_channel *chan)
 	return 0;
 }
 
-
 static struct ast_frame *moh_files_readframe(struct ast_channel *chan) 
 {
 	struct ast_frame *f = NULL;
@@ -329,7 +308,6 @@ static int moh_files_generator(struct ast_channel *chan, void *data, int len, in
 	return res;
 }
 
-
 static void *moh_files_alloc(struct ast_channel *chan, void *params)
 {
 	struct moh_files_state *state;
@@ -337,53 +315,62 @@ 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;
-		state->class = class;
+		state->class = mohclass_ref(class);
 		state->save_pos = -1;
-	} else 
+	} else {
 		state = chan->music_state;
+	}
 
-	if (state) {
-		if (state->class != class) {
-			/* initialize */
-			memset(state, 0, sizeof(*state));
-			state->class = class;
-			if (ast_test_flag(state->class, MOH_RANDOMIZE) && class->total_files)
-				state->pos = ast_random() % class->total_files;
+	if (!state) {
+		return NULL;
+	}
+
+	if (state->class != class) {
+		/* (re-)initialize */
+		if (state->class) {
+			state->class = mohclass_unref(state->class);
 		}
+		memset(state, 0, sizeof(*state));
+		state->class = mohclass_ref(class);
+		if (ast_test_flag(state->class, MOH_RANDOMIZE) && class->total_files) {
+			state->pos = ast_random() % class->total_files;
+		}
+	}
 
-		state->origwfmt = chan->writeformat;
+	state->origwfmt = chan->writeformat;
 
-		ast_verb(3, "Started music on hold, class '%s', on %s\n", class->name, chan->name);
-	}
+	ast_verb(3, "Started music on hold, class '%s', on %s\n", class->name, chan->name);
 	
 	return chan->music_state;
 }
 
-/*! \note This function should be called with the mohclasses list locked */
-static struct mohclass *get_mohbydigit(char digit)
+static int moh_digit_match(void *obj, void *arg, int flags)
 {
-	struct mohclass *moh = NULL;
+	char *digit = arg;
+	struct mohclass *class = obj;
 
-	AST_RWLIST_TRAVERSE(&mohclasses, moh, list) {
-		if (digit == moh->digit)
-			break;
-	}
+	return (*digit == class->digit) ? CMP_MATCH | CMP_STOP : 0;
+}
 
-	return moh;
+/*! \note This function should be called with the mohclasses list locked */
+static struct mohclass *get_mohbydigit(char digit)
+{
+	return ao2_callback(mohclasses, 0, moh_digit_match, &digit);
 }
 
 static void moh_handle_digit(struct ast_channel *chan, char digit)
 {
-	struct mohclass *moh;
+	struct mohclass *class;
 	const char *classname = NULL;
 
-	AST_RWLIST_RDLOCK(&mohclasses);
-	if ((moh = get_mohbydigit(digit)))
-		classname = ast_strdupa(moh->name);
-	AST_RWLIST_UNLOCK(&mohclasses);
+	if ((class = get_mohbydigit(digit))) {
+		classname = ast_strdupa(class->name);
+		class = mohclass_unref(class);
+	}
 
-	if (!moh)
+	if (!class) {
 		return;
+	}
 
 	ast_moh_stop(chan);
 	ast_moh_start(chan, classname, NULL);
@@ -391,10 +378,10 @@ static void moh_handle_digit(struct ast_channel *chan, char digit)
 
 static struct ast_generator moh_file_stream = 
 {
-	alloc: moh_files_alloc,
-	release: moh_files_release,
-	generate: moh_files_generator,
-	digit: moh_handle_digit,
+	.alloc    = moh_files_alloc,
+	.release  = moh_files_release,
+	.generate = moh_files_generator,
+	.digit    = moh_handle_digit,
 };
 
 static int spawn_mp3(struct mohclass *class)
@@ -610,15 +597,17 @@ static void *monmp3thread(void *data)
 			}
 			continue;
 		}
+
 		pthread_testcancel();
-		AST_RWLIST_RDLOCK(&mohclasses);
-		AST_RWLIST_TRAVERSE(&class->members, moh, list) {
+
+		ao2_lock(class);
+		AST_LIST_TRAVERSE(&class->members, moh, list) {
 			/* Write data */
 			if ((res = write(moh->pipe[1], sbuf, res2)) != res2) {
 				ast_debug(1, "Only wrote %d of %d bytes to pipe\n", res, res2);
 			}
 		}
-		AST_RWLIST_UNLOCK(&mohclasses);
+		ao2_unlock(class);
 	}
 	return NULL;
 }
@@ -729,18 +718,20 @@ static int stop_moh_exec(struct ast_channel *chan, void *data)
 	return 0;
 }
 
-/*! \note This function should be called with the mohclasses list locked */
 static struct mohclass *get_mohbyname(const char *name, int warn)
 {
 	struct mohclass *moh = NULL;
+	struct mohclass tmp_class = {
+		.flags = 0,
+	};
 
-	AST_RWLIST_TRAVERSE(&mohclasses, moh, list) {
-		if (!strcasecmp(name, moh->name))
-			break;
-	}
+	ast_copy_string(tmp_class.name, name, sizeof(tmp_class.name));
 
-	if (!moh && warn)
+	moh = ao2_find(mohclasses, &tmp_class, 0);
+
+	if (!moh && warn) {
 		ast_log(LOG_DEBUG, "Music on Hold class '%s' not found in memory\n", name);
+	}
 
 	return moh;
 }
@@ -769,11 +760,11 @@ static struct mohdata *mohalloc(struct mohclass *cl)
 	moh->f.subclass = cl->format;
 	moh->f.offset = AST_FRIENDLY_OFFSET;
 
-	moh->parent = cl;
+	moh->parent = mohclass_ref(cl);
 
-	AST_RWLIST_WRLOCK(&mohclasses);
+	ao2_lock(cl);
 	AST_LIST_INSERT_HEAD(&cl->members, moh, list);
-	AST_RWLIST_UNLOCK(&mohclasses);
+	ao2_unlock(cl);
 	
 	return moh;
 }
@@ -781,26 +772,28 @@ static struct mohdata *mohalloc(struct mohclass *cl)
 static void moh_release(struct ast_channel *chan, void *data)
 {
 	struct mohdata *moh = data;
+	struct mohclass *class = moh->parent;
 	int oldwfmt;
-	struct moh_files_state *state;
 
-	AST_RWLIST_WRLOCK(&mohclasses);
-	AST_RWLIST_REMOVE(&moh->parent->members, moh, list);	
-	AST_RWLIST_UNLOCK(&mohclasses);
+	ao2_lock(class);
+	AST_LIST_REMOVE(&moh->parent->members, moh, list);	
+	ao2_unlock(class);
 	
 	close(moh->pipe[0]);
 	close(moh->pipe[1]);
+
 	oldwfmt = moh->origwfmt;
-	state = chan->music_state;
-	if (moh->parent->delete && ast_atomic_dec_and_test(&moh->parent->inuse))
-		ast_moh_destroy_one(moh->parent);
-	if (ast_atomic_dec_and_test(&state->class->inuse) && state->class->delete)
-		ast_moh_destroy_one(state->class);
+
+	moh->parent = class = mohclass_unref(class);
 
 	ast_free(moh);
+
 	if (chan) {
-		if (oldwfmt && ast_set_write_format(chan, oldwfmt)) 
-			ast_log(LOG_WARNING, "Unable to restore channel '%s' to format %s\n", chan->name, ast_getformatname(oldwfmt));
+		if (oldwfmt && ast_set_write_format(chan, oldwfmt)) {
+			ast_log(LOG_WARNING, "Unable to restore channel '%s' to format %s\n",
+					chan->name, ast_getformatname(oldwfmt));
+		}
+
 		ast_verb(3, "Stopped music on hold on %s\n", chan->name);
 	}
 }
@@ -840,10 +833,6 @@ static int moh_generate(struct ast_channel *chan, void *data, int len, int sampl
 	short buf[1280 + AST_FRIENDLY_OFFSET / 2];
 	int res;
 
-	if (!moh->parent->pid && moh->parent->inuse == 0) {
-		return -1;
-	}
-
 	len = ast_codec_get_len(moh->parent->format, samples);
 
 	if (len > sizeof(buf) - AST_FRIENDLY_OFFSET) {
@@ -866,11 +855,10 @@ static int moh_generate(struct ast_channel *chan, void *data, int len, int sampl
 	return 0;
 }
 
-static struct ast_generator mohgen = 
-{
-	alloc: moh_alloc,
-	release: moh_release,
-	generate: moh_generate,
+static struct ast_generator mohgen = {
+	.alloc    = moh_alloc,
+	.release  = moh_release,
+	.generate = moh_generate,
 	digit: moh_handle_digit
 };
 
@@ -991,98 +979,135 @@ static int moh_scan_files(struct mohclass *class) {
 	return class->total_files;
 }
 
+static int init_files_class(struct mohclass *class)
+{
+	int res;
+
+	res = moh_scan_files(class);
+
+	if (res < 0) {
+		return -1;
+	}
+
+	if (!res) {
+		if (option_verbose > 2) {
+			ast_verbose(VERBOSE_PREFIX_3 "Files not found in %s for moh class:%s\n",
+					class->dir, class->name);
+		}
+		return -1;
+	}
+
+	if (strchr(class->args, 'r')) {
+		ast_set_flag(class, MOH_RANDOMIZE);
+	}
+
+	return 0;
+}
+
+
 static int moh_diff(struct mohclass *old, struct mohclass *new)
 {
-	if (!old || !new)
+	if (!old || !new) {
 		return -1;
+	}
 
-	if (strcmp(old->dir, new->dir))
+	if (strcmp(old->dir, new->dir)) {
 		return -1;
-	else if (strcmp(old->mode, new->mode))
+	} else if (strcmp(old->mode, new->mode)) {
 		return -1;
-	else if (strcmp(old->args, new->args))
+	} else if (strcmp(old->args, new->args)) {
 		return -1;
-	else if (old->flags != new->flags)
+	} else if (old->flags != new->flags) {
 		return -1;
+	}
 
 	return 0;
 }
 
-static int moh_register(struct mohclass *moh, int is_reload)
+static int init_app_class(struct mohclass *class)
 {
 #ifdef HAVE_DAHDI
 	int x;
 #endif
+
+	if (!strcasecmp(class->mode, "custom")) {
+		ast_set_flag(class, MOH_CUSTOM);
+	} else if (!strcasecmp(class->mode, "mp3nb")) {
+		ast_set_flag(class, MOH_SINGLE);
+	} else if (!strcasecmp(class->mode, "quietmp3nb")) {
+		ast_set_flag(class, MOH_SINGLE | MOH_QUIET);
+	} else if (!strcasecmp(class->mode, "quietmp3")) {
+		ast_set_flag(class, MOH_QUIET);
+	}
+		
+	class->srcfd = -1;
+	class->pseudofd = -1;
+
+#ifdef HAVE_DAHDI
+	/* Open /dev/zap/pseudo for timing...  Is
+	   there a better, yet reliable way to do this? */
+	class->pseudofd = open("/dev/dahdi/psuedo", O_RDONLY);
+	if (class->pseudofd < 0) {
+		ast_log(LOG_WARNING, "Unable to open pseudo channel for timing...  Sound may be choppy.\n");
+	} else {
+		x = 320;
+		ioctl(class->pseudofd, DAHDI_SET_BLOCKSIZE, &x);
+	}
+#endif
+
+	if (ast_pthread_create_background(&class->thread, NULL, monmp3thread, class)) {
+		ast_log(LOG_WARNING, "Unable to create moh thread...\n");
+		if (class->pseudofd > -1) {
+			close(class->pseudofd);
+			class->pseudofd = -1;
+		}
+		return -1;
+	}
+
+	return 0;
+}
+
+/*!
+ * \note This function owns the reference it gets to moh
+ */
+static int moh_register(struct mohclass *moh, int reload)
+{
 	struct mohclass *mohclass = NULL;
-	int res = 0;
 
-	AST_RWLIST_WRLOCK(&mohclasses);
 	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);
-			ast_free(moh);
-			AST_RWLIST_UNLOCK(&mohclasses);
+ 			ast_log(LOG_WARNING, "Music on Hold class '%s' already exists\n", moh->name);
+			mohclass = mohclass_unref(mohclass);
+			moh = mohclass_unref(moh);
 			return -1;
 		}
+		mohclass = mohclass_unref(mohclass);
 	}
-	AST_RWLIST_UNLOCK(&mohclasses);
 
 	time(&moh->start);
 	moh->start -= respawn_time;
 	
 	if (!strcasecmp(moh->mode, "files")) {
-		res = moh_scan_files(moh);
-		if (res <= 0) {
-			if (res == 0) {
-				if (option_verbose > 2)
-					ast_verbose(VERBOSE_PREFIX_3 "Files not found in %s for moh class:%s\n", moh->dir, moh->name);
-			}
-			ast_moh_free_class(&moh);
+		if (init_files_class(moh)) {
+			moh = mohclass_unref(moh);
 			return -1;
 		}
-		if (strchr(moh->args, 'r'))
-			ast_set_flag(moh, MOH_RANDOMIZE);
-	} 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 (!strcasecmp(moh->mode, "custom"))
-			ast_set_flag(moh, MOH_CUSTOM);
-		else if (!strcasecmp(moh->mode, "mp3nb"))
-			ast_set_flag(moh, MOH_SINGLE);
-		else if (!strcasecmp(moh->mode, "quietmp3nb"))
-			ast_set_flag(moh, MOH_SINGLE | MOH_QUIET);
-		else if (!strcasecmp(moh->mode, "quietmp3"))
-			ast_set_flag(moh, MOH_QUIET);
-		
-		moh->srcfd = -1;
-#ifdef HAVE_DAHDI
-		/* Open /dev/dahdi/pseudo for timing...  Is
-		   there a better, yet reliable way to do this? */
-		moh->pseudofd = open("/dev/dahdi/pseudo", O_RDONLY);
-		if (moh->pseudofd < 0) {
-			ast_log(LOG_WARNING, "Unable to open pseudo channel for timing...  Sound may be choppy.\n");
-		} else {
-			x = 320;
-			ioctl(moh->pseudofd, DAHDI_SET_BLOCKSIZE, &x);
-		}
-#else
-		moh->pseudofd = -1;
-#endif
-		if (ast_pthread_create_background(&moh->thread, NULL, monmp3thread, moh)) {
-			ast_log(LOG_WARNING, "Unable to create moh...\n");
-			if (moh->pseudofd > -1)
-				close(moh->pseudofd);
-			ast_moh_free_class(&moh);
+	} 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);
 			return -1;
 		}
 	} else {
 		ast_log(LOG_WARNING, "Don't know how to do a mode '%s' music on hold\n", moh->mode);
-		ast_moh_free_class(&moh);
+		moh = mohclass_unref(moh);
 		return -1;
 	}
 
-	AST_RWLIST_WRLOCK(&mohclasses);
-	AST_RWLIST_INSERT_HEAD(&mohclasses, moh, list);
-	AST_RWLIST_UNLOCK(&mohclasses);
+	ao2_link(mohclasses, moh);
+
+	moh = mohclass_unref(moh);
 	
 	return 0;
 }
@@ -1092,34 +1117,19 @@ static void local_ast_moh_cleanup(struct ast_channel *chan)
 	struct moh_files_state *state = chan->music_state;
 
 	if (state) {
-		if (state->class->realtime) {
-			if (ast_test_flag(global_flags, MOH_CACHERTCLASSES)) {
-				/* We are cleaning out cached RT class, we should remove it from list, if no one else using it */
-				if (!(state->class->inuse)) {
-					/* Remove this class from list */
-					AST_RWLIST_WRLOCK(&mohclasses);
-					AST_RWLIST_REMOVE(&mohclasses, state->class, list);
-					AST_RWLIST_UNLOCK(&mohclasses);
-	
-					/* Free some memory */
-					ast_moh_destroy_one(state->class);
-				}
-			} else {
-				ast_moh_destroy_one(state->class);
-			}
-		}
 		ast_free(chan->music_state);
 		chan->music_state = NULL;
 	}
 }
 
+static void moh_class_destructor(void *obj);
+
 static struct mohclass *moh_class_malloc(void)
 {
 	struct mohclass *class;
 
-	if ((class = ast_calloc(1, sizeof(*class)))) {
+	if ((class = ao2_alloc(sizeof(*class), moh_class_destructor))) {
 		class->format = AST_FORMAT_SLINEAR;
-		class->realtime = 0;
 	}
 
 	return class;
@@ -1128,12 +1138,8 @@ static struct mohclass *moh_class_malloc(void)
 static int local_ast_moh_start(struct ast_channel *chan, const char *mclass, const char *interpclass)
 {
 	struct mohclass *mohclass = NULL;
-	struct ast_variable *var = NULL;
-	struct ast_variable *tmp = NULL;
 	struct moh_files_state *state = chan->music_state;
-#ifdef HAVE_DAHDI
-	int x;
-#endif
+	int res;
 
 	/* The following is the order of preference for which class to use:
 	 * 1) The channels explicitly set musicclass, which should *only* be
@@ -1146,19 +1152,20 @@ static int local_ast_moh_start(struct ast_channel *chan, const char *mclass, con
 	 *    option.
 	 * 4) The default class.
 	 */
-	
-	/* First, let's check in memory for static and cached RT classes */
-	AST_RWLIST_RDLOCK(&mohclasses);
-	if (!ast_strlen_zero(chan->musicclass))
+	if (!ast_strlen_zero(chan->musicclass)) {
 		mohclass = get_mohbyname(chan->musicclass, 1);
-	if (!mohclass && !ast_strlen_zero(mclass))
+	}
+	if (!mohclass && !ast_strlen_zero(mclass)) {
 		mohclass = get_mohbyname(mclass, 1);
-	if (!mohclass && !ast_strlen_zero(interpclass))
+	}
+	if (!mohclass && !ast_strlen_zero(interpclass)) {
 		mohclass = get_mohbyname(interpclass, 1);
-	AST_RWLIST_UNLOCK(&mohclasses);
+	}
 
 	/* If no moh class found in memory, then check RT */
 	if (!mohclass && ast_check_realtime("musiconhold")) {
+		struct ast_variable *var = NULL, *tmp = NULL;
+
 		if (!ast_strlen_zero(chan->musicclass)) {
 			var = ast_load_realtime("musiconhold", "name", chan->musicclass, SENTINEL);
 		}
@@ -1201,18 +1208,18 @@ static int local_ast_moh_start(struct ast_channel *chan, const char *mclass, con
 					strcpy(mohclass->dir, "nodir");
 				} else {
 					ast_log(LOG_WARNING, "A directory must be specified for class '%s'!\n", mohclass->name);
-					ast_free(mohclass);
+					mohclass = mohclass_unref(mohclass);
 					return -1;
 				}
 			}
 			if (ast_strlen_zero(mohclass->mode)) {
 				ast_log(LOG_WARNING, "A mode must be specified for class '%s'!\n", mohclass->name);
-				ast_free(mohclass);
+				mohclass = mohclass_unref(mohclass);
 				return -1;
 			}
 			if (ast_strlen_zero(mohclass->args) && !strcasecmp(mohclass->mode, "custom")) {
 				ast_log(LOG_WARNING, "An application must be specified for class '%s'!\n", mohclass->name);
-				ast_free(mohclass);
+				mohclass = mohclass_unref(mohclass);
 				return -1;
 			}
 
@@ -1223,13 +1230,12 @@ static int local_ast_moh_start(struct ast_channel *chan, const char *mclass, con
 					ast_log(LOG_NOTICE, "This channel already has a MOH class attached (%s)!\n", state->class->name);
 					if (state->class->realtime && !ast_test_flag(global_flags, MOH_CACHERTCLASSES) && !strcasecmp(mohclass->name, state->class->name)) {
 						/* we found RT class with the same name, seems like we should continue playing existing one */
-						ast_moh_free_class(&mohclass);
+						mohclass = mohclass_unref(mohclass);
 						mohclass = state->class;
 					}
 				}
 				moh_register(mohclass, 0);
 			} else {
-
 				/* We don't register RT moh class, so let's init it manualy */
 
 				time(&mohclass->start);
@@ -1237,7 +1243,7 @@ static int local_ast_moh_start(struct ast_channel *chan, const char *mclass, con
 	
 				if (!strcasecmp(mohclass->mode, "files")) {
 					if (!moh_scan_files(mohclass)) {
-						ast_moh_free_class(&mohclass);
+						mohclass = mohclass_unref(mohclass);
 						return -1;
 					}
 					if (strchr(mohclass->args, 'r'))
@@ -1261,7 +1267,7 @@ static int local_ast_moh_start(struct ast_channel *chan, const char *mclass, con
 					if (mohclass->pseudofd < 0) {
 						ast_log(LOG_WARNING, "Unable to open pseudo channel for timing...  Sound may be choppy.\n");
 					} else {
-						x = 320;
+						int x = 320;
 						ioctl(mohclass->pseudofd, DAHDI_SET_BLOCKSIZE, &x);
 					}
 #else
@@ -1273,48 +1279,38 @@ static int local_ast_moh_start(struct ast_channel *chan, const char *mclass, con
 						ast_log(LOG_NOTICE, "This channel already has a MOH class attached (%s)!\n", state->class->name);
 						if (state->class->realtime && !ast_test_flag(global_flags, MOH_CACHERTCLASSES) && !strcasecmp(mohclass->name, state->class->name)) {
 							/* we found RT class with the same name, seems like we should continue playing existing one */
-							ast_moh_free_class(&mohclass);
+							mohclass = mohclass_unref(mohclass);
 							mohclass = state->class;
-	
 						}
 					} else {
 						if (ast_pthread_create_background(&mohclass->thread, NULL, monmp3thread, mohclass)) {
 							ast_log(LOG_WARNING, "Unable to create moh...\n");
-							if (mohclass->pseudofd > -1)
+							if (mohclass->pseudofd > -1) {
 								close(mohclass->pseudofd);
-							ast_moh_free_class(&mohclass);
+								mohclass->pseudofd = -1;
+							}
+							mohclass = mohclass_unref(mohclass);
 							return -1;
 						}
 					}
 				} else {
 					ast_log(LOG_WARNING, "Don't know how to do a mode '%s' music on hold\n", mohclass->mode);
-					ast_moh_free_class(&mohclass);
+					mohclass = mohclass_unref(mohclass);
 					return -1;
 				}
-
 			}
-
-		} else if (var)
+		} else if (var) {
 			ast_variables_destroy(var);
+		}
 	}
 
-	
-
-	/* Requested MOH class not found, check for 'default' class in musiconhold.conf  */
 	if (!mohclass) {
-		AST_RWLIST_RDLOCK(&mohclasses);
 		mohclass = get_mohbyname("default", 1);
-		if (mohclass)
-			ast_atomic_fetchadd_int(&mohclass->inuse, +1);
-		AST_RWLIST_UNLOCK(&mohclasses);
-	} else {
-		AST_RWLIST_RDLOCK(&mohclasses);
-		ast_atomic_fetchadd_int(&mohclass->inuse, +1);
-		AST_RWLIST_UNLOCK(&mohclasses);
 	}
 
-	if (!mohclass)
+	if (!mohclass) {
 		return -1;
+	}
 
 	manager_event(EVENT_FLAG_CALL, "MusicOnHold",
 		"State: Start\r\n"
@@ -1323,10 +1319,16 @@ static int local_ast_moh_start(struct ast_channel *chan, const char *mclass, con
 		chan->name, chan->uniqueid);
 
 	ast_set_flag(chan, AST_FLAG_MOH);
+
 	if (mohclass->total_files) {
-		return ast_activate_generator(chan, &moh_file_stream, mohclass);
-	} else
-		return ast_activate_generator(chan, &mohgen, mohclass);
+		res = ast_activate_generator(chan, &moh_file_stream, mohclass);
+	} else {
+		res = ast_activate_generator(chan, &mohgen, mohclass);
+	}
+
+	mohclass = mohclass_unref(mohclass);
+
+	return res;
 }
 
 static void local_ast_moh_stop(struct ast_channel *chan)
@@ -1349,14 +1351,85 @@ static void local_ast_moh_stop(struct ast_channel *chan)
 		chan->name, chan->uniqueid);
 }
 
-static int load_moh_classes(int is_reload)
+static void moh_class_destructor(void *obj)
+{
+	struct mohclass *class = obj;
+	struct mohdata *member;
+
+	ast_debug(1, "Destroying MOH class '%s'\n", class->name);
+
+	if (class->pid > 1) {
+		char buff[8192];
+		int bytes, tbytes = 0, stime = 0, pid = 0;
+
+		ast_log(LOG_DEBUG, "killing %d!\n", class->pid);
+
+		stime = time(NULL) + 2;
+		pid = class->pid;
+		class->pid = 0;
+
+		/* 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);
+
+		while ((ast_wait_for_input(class->srcfd, 100) > 0) && 
+				(bytes = read(class->srcfd, buff, 8192)) && time(NULL) < stime) {
+			tbytes = tbytes + bytes;
+		}
+
+		ast_log(LOG_DEBUG, "mpg123 pid %d and child died after %d bytes read\n", pid, tbytes);
+
+		close(class->srcfd);
+	}
+
+	while ((member = AST_LIST_REMOVE_HEAD(&class->members, list))) {
+		free(member);
+	}
+	
+	if (class->thread) {
+		pthread_cancel(class->thread);
+		class->thread = AST_PTHREADT_NULL;
+	}
+
+	if (class->filearray) {
+		int i;
+		for (i = 0; i < class->total_files; i++) {
+			free(class->filearray[i]);
+		}
+		free(class->filearray);
+		class->filearray = NULL;
+	}
+}
+
+static int moh_class_mark(void *obj, void *arg, int flags)
+{
+	struct mohclass *class = obj;
+
+	class->delete = 1;
+
+	return 0;
+}
+
+static int moh_classes_delete_marked(void *obj, void *arg, int flags)
+{
+	struct mohclass *class = obj;
+
+	return class->delete ? CMP_MATCH : 0;
+}
+
+static int load_moh_classes(int reload)
 {
 	struct ast_config *cfg;
 	struct ast_variable *var;
 	struct mohclass *class;	
 	char *cat;
 	int numclasses = 0;
-	struct ast_flags config_flags = { is_reload ? CONFIG_FLAG_FILEUNCHANGED : 0 };
+	struct ast_flags config_flags = { reload ? CONFIG_FLAG_FILEUNCHANGED : 0 };
 
 	cfg = ast_config_load("musiconhold.conf", config_flags);
 
@@ -1364,12 +1437,8 @@ static int load_moh_classes(int is_reload)
 		return 0;
 	}
 
-	if (is_reload) {
-		AST_RWLIST_WRLOCK(&mohclasses);
-		AST_RWLIST_TRAVERSE(&mohclasses, class, list) {
-			class->delete = 1;
-		}
-		AST_RWLIST_UNLOCK(&mohclasses);
+	if (reload) {
+		ao2_callback(mohclasses, OBJ_NODATA, moh_class_mark, NULL);
 	}
 	
 	ast_clear_flag(global_flags, AST_FLAGS_ALL);
@@ -1378,121 +1447,87 @@ static int load_moh_classes(int is_reload)
 	for (; cat; cat = ast_category_browse(cfg, cat)) {
 		/* Setup common options from [general] section */
 		if (!strcasecmp(cat, "general")) {
-			var = ast_variable_browse(cfg, cat);
-			while (var) {
+			for (var = ast_variable_browse(cfg, cat); var; var = var->next) {
 				if (!strcasecmp(var->name, "cachertclasses")) {
 					ast_set2_flag(global_flags, ast_true(var->value), MOH_CACHERTCLASSES);
 				} else {
 					ast_log(LOG_WARNING, "Unknown option '%s' in [general] section of musiconhold.conf\n", var->name);
 				}
-				var = var->next;
 			}
 		}
 		/* These names were deprecated in 1.4 and should not be used until after the next major release. */
-		if (strcasecmp(cat, "classes") && strcasecmp(cat, "moh_files") && strcasecmp(cat, "general")) {
-			if (!(class = moh_class_malloc()))
-				break;
+		if (!strcasecmp(cat, "classes") || !strcasecmp(cat, "moh_files") || 
+				!strcasecmp(cat, "general")) {
+			continue;
+		}
 
-			ast_copy_string(class->name, cat, sizeof(class->name));	
-			var = ast_variable_browse(cfg, cat);
-			while (var) {
-				if (!strcasecmp(var->name, "mode"))
-					ast_copy_string(class->mode, var->value, sizeof(class->mode)); 
-				else if (!strcasecmp(var->name, "directory"))
-					ast_copy_string(class->dir, var->value, sizeof(class->dir));
-				else if (!strcasecmp(var->name, "application"))
-					ast_copy_string(class->args, var->value, sizeof(class->args));
-				else if (!strcasecmp(var->name, "digit") && (isdigit(*var->value) || strchr("*#", *var->value)))
-					class->digit = *var->value;
-				else if (!strcasecmp(var->name, "random"))
-					ast_set2_flag(class, ast_true(var->value), MOH_RANDOMIZE);
-				else if (!strcasecmp(var->name, "sort") && !strcasecmp(var->value, "random"))
-					ast_set_flag(class, MOH_RANDOMIZE);
-				else if (!strcasecmp(var->name, "sort") && !strcasecmp(var->value, "alpha")) 
-					ast_set_flag(class, MOH_SORTALPHA);
-				else if (!strcasecmp(var->name, "format")) {
-					class->format = ast_getformatbyname(var->value);
-					if (!class->format) {
-						ast_log(LOG_WARNING, "Unknown format '%s' -- defaulting to SLIN\n", var->value);
-						class->format = AST_FORMAT_SLINEAR;
-					}
-				}
-				var = var->next;
-			}
+		if (!(class = moh_class_malloc())) {
+			break;
+		}
 
-			if (ast_strlen_zero(class->dir)) {
-				if (!strcasecmp(class->mode, "custom")) {
-					strcpy(class->dir, "nodir");
-				} else {
-					ast_log(LOG_WARNING, "A directory must be specified for class '%s'!\n", class->name);
-					ast_free(class);
-					continue;
+		ast_copy_string(class->name, cat, sizeof(class->name));	
+		for (var = ast_variable_browse(cfg, cat); var; var = var->next) {
+			if (!strcasecmp(var->name, "mode"))
+				ast_copy_string(class->mode, var->value, sizeof(class->mode)); 
+			else if (!strcasecmp(var->name, "directory"))
+				ast_copy_string(class->dir, var->value, sizeof(class->dir));
+			else if (!strcasecmp(var->name, "application"))
+				ast_copy_string(class->args, var->value, sizeof(class->args));
+			else if (!strcasecmp(var->name, "digit") && (isdigit(*var->value) || strchr("*#", *var->value)))
+				class->digit = *var->value;
+			else if (!strcasecmp(var->name, "random"))
+				ast_set2_flag(class, ast_true(var->value), MOH_RANDOMIZE);
+			else if (!strcasecmp(var->name, "sort") && !strcasecmp(var->value, "random"))
+				ast_set_flag(class, MOH_RANDOMIZE);
+			else if (!strcasecmp(var->name, "sort") && !strcasecmp(var->value, "alpha")) 
+				ast_set_flag(class, MOH_SORTALPHA);
+			else if (!strcasecmp(var->name, "format")) {
+				class->format = ast_getformatbyname(var->value);
+				if (!class->format) {
+					ast_log(LOG_WARNING, "Unknown format '%s' -- defaulting to SLIN\n", var->value);
+					class->format = AST_FORMAT_SLINEAR;
 				}
 			}
-			if (ast_strlen_zero(class->mode)) {
-				ast_log(LOG_WARNING, "A mode must be specified for class '%s'!\n", class->name);
-				ast_free(class);
-				continue;
-			}
-			if (ast_strlen_zero(class->args) && !strcasecmp(class->mode, "custom")) {
-				ast_log(LOG_WARNING, "An application must be specified for class '%s'!\n", class->name);
-				ast_free(class);
+		}
+
+		if (ast_strlen_zero(class->dir)) {
+			if (!strcasecmp(class->mode, "custom")) {
+				strcpy(class->dir, "nodir");
+			} else {
+				ast_log(LOG_WARNING, "A directory must be specified for class '%s'!\n", class->name);
+				class = mohclass_unref(class);
 				continue;
 			}
+		}
+		if (ast_strlen_zero(class->mode)) {
+			ast_log(LOG_WARNING, "A mode must be specified for class '%s'!\n", class->name);
+			class = mohclass_unref(class);
+			continue;
+		}
+		if (ast_strlen_zero(class->args) && !strcasecmp(class->mode, "custom")) {
+			ast_log(LOG_WARNING, "An application must be specified for class '%s'!\n", class->name);
+			class = mohclass_unref(class);
+			continue;
+		}
 
-			/* Don't leak a class when it's already registered */
-			moh_register(class, is_reload);
+		/* Don't leak a class when it's already registered */
+		moh_register(class, reload);
 
-			numclasses++;
-		}
+		numclasses++;
 	}
 
 	ast_config_destroy(cfg);
 
-	return numclasses;
-}
-
-static int ast_moh_destroy_one(struct mohclass *moh)
-{
-	char buff[8192];
-	int bytes, tbytes = 0, stop_time = 0, pid = 0;
-
-	if (moh) {
-		if (moh->pid > 1) {
-			ast_debug(1, "killing %d!\n", moh->pid);
-			stop_time = time(NULL) + 2;
-			pid = moh->pid;
-			moh->pid = 0;
-			/* 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);
-			while ((ast_wait_for_input(moh->srcfd, 100) > 0) && (bytes = read(moh->srcfd, buff, 8192)) && time(NULL) < stop_time)
-				tbytes = tbytes + bytes;
-			ast_debug(1, "mpg123 pid %d and child died after %d bytes read\n", pid, tbytes);
-			close(moh->srcfd);
-		}
-		ast_moh_free_class(&moh);
-	}
+	ao2_callback(mohclasses, OBJ_UNLINK | OBJ_NODATA | OBJ_MULTIPLE, 
+			moh_classes_delete_marked, NULL);
 
-	return 0;
+	return numclasses;
 }
 
 static void ast_moh_destroy(void)
 {
-	struct mohclass *moh;
-
 	ast_verb(2, "Destroying musiconhold processes\n");
-
-	AST_RWLIST_WRLOCK(&mohclasses);
-	while ((moh = AST_RWLIST_REMOVE_HEAD(&mohclasses, list))) {
-		ast_moh_destroy_one(moh);
-	}
-	AST_RWLIST_UNLOCK(&mohclasses);
+	ao2_callback(mohclasses, OBJ_UNLINK | OBJ_NODATA | OBJ_MULTIPLE, NULL, NULL);
 }
 
 static char *handle_cli_moh_reload(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)
@@ -1519,8 +1554,8 @@ static char *handle_cli_moh_reload(struct ast_cli_entry *e, int cmd, struct ast_
 
 static char *handle_cli_moh_show_files(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)
 {
-	int i;
 	struct mohclass *class;
+	struct ao2_iterator i;
 
 	switch (cmd) {
 	case CLI_INIT:
@@ -1537,16 +1572,20 @@ static char *handle_cli_moh_show_files(struct ast_cli_entry *e, int cmd, struct
 	if (a->argc != e->args)
 		return CLI_SHOWUSAGE;
 
-	AST_RWLIST_RDLOCK(&mohclasses);
-	AST_RWLIST_TRAVERSE(&mohclasses, class, list) {
-		if (!class->total_files)
+	i = ao2_iterator_init(mohclasses, 0);
+
+	for (; (class = ao2_iterator_next(&i)); mohclass_unref(class)) {
+		int x;
+
+		if (!class->total_files) {
 			continue;
+		}
 
 		ast_cli(a->fd, "Class: %s\n", class->name);
-		for (i = 0; i < class->total_files; i++)
-			ast_cli(a->fd, "\tFile: %s\n", class->filearray[i]);
+		for (x = 0; x < class->total_files; x++) {
+			ast_cli(a->fd, "\tFile: %s\n", class->filearray[x]);
+		}
 	}
-	AST_RWLIST_UNLOCK(&mohclasses);
 
 	return CLI_SUCCESS;
 }
@@ -1554,6 +1593,7 @@ static char *handle_cli_moh_show_files(struct ast_cli_entry *e, int cmd, struct
 static char *handle_cli_moh_show_classes(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)
 {
 	struct mohclass *class;
+	struct ao2_iterator i;
 
 	switch (cmd) {
 	case CLI_INIT:
@@ -1569,20 +1609,19 @@ static char *handle_cli_moh_show_classes(struct ast_cli_entry *e, int cmd, struc
 	if (a->argc != e->args)
 		return CLI_SHOWUSAGE;
 
-	AST_RWLIST_RDLOCK(&mohclasses);
-	AST_RWLIST_TRAVERSE(&mohclasses, class, list) {
+	i = ao2_iterator_init(mohclasses, 0);
+
+	for (; (class = ao2_iterator_next(&i)); mohclass_unref(class)) {
 		ast_cli(a->fd, "Class: %s\n", class->name);
 		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, "\tUse Count: %d\n", class->inuse);
-		if (class->digit)
-			ast_cli(a->fd, "\tDigit: %c\n", class->digit);
-		if (ast_test_flag(class, MOH_CUSTOM))
+		if (ast_test_flag(class, MOH_CUSTOM)) {
 			ast_cli(a->fd, "\tApplication: %s\n", S_OR(class->args, "<none>"));
-		if (strcasecmp(class->mode, "files"))
+		}
+		if (strcasecmp(class->mode, "files")) {
 			ast_cli(a->fd, "\tFormat: %s\n", ast_getformatname(class->format));
+		}
 	}
-	AST_RWLIST_UNLOCK(&mohclasses);
 
 	return CLI_SUCCESS;
 }
@@ -1593,31 +1632,36 @@ static struct ast_cli_entry cli_moh[] = {
 	AST_CLI_DEFINE(handle_cli_moh_show_files,   "List MusicOnHold file-based classes")
 };
 
-static int init_classes(int is_reload) 
+static int moh_class_hash(const void *obj, const int flags)
 {
-	struct mohclass *moh;
-    
-	if (!load_moh_classes(is_reload)) 		/* Load classes from config */
-		return 0;			/* Return if nothing is found */
-
-	AST_RWLIST_WRLOCK(&mohclasses);
-	AST_RWLIST_TRAVERSE_SAFE_BEGIN(&mohclasses, moh, list) {
-		if (is_reload && moh->delete) {
-			AST_RWLIST_REMOVE_CURRENT(list);
-			if (!moh->inuse)
-				ast_moh_destroy_one(moh);
-		}
-	}
-	AST_RWLIST_TRAVERSE_SAFE_END
-	AST_RWLIST_UNLOCK(&mohclasses);
+	const struct mohclass *class = obj;
+
+	return ast_str_case_hash(class->name);
+}
 
-	return 1;
+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;
 }
 
 static int load_module(void)
 {
 	int res;
 
+	if (!(mohclasses = ao2_container_alloc(53, moh_class_hash, moh_class_cmp))) {
+		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");
+	} else {
+		ast_install_music_functions(local_ast_moh_start, local_ast_moh_stop,
+				local_ast_moh_cleanup);
+	}
+
 	res = ast_register_application(play_moh, play_moh_exec, play_moh_syn, play_moh_desc);
 	ast_register_atexit(ast_moh_destroy);
 	ast_cli_register_multiple(cli_moh, ARRAY_LEN(cli_moh));
@@ -1630,21 +1674,24 @@ static int load_module(void)
 	if (!res)
 		res = ast_register_application(stop_moh, stop_moh_exec, stop_moh_syn, stop_moh_desc);
 
-	if (!init_classes(0)) { 	/* No music classes configured, so skip it */
-		ast_log(LOG_WARNING, "No music on hold classes configured, disabling music on hold.\n");
-	} else {
-		ast_install_music_functions(local_ast_moh_start, local_ast_moh_stop, local_ast_moh_cleanup);
+	return AST_MODULE_LOAD_SUCCESS;
+}
+
+static int reload(void)
+{
+	if (load_moh_classes(1)) {
+		ast_install_music_functions(local_ast_moh_start, local_ast_moh_stop,
+				local_ast_moh_cleanup);
 	}
 
 	return AST_MODULE_LOAD_SUCCESS;
 }
 
-static int reload(void)
+static int moh_class_inuse(void *obj, void *arg, int flags)
 {
-	if (init_classes(1))
-		ast_install_music_functions(local_ast_moh_start, local_ast_moh_stop, local_ast_moh_cleanup);
+	struct mohclass *class = obj;
 
-	return 0;
+	return AST_LIST_EMPTY(&class->members) ? 0 : CMP_MATCH | CMP_STOP;
 }
 
 static int unload_module(void)
@@ -1652,20 +1699,20 @@ static int unload_module(void)
 	int res = 0;
 	struct mohclass *class = NULL;
 
-	AST_RWLIST_WRLOCK(&mohclasses);
-	AST_LIST_TRAVERSE(&mohclasses, class, list) {
-		if (class->inuse > 0) {
-			res = -1;
-			break;
-		}
+	/* XXX This check shouldn't be required if module ref counting was being used
+	 * properly ... */
+	if ((class = ao2_callback(mohclasses, 0, moh_class_inuse, NULL))) {
+		class = mohclass_unref(class);
+		res = -1;
 	}
-	AST_RWLIST_UNLOCK(&mohclasses);
+
 	if (res < 0) {
 		ast_log(LOG_WARNING, "Unable to unload res_musiconhold due to active MOH channels\n");
 		return res;
 	}
 
 	ast_uninstall_music_functions();
+
 	ast_moh_destroy();
 	res = ast_unregister_application(play_moh);
 	res |= ast_unregister_application(wait_moh);
@@ -1674,11 +1721,12 @@ static int unload_module(void)
 	res |= ast_unregister_application(stop_moh);
 	ast_cli_unregister_multiple(cli_moh, ARRAY_LEN(cli_moh));
 	ast_unregister_atexit(ast_moh_destroy);
+
 	return res;
 }
 
 AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_DEFAULT, "Music On Hold Resource",
-		.load = load_module,
-		.unload = unload_module,
-		.reload = reload,
-	       );
+	.load = load_module,
+	.unload = unload_module,
+	.reload = reload,
+);