Index: res/res_musiconhold.c =================================================================== --- res/res_musiconhold.c (revision 231886) +++ res/res_musiconhold.c (working copy) @@ -50,6 +50,8 @@ #include #endif +#define REF_DEBUG + #include "asterisk/lock.h" #include "asterisk/file.h" #include "asterisk/channel.h" @@ -68,6 +70,8 @@ #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 +125,13 @@ 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; }; @@ -136,6 +142,7 @@ #define MOH_SORTALPHA (1 << 4) #define MOH_CACHERTCLASSES (1 << 5) /*!< Should we use a separate instance of MOH for each user or not */ +#define MOH_NOTDELETED (1 << 6) /*!< Find only records that aren't deleted? */ static struct ast_flags global_flags[1] = {{0}}; /*!< global MOH_ flags */ @@ -178,6 +185,8 @@ }; 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 @@ 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 @@ 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 @@ 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 @@ 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,17 @@ class->srcfd = -1; pthread_testcancel(); if (class->pid > 1) { - killpg(class->pid, SIGHUP); + if (killpg(class->pid, SIGHUP) < 0 && kill(class->pid, SIGHUP) < 0) { + ast_log(LOG_WARNING, "Unable to send a SIGHUP to MOH process?!!: %s\n", strerror(errno)); + } usleep(100000); - killpg(class->pid, SIGTERM); + if (killpg(class->pid, SIGTERM) < 0 && kill(class->pid, SIGTERM) < 0) { + ast_log(LOG_WARNING, "Unable to terminate MOH process?!!: %s\n", strerror(errno)); + } usleep(100000); - killpg(class->pid, SIGKILL); + if (killpg(class->pid, SIGKILL) < 0 && kill(class->pid, SIGKILL) < 0) { + ast_log(LOG_WARNING, "Unable to kill MOH process?!!: %s\n", strerror(errno)); + } class->pid = 0; } } else { @@ -707,16 +767,22 @@ 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, char *file, int lineno, const char *funcname) { struct mohclass *moh = NULL; struct mohclass tmp_class = { - .flags = 0, + .flags = flags, }; 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, 0, "get_mohbyname", file, lineno, funcname); +#else + moh = _ao2_find(mohclasses, &tmp_class, 0); +#endif if (!moh && warn) { ast_log(LOG_DEBUG, "Music on Hold class '%s' not found in memory\n", name); @@ -1048,22 +1114,23 @@ } /*! - * \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, char *file, int line, 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 +1168,9 @@ 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 +1178,21 @@ 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 +1219,26 @@ * 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 +1316,7 @@ * 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 +1433,20 @@ { 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 > 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 +1460,17 @@ /* 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); + if (killpg(class->pid, SIGHUP) < 0 && kill(class->pid, SIGHUP) < 0) { + ast_log(LOG_WARNING, "Unable to send a SIGHUP to MOH process?!!: %s\n", strerror(errno)); + } usleep(100000); - killpg(pid, SIGTERM); + if (killpg(class->pid, SIGTERM) < 0 && kill(class->pid, SIGTERM) < 0) { + ast_log(LOG_WARNING, "Unable to terminate MOH process?!!: %s\n", strerror(errno)); + } usleep(100000); - killpg(pid, SIGKILL); + if (killpg(class->pid, SIGKILL) < 0 && kill(class->pid, SIGKILL) < 0) { + ast_log(LOG_WARNING, "Unable to kill MOH process?!!: %s\n", strerror(errno)); + } while ((ast_wait_for_input(class->srcfd, 100) > 0) && (bytes = read(class->srcfd, buff, 8192)) && time(NULL) < stime) { @@ -1389,12 +1486,6 @@ 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 +1494,11 @@ 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 +1514,12 @@ { 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 +1610,7 @@ } /* 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 +1721,20 @@ } } 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, "")); + ast_cli(a->fd, "\tDirectory: %s\n", S_OR(class->dir, "")); + 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, "")); + } + 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 +1756,7 @@ { 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 ? 0 : CMP_MATCH | CMP_STOP; } static int load_module(void) @@ -1652,6 +1767,14 @@ 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 +1843,10 @@ 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; }