Index: include/asterisk/astobj2.h =================================================================== --- include/asterisk/astobj2.h (revision 231886) +++ include/asterisk/astobj2.h (working copy) @@ -206,7 +206,7 @@ #ifdef REF_DEBUG #define dialog_ref(arg1,arg2) dialog_ref_debug((arg1),(arg2), __FILE__, __LINE__, __PRETTY_FUNCTION__) #define dialog_unref(arg1,arg2) dialog_unref_debug((arg1),(arg2), __FILE__, __LINE__, __PRETTY_FUNCTION__) -static struct sip_pvt *dialog_ref_debug(struct sip_pvt *p, char *tag, char *file, int line, const char *func) +static struct sip_pvt *dialog_ref_debug(struct sip_pvt *p, char *tag, const char *file, int line, const char *func) { if (p) ao2_ref_debug(p, 1, tag, file, line, func); @@ -215,7 +215,7 @@ return p; } -static struct sip_pvt *dialog_unref_debug(struct sip_pvt *p, char *tag, char *file, int line, const char *func) +static struct sip_pvt *dialog_unref_debug(struct sip_pvt *p, char *tag, const char *file, int line, const char *func) { if (p) ao2_ref_debug(p, -1, tag, file, line, func); @@ -456,7 +456,7 @@ #endif -int _ao2_ref_debug(void *o, int delta, char *tag, char *file, int line, const char *funcname); +int _ao2_ref_debug(void *o, int delta, char *tag, const char *file, int line, const char *funcname); int _ao2_ref(void *o, int delta); /*! @} */ @@ -720,7 +720,7 @@ ao2_hash_fn *hash_fn, ao2_callback_fn *cmp_fn); struct ao2_container *_ao2_container_alloc_debug(const unsigned int n_buckets, ao2_hash_fn *hash_fn, ao2_callback_fn *cmp_fn, - char *tag, char *file, int line, const char *funcname, + char *tag, const char *file, int line, const char *funcname, int ref_debug); /*! \brief @@ -766,7 +766,7 @@ #endif -void *_ao2_link_debug(struct ao2_container *c, void *new_obj, char *tag, char *file, int line, const char *funcname); +void *_ao2_link_debug(struct ao2_container *c, void *new_obj, char *tag, const char *file, int line, const char *funcname); void *_ao2_link(struct ao2_container *c, void *newobj); /*! @@ -797,7 +797,7 @@ #endif -void *_ao2_unlink_debug(struct ao2_container *c, void *obj, char *tag, char *file, int line, const char *funcname); +void *_ao2_unlink_debug(struct ao2_container *c, void *obj, char *tag, const char *file, int line, const char *funcname); void *_ao2_unlink(struct ao2_container *c, void *obj); @@ -900,7 +900,7 @@ void *_ao2_callback_debug(struct ao2_container *c, enum search_flags flags, ao2_callback_fn *cb_fn, void *arg, char *tag, - char *file, int line, const char *funcname); + const char *file, int line, const char *funcname); void *_ao2_callback(struct ao2_container *c, enum search_flags flags, ao2_callback_fn *cb_fn, void *arg); @@ -920,7 +920,7 @@ #endif -void *_ao2_find_debug(struct ao2_container *c, void *arg, enum search_flags flags, char *tag, char *file, int line, const char *funcname); +void *_ao2_find_debug(struct ao2_container *c, void *arg, enum search_flags flags, char *tag, const char *file, int line, const char *funcname); void *_ao2_find(struct ao2_container *c, void *arg, enum search_flags flags); /*! \brief @@ -1072,7 +1072,7 @@ #endif -void *_ao2_iterator_next_debug(struct ao2_iterator *a, char *tag, char *file, int line, const char *funcname); +void *_ao2_iterator_next_debug(struct ao2_iterator *a, char *tag, const char *file, int line, const char *funcname); void *_ao2_iterator_next(struct ao2_iterator *a); /* extra functions */ Index: main/astobj2.c =================================================================== --- main/astobj2.c (revision 231886) +++ main/astobj2.c (working copy) @@ -135,7 +135,7 @@ static struct bucket_list *__ao2_link(struct ao2_container *c, void *user_data); static void *__ao2_callback(struct ao2_container *c, const enum search_flags flags, ao2_callback_fn *cb_fn, void *arg, - char *tag, char *file, int line, const char *funcname); + char *tag, const char *file, int line, const char *funcname); static void * __ao2_iterator_next(struct ao2_iterator *a, struct bucket_list **q); #ifndef DEBUG_THREADS @@ -221,7 +221,7 @@ */ -int _ao2_ref_debug(void *user_data, const int delta, char *tag, char *file, int line, const char *funcname) +int _ao2_ref_debug(void *user_data, const int delta, char *tag, const char *file, int line, const char *funcname) { struct astobj2 *obj = INTERNAL_OBJ(user_data); @@ -439,7 +439,7 @@ } struct ao2_container *_ao2_container_alloc_debug(const unsigned int n_buckets, ao2_hash_fn *hash_fn, - ao2_callback_fn *cmp_fn, char *tag, char *file, int line, + ao2_callback_fn *cmp_fn, char *tag, const char *file, int line, const char *funcname, int ref_debug) { /* XXX maybe consistency check on arguments ? */ @@ -516,7 +516,7 @@ return p; } -void *_ao2_link_debug(struct ao2_container *c, void *user_data, char *tag, char *file, int line, const char *funcname) +void *_ao2_link_debug(struct ao2_container *c, void *user_data, char *tag, const char *file, int line, const char *funcname) { struct bucket_list *p = __ao2_link(c, user_data); @@ -551,7 +551,7 @@ * and destroy the associated * ao2_bucket_list structure. */ void *_ao2_unlink_debug(struct ao2_container *c, void *user_data, char *tag, - char *file, int line, const char *funcname) + const char *file, int line, const char *funcname) { if (INTERNAL_OBJ(user_data) == NULL) /* safety check on the argument */ return NULL; @@ -589,7 +589,7 @@ */ static void *__ao2_callback(struct ao2_container *c, const enum search_flags flags, ao2_callback_fn *cb_fn, void *arg, - char *tag, char *file, int line, const char *funcname) + char *tag, const char *file, int line, const char *funcname) { int i, start, last; /* search boundaries */ void *ret = NULL; @@ -701,7 +701,7 @@ void *_ao2_callback_debug(struct ao2_container *c, const enum search_flags flags, ao2_callback_fn *cb_fn, void *arg, - char *tag, char *file, int line, const char *funcname) + char *tag, const char *file, int line, const char *funcname) { return __ao2_callback(c,flags, cb_fn, arg, tag, file, line, funcname); } @@ -715,7 +715,7 @@ /*! * the find function just invokes the default callback with some reasonable flags. */ -void *_ao2_find_debug(struct ao2_container *c, void *arg, enum search_flags flags, char *tag, char *file, int line, const char *funcname) +void *_ao2_find_debug(struct ao2_container *c, void *arg, enum search_flags flags, char *tag, const char *file, int line, const char *funcname) { return _ao2_callback_debug(c, flags, c->cmp_fn, arg, tag, file, line, funcname); } @@ -807,7 +807,7 @@ return ret; } -void * _ao2_iterator_next_debug(struct ao2_iterator *a, char *tag, char *file, int line, const char *funcname) +void * _ao2_iterator_next_debug(struct ao2_iterator *a, char *tag, const char *file, int line, const char *funcname) { struct bucket_list *p; void *ret = NULL; 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; }; @@ -137,6 +143,9 @@ #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 +187,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 +224,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 +333,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 +347,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 +533,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 +630,25 @@ 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 +777,9 @@ 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 +788,11 @@ 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 +1124,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, 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 +1178,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 +1188,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 +1229,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 +1326,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 +1443,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 != 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 +1470,25 @@ /* 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 +1504,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 +1512,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 +1532,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 +1628,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 +1739,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 +1774,9 @@ { 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 +1787,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 +1863,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; }