Fix timing source dependency issues with MOH

Prior to this patch, res_musiconhold existed at the same module priority level
as the timing sources that it depends on.  This would cause a problem when
music on hold was reloaded, as the timing source could be changed after
res_musiconhold was processed.  This patch adds a new module priority level,
AST_MODPRI_TIMING, that the various timing modules are now loaded at.  This
now occurs before loading other resource modules, such that the timing source
is guaranteed to be set prior to resolving the timing source dependencies.

(closes issue ASTERISK-17474)
Reporter: Luke H
Tested by: Luke H, Vladimir Mikhelson, zzsurf, Wes Van Tlghem, elguero, Thomas Arimont
Patches:
 asterisk-17474-dahdi_timing-infinite-wait-fix_v3_branch-1.8.diff uploaded by elguero (License #5026)
 asterisk-17474-dahdi_timing-infinite-wait-fix_v3_branch-10.diff uploaded by elguero (License #5026)
 asterisk-17474-dahdi_timing-infinite-wait-fix_v3.diff uploaded by elguero (License #5026)

Review: https://reviewboard.asterisk.org/r/1578/

git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.8@349194 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/include/asterisk/module.h b/include/asterisk/module.h
index db9c5b46568e8ede1d5ee65199f6a46d3edf5eb9..df9f241378e9a41a78a5549de39b78232abda4b2 100644 (file)
--- a/include/asterisk/module.h
+++ b/include/asterisk/module.h
@@ -205,6 +205,7 @@ enum ast_module_load_priority {
        AST_MODPRI_REALTIME_DEPEND =    10,  /*!< Dependency for a realtime driver */
        AST_MODPRI_REALTIME_DEPEND2 =   20,  /*!< Second level dependency for a realtime driver (func_curl needs res_curl, but is needed by res_config_curl) */
        AST_MODPRI_REALTIME_DRIVER =    30,  /*!< A realtime driver, which provides configuration services for other modules */
+       AST_MODPRI_TIMING =             40,  /*!< Dependency for a channel (MOH needs timing interfaces to be fully loaded) */
        AST_MODPRI_CHANNEL_DEPEND =     50,  /*!< Channel driver dependency (may depend upon realtime, e.g. MOH) */
        AST_MODPRI_CHANNEL_DRIVER =     60,  /*!< Channel drivers (provide devicestate) */
        AST_MODPRI_APP_DEPEND =         70,  /*!< Dependency for an application */

diff --git a/res/res_musiconhold.c b/res/res_musiconhold.c
index 846c4e0cdd6bf06953e17bcf29abdc98a36b983b..e033fecad51b58017fd1a6b6379b44836e81c932 100644 (file)
--- a/res/res_musiconhold.c
+++ b/res/res_musiconhold.c
@@ -646,7 +646,7 @@ static void *monmp3thread(void *data)
                        }
                }
                if (class->timer) {
-                       struct pollfd pfd = { .fd = ast_timer_fd(class->timer), .events = POLLIN, };
+                       struct pollfd pfd = { .fd = ast_timer_fd(class->timer), .events = POLLIN | POLLPRI, };
 
 #ifdef SOLARIS
                        thr_yield();
@@ -656,7 +656,7 @@ static void *monmp3thread(void *data)
                                ast_timer_ack(class->timer, 1);
                                res = 320;
                        } else {
-                               ast_log(LOG_ERROR, "poll() failed: %s\n", strerror(errno));
+                               ast_log(LOG_WARNING, "poll() failed: %s\n", strerror(errno));
                                res = 0;
                        }
                        pthread_testcancel();
@@ -1195,6 +1195,7 @@ static int init_app_class(struct mohclass *class)
 
        if (!(class->timer = ast_timer_open())) {
                ast_log(LOG_WARNING, "Unable to create timer: %s\n", strerror(errno));
+               return -1;
        }
        if (class->timer && ast_timer_set_rate(class->timer, 25)) {
                ast_log(LOG_WARNING, "Unable to set 40ms frame rate: %s\n", strerror(errno));
@@ -1222,7 +1223,9 @@ static int _moh_register(struct mohclass *moh, int reload, int unref, const char
 {
        struct mohclass *mohclass = NULL;
 
-       if ((mohclass = _get_mohbyname(moh->name, 0, MOH_NOTDELETED, file, line, funcname)) && !moh_diff(mohclass, moh)) {
+       mohclass = _get_mohbyname(moh->name, 0, MOH_NOTDELETED, file, line, funcname);
+
+       if (mohclass && !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) {
@@ -1561,6 +1564,12 @@ static void moh_class_destructor(void *obj)
 
        ast_debug(1, "Destroying MOH class '%s'\n", class->name);
 
+       ao2_lock(class);
+       while ((member = AST_LIST_REMOVE_HEAD(&class->members, list))) {
+               free(member);
+       }
+       ao2_unlock(class);
+
        /* 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) {
@@ -1612,10 +1621,7 @@ static void moh_class_destructor(void *obj)
                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);
+               class->srcfd = -1;
        }
 
        if (class->filearray) {
@@ -1636,6 +1642,7 @@ static void moh_class_destructor(void *obj)
        if (tid > 0) {
                pthread_join(tid, NULL);
        }
+
 }
 
 static int moh_class_mark(void *obj, void *arg, int flags)

diff --git a/res/res_timing_dahdi.c b/res/res_timing_dahdi.c
index 61dda6a44c84e1e0a220f972c342992b4ef755f7..7aafe796675dbcfeaa85ab01ec09218e86d8fc12 100644 (file)
--- a/res/res_timing_dahdi.c
+++ b/res/res_timing_dahdi.c
@@ -203,5 +203,5 @@ static int unload_module(void)
 AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_LOAD_ORDER, "DAHDI Timing Interface",
                .load = load_module,
                .unload = unload_module,
-               .load_pri = AST_MODPRI_CHANNEL_DEPEND,
+               .load_pri = AST_MODPRI_TIMING,
                );

diff --git a/res/res_timing_pthread.c b/res/res_timing_pthread.c
index e89ded2d674eab8cdaca50e4444d2a01b8adc03d..654f1d082943d0346b0e812041f81faf2c4cb9aa 100644 (file)
--- a/res/res_timing_pthread.c
+++ b/res/res_timing_pthread.c
@@ -525,5 +525,5 @@ static int unload_module(void)
 AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_LOAD_ORDER, "pthread Timing Interface",
                .load = load_module,
                .unload = unload_module,
-               .load_pri = AST_MODPRI_CHANNEL_DEPEND,
+               .load_pri = AST_MODPRI_TIMING,
                );

diff --git a/res/res_timing_timerfd.c b/res/res_timing_timerfd.c
index 81331a99faef515fd17a92ccab9c319583241006..944687459aa068c0fe53ab50b9a61a50d107788a 100644 (file)
--- a/res/res_timing_timerfd.c
+++ b/res/res_timing_timerfd.c
@@ -337,5 +337,5 @@ static int unload_module(void)
 AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_LOAD_ORDER, "Timerfd Timing Interface",
                .load = load_module,
                .unload = unload_module,
-               .load_pri = AST_MODPRI_CHANNEL_DEPEND,
+               .load_pri = AST_MODPRI_TIMING,
                );