aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authortilghman <tilghman@f38db490-d61c-443f-a65b-d21fe96a405b>2009-12-03 00:18:02 +0000
committertilghman <tilghman@f38db490-d61c-443f-a65b-d21fe96a405b>2009-12-03 00:18:02 +0000
commitee03fd27bd722ac0814d8b5d08336f91cf96ba76 (patch)
tree31d83bda6a9f65cee2f938f7536f5933249f9af4
parent26bc076ced5f82412ed8a299e1c8fed786648904 (diff)
Recorded merge of revisions 232660-232661 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk ........ r232660 | tilghman | 2009-12-02 18:08:55 -0600 (Wed, 02 Dec 2009) | 19 lines Fix multiple issues with musiconhold, which led to classes not getting destroyed properly. * Classes are now tracked past removal from the core container, and module removal is actively prevented until all references are freed. * A hanging reference stored in the channel has been removed. This could have caused a mismatch and the music state not properly cleared, if two or more reloads occurred between MOH being stopped and MOH being restarted. * In certain circumstances, duplicate classes were possible. * A race existed at reload time between a process being killed and the thread responsible for reading from the related pipe respawning that process. * Several reference counts have also been corrected. At least one could have caused deleted classes to stick around forever, consuming resources. This originally manifested as MOH external processes that were not killed at reload time. (closes issue #16279, closes issue #16207) Reported by: parisioa, dcabot Patches: 20091202__issue16279__2.diff.txt uploaded by tilghman (license 14) Tested by: parisioa, tilghman ........ r232661 | tilghman | 2009-12-02 18:09:36 -0600 (Wed, 02 Dec 2009) | 2 lines Remove debugging line ........ git-svn-id: http://svn.digium.com/svn/asterisk/branches/1.6.1@232666 f38db490-d61c-443f-a65b-d21fe96a405b
-rw-r--r--res/res_musiconhold.c227
1 files changed, 186 insertions, 41 deletions
diff --git a/res/res_musiconhold.c b/res/res_musiconhold.c
index fde175e13..af4a32350 100644
--- a/res/res_musiconhold.c
+++ b/res/res_musiconhold.c
@@ -68,6 +68,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";
@@ -121,11 +123,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;
};
@@ -137,6 +141,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 {
@@ -178,6 +185,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"
@@ -213,7 +222,7 @@ static void moh_files_release(struct ast_channel *chan, void *data)
state->save_pos = state->pos;
- mohclass_unref(state->class, "Unreffing channel's music class upon deactivation of generator");
+ state->class = mohclass_unref(state->class, "Unreffing channel's music class upon deactivation of generator");
}
static int ast_moh_files_next(struct ast_channel *chan)
@@ -322,7 +331,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;
@@ -331,6 +345,9 @@ static void *moh_files_alloc(struct ast_channel *chan, void *params)
state->class = mohclass_ref(class, "Reffing music class for channel");
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);
@@ -514,6 +531,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
@@ -574,11 +628,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 {
@@ -707,7 +775,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 = {
@@ -716,7 +786,11 @@ static struct mohclass *get_mohbyname(const char *name, int warn)
ast_copy_string(tmp_class.name, name, sizeof(tmp_class.name));
- moh = ao2_t_find(mohclasses, &tmp_class, 0, "Finding by name");
+#ifdef REF_DEBUG
+ moh = _ao2_find_debug(mohclasses, &tmp_class, flags, "get_mohbyname", (char *) file, lineno, funcname);
+#else
+ moh = _ao2_find(mohclasses, &tmp_class, flags);
+#endif
if (!moh && warn) {
ast_log(LOG_DEBUG, "Music on Hold class '%s' not found in memory\n", name);
@@ -1048,22 +1122,23 @@ 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, "unreffing mohclass we just found by name");
- if (unref) {
- moh = mohclass_unref(moh, "unreffing potential new moh class (it is a duplicate)");
- }
- 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, "unreffing mohclass we just found by name");
+ if (unref) {
+ moh = mohclass_unref(moh, "unreffing potential new moh class (it is a duplicate)");
}
- mohclass = mohclass_unref(mohclass, "Unreffing mohclass we just found by name");
+ return -1;
+ } else if (mohclass) {
+ /* Found a class, but it's different from the one being registered */
+ mohclass = mohclass_unref(mohclass, "unreffing mohclass we just found by name");
}
time(&moh->start);
@@ -1101,6 +1176,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, "Channel MOH state destruction");
+ }
ast_free(chan->music_state);
chan->music_state = NULL;
}
@@ -1108,11 +1186,21 @@ 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;
- if ((class = ao2_t_alloc(sizeof(*class), moh_class_destructor, "Allocating new moh class"))) {
+ if ((class =
+#ifdef REF_DEBUG
+ _ao2_alloc_debug(sizeof(*class), moh_class_destructor, "Allocating new moh class", file, line, funcname, 1)
+#elif defined(__AST_DEBUG_MALLOC)
+ _ao2_alloc_debug(sizeof(*class), moh_class_destructor, "Allocating new moh class", file, line, funcname, 0)
+#else
+ ao2_alloc(sizeof(*class), moh_class_destructor)
+#endif
+ )) {
class->format = AST_FORMAT_SLINEAR;
}
@@ -1139,26 +1227,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, SENTINEL);
}
}
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, SENTINEL);
}
}
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, SENTINEL);
}
}
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", SENTINEL);
}
@@ -1236,7 +1324,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 */
@@ -1353,9 +1441,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;
@@ -1369,11 +1468,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) {
@@ -1389,12 +1502,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++) {
@@ -1403,6 +1510,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)
@@ -1418,7 +1530,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)
@@ -1509,7 +1626,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++;
}
}
@@ -1620,6 +1737,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_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;
}
@@ -1641,7 +1772,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)
@@ -1652,6 +1785,14 @@ 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");
@@ -1720,6 +1861,10 @@ static int unload_module(void)
ast_cli_unregister_multiple(cli_moh, sizeof(cli_moh) / sizeof(struct ast_cli_entry));
ast_unregister_atexit(ast_moh_destroy);
+ pthread_cancel(deleted_thread);
+ pthread_kill(deleted_thread, SIGURG);
+ pthread_join(deleted_thread, NULL);
+
return res;
}