From 8372a9bf08a152a65f8483324d6c0f679b660ff2 Mon Sep 17 00:00:00 2001
From: Russell Bryant <russell@russellbryant.com>
Date: Mon, 3 Mar 2008 15:59:50 +0000
Subject: [PATCH] 3) In addition to merging the changes below, change trunk
back to a regular LIST instead of an RWLIST. The way this list works
makes it such that a RWLIST provides no additional benefit. Also, a mutex
is needed for use with the thread condition.
Merged revisions 105563 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4
........
r105563 | russell | 2008-03-03 09:50:43 -0600 (Mon, 03 Mar 2008) | 24 lines
Merge in some changes from team/russell/autoservice-nochans-1.4
These changes fix up some dubious code that I came across while auditing what
happens in the autoservice thread when there are no channels currently in
autoservice.
1) Change it so that autoservice thread doesn't keep looping around calling
ast_waitfor_n() on 0 channels twice a second. Instead, use a thread condition
so that the thread properly goes to sleep and does not wake up until a
channel is put into autoservice.
This actually fixes an interesting bug, as well. If the autoservice thread
is already running (almost always is the case), then when the thread goes
from having 0 channels to have 1 channel to autoservice, that channel would
have to wait for up to 1/2 of a second to have the first frame read from it.
2) Fix up the code in ast_waitfor_nandfds() for when it gets called with no
channels and no fds to poll() on, such as was the case with the previous code
for the autoservice thread. In this case, the code would call alloca(0), and
pass the result as the first argument to poll(). In this case, the 2nd
argument to poll() specified that there were no fds, so this invalid pointer
shouldn't actually get dereferenced, but, this code makes it explicit and
ensures the pointers are NULL unless we have valid data to put there.
(related to issue #12116)
........
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@105564 65c4cc65-6c06-0410-ace0-fbb531ad65f3
---
include/asterisk/_private.h | 1 +
main/asterisk.c | 2 ++
main/autoservice.c | 56 +++++++++++++++++++++++--------------
main/channel.c | 11 ++++----
4 files changed, 44 insertions(+), 26 deletions(-)
diff --git a/include/asterisk/_private.h b/include/asterisk/_private.h
index 33ba835937..b477d0e93c 100644
--- a/include/asterisk/_private.h
+++ b/include/asterisk/_private.h
@@ -33,6 +33,7 @@ int ast_device_state_engine_init(void); /*!< Provided by devicestate.c */
int astobj2_init(void); /*!< Provided by astobj2.c */
int ast_file_init(void); /*!< Provided by file.c */
int ast_features_init(void); /*!< Provided by features.c */
+void ast_autoservice_init(void); /*!< Provided by autoservice.c */
/*!
* \brief Reload asterisk modules.
diff --git a/main/asterisk.c b/main/asterisk.c
index 5de317fc5d..2548d43164 100644
--- a/main/asterisk.c
+++ b/main/asterisk.c
@@ -3186,6 +3186,8 @@ int main(int argc, char *argv[])
astobj2_init();
+ ast_autoservice_init();
+
if (load_modules(1)) { /* Load modules, pre-load only */
printf(term_quit());
exit(1);
diff --git a/main/autoservice.c b/main/autoservice.c
index 7fb86dad7b..74b1015eff 100644
--- a/main/autoservice.c
+++ b/main/autoservice.c
@@ -30,6 +30,8 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
#include <sys/time.h>
#include <signal.h>
+#include "asterisk/_private.h" /* prototype for ast_autoservice_init() */
+
#include "asterisk/pbx.h"
#include "asterisk/frame.h"
#include "asterisk/sched.h"
@@ -56,7 +58,8 @@ struct asent {
AST_LIST_ENTRY(asent) list;
};
-static AST_RWLIST_HEAD_STATIC(aslist, asent);
+static AST_LIST_HEAD_STATIC(aslist, asent);
+static ast_cond_t as_cond;
static pthread_t asthread = AST_PTHREADT_NULL;
@@ -67,14 +70,14 @@ static void defer_frame(struct ast_channel *chan, struct ast_frame *f)
struct ast_frame *dup_f;
struct asent *as;
- AST_RWLIST_WRLOCK(&aslist);
- AST_RWLIST_TRAVERSE(&aslist, as, list) {
+ AST_LIST_LOCK(&aslist);
+ AST_LIST_TRAVERSE(&aslist, as, list) {
if (as->chan != chan)
continue;
if ((dup_f = ast_frdup(f)))
AST_LIST_INSERT_TAIL(&as->dtmf_frames, dup_f, frame_list);
}
- AST_RWLIST_UNLOCK(&aslist);
+ AST_LIST_UNLOCK(&aslist);
}
static void *autoservice_run(void *ign)
@@ -84,13 +87,16 @@ static void *autoservice_run(void *ign)
struct asent *as;
int x = 0, ms = 500;
- AST_RWLIST_RDLOCK(&aslist);
+ AST_LIST_LOCK(&aslist);
/* At this point, we know that no channels that have been removed are going
* to get used again. */
as_chan_list_state++;
- AST_RWLIST_TRAVERSE(&aslist, as, list) {
+ if (AST_LIST_EMPTY(&aslist))
+ ast_cond_wait(&as_cond, &aslist.lock);
+
+ AST_LIST_TRAVERSE(&aslist, as, list) {
if (!ast_check_hangup(as->chan)) {
if (x < MAX_AUTOMONS)
mons[x++] = as->chan;
@@ -98,7 +104,8 @@ static void *autoservice_run(void *ign)
ast_log(LOG_WARNING, "Exceeded maximum number of automatic monitoring events. Fix autoservice.c\n");
}
}
- AST_RWLIST_UNLOCK(&aslist);
+
+ AST_LIST_UNLOCK(&aslist);
if ((chan = ast_waitfor_n(mons, x, &ms))) {
struct ast_frame *f = ast_read(chan);
@@ -158,14 +165,14 @@ int ast_autoservice_start(struct ast_channel *chan)
int res = 0;
struct asent *as;
- AST_RWLIST_WRLOCK(&aslist);
- AST_RWLIST_TRAVERSE(&aslist, as, list) {
+ AST_LIST_LOCK(&aslist);
+ AST_LIST_TRAVERSE(&aslist, as, list) {
if (as->chan == chan) {
as->use_count++;
break;
}
}
- AST_RWLIST_UNLOCK(&aslist);
+ AST_LIST_UNLOCK(&aslist);
if (as) {
/* Entry exists, autoservice is already handling this channel */
@@ -185,18 +192,20 @@ int ast_autoservice_start(struct ast_channel *chan)
ast_set_flag(chan, AST_FLAG_END_DTMF_ONLY);
ast_channel_unlock(chan);
- AST_RWLIST_WRLOCK(&aslist);
- AST_RWLIST_INSERT_HEAD(&aslist, as, list);
- AST_RWLIST_UNLOCK(&aslist);
+ AST_LIST_LOCK(&aslist);
+ if (AST_LIST_EMPTY(&aslist))
+ ast_cond_signal(&as_cond);
+ AST_LIST_INSERT_HEAD(&aslist, as, list);
+ AST_LIST_UNLOCK(&aslist);
if (asthread == AST_PTHREADT_NULL) { /* need start the thread */
if (ast_pthread_create_background(&asthread, NULL, autoservice_run, NULL)) {
ast_log(LOG_WARNING, "Unable to create autoservice thread :(\n");
/* There will only be a single member in the list at this point,
the one we just added. */
- AST_RWLIST_WRLOCK(&aslist);
- AST_RWLIST_REMOVE(&aslist, as, list);
- AST_RWLIST_UNLOCK(&aslist);
+ AST_LIST_LOCK(&aslist);
+ AST_LIST_REMOVE(&aslist, as, list);
+ AST_LIST_UNLOCK(&aslist);
free(as);
res = -1;
} else
@@ -218,7 +227,7 @@ int ast_autoservice_stop(struct ast_channel *chan)
AST_LIST_HEAD_INIT_NOLOCK(&dtmf_frames);
- AST_RWLIST_WRLOCK(&aslist);
+ AST_LIST_LOCK(&aslist);
/* Save the autoservice channel list state. We _must_ verify that the channel
* list has been rebuilt before we return. Because, after we return, the channel
@@ -226,9 +235,9 @@ int ast_autoservice_stop(struct ast_channel *chan)
* it after its gone! */
chan_list_state = as_chan_list_state;
- AST_RWLIST_TRAVERSE_SAFE_BEGIN(&aslist, as, list) {
+ AST_LIST_TRAVERSE_SAFE_BEGIN(&aslist, as, list) {
if (as->chan == chan) {
- AST_RWLIST_REMOVE_CURRENT(list);
+ AST_LIST_REMOVE_CURRENT(list);
as->use_count--;
if (as->use_count)
break;
@@ -241,12 +250,12 @@ int ast_autoservice_stop(struct ast_channel *chan)
break;
}
}
- AST_RWLIST_TRAVERSE_SAFE_END;
+ AST_LIST_TRAVERSE_SAFE_END;
if (removed && asthread != AST_PTHREADT_NULL)
pthread_kill(asthread, SIGURG);
- AST_RWLIST_UNLOCK(&aslist);
+ AST_LIST_UNLOCK(&aslist);
if (!removed)
return 0;
@@ -268,3 +277,8 @@ int ast_autoservice_stop(struct ast_channel *chan)
return res;
}
+
+void ast_autoservice_init(void)
+{
+ ast_cond_init(&as_cond, NULL);
+}
diff --git a/main/channel.c b/main/channel.c
index a7e158d329..5742341c4a 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -1771,7 +1771,7 @@ struct ast_channel *ast_waitfor_nandfds(struct ast_channel **c, int n, int *fds,
#endif
{
struct timeval start = { 0 , 0 };
- struct pollfd *pfds;
+ struct pollfd *pfds = NULL;
int res;
long rms;
int x, y, max;
@@ -1782,11 +1782,12 @@ struct ast_channel *ast_waitfor_nandfds(struct ast_channel **c, int n, int *fds,
struct fdmap {
int chan;
int fdno;
- } *fdmap;
+ } *fdmap = NULL;
- sz = n * AST_MAX_FDS + nfds;
- pfds = alloca(sizeof(*pfds) * sz);
- fdmap = alloca(sizeof(*fdmap) * sz);
+ if ((sz = n * AST_MAX_FDS + nfds)) {
+ pfds = alloca(sizeof(*pfds) * sz);
+ fdmap = alloca(sizeof(*fdmap) * sz);
+ }
if (outfd)
*outfd = -99999;
--
GitLab