[asterisk-commits] wedhorn: branch wedhorn/readq-locking r189269 - in /team/wedhorn/readq-lockin...
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Sat Apr 18 22:33:57 CDT 2009
Author: wedhorn
Date: Sat Apr 18 22:33:53 2009
New Revision: 189269
URL: http://svn.digium.com/svn-view/asterisk?view=rev&rev=189269
Log:
Initial commit of readq list locking. Includes the simpler fixes in chan's SIP IAX and DAHDI.
Modified:
team/wedhorn/readq-locking/channels/chan_dahdi.c
team/wedhorn/readq-locking/channels/chan_iax2.c
team/wedhorn/readq-locking/channels/chan_sip.c
team/wedhorn/readq-locking/include/asterisk/channel.h
team/wedhorn/readq-locking/main/channel.c
Modified: team/wedhorn/readq-locking/channels/chan_dahdi.c
URL: http://svn.digium.com/svn-view/asterisk/team/wedhorn/readq-locking/channels/chan_dahdi.c?view=diff&rev=189269&r1=189268&r2=189269
==============================================================================
--- team/wedhorn/readq-locking/channels/chan_dahdi.c (original)
+++ team/wedhorn/readq-locking/channels/chan_dahdi.c Sat Apr 18 22:33:53 2009
@@ -1539,89 +1539,16 @@
static void wakeup_sub(struct dahdi_pvt *p, int a, struct dahdi_pri *pri)
{
-#ifdef HAVE_PRI
- if (pri)
- ast_mutex_unlock(&pri->lock);
-#endif
- for (;;) {
- if (p->subs[a].owner) {
- if (ast_channel_trylock(p->subs[a].owner)) {
- DEADLOCK_AVOIDANCE(&p->lock);
- } else {
- ast_queue_frame(p->subs[a].owner, &ast_null_frame);
- ast_channel_unlock(p->subs[a].owner);
- break;
- }
- } else
- break;
- }
-#ifdef HAVE_PRI
- if (pri)
- ast_mutex_lock(&pri->lock);
-#endif
+ if (p->subs[a].owner) {
+ ast_queue_frame(p->subs[a].owner, &ast_null_frame);
+ }
}
static void dahdi_queue_frame(struct dahdi_pvt *p, struct ast_frame *f, void *data)
{
-#ifdef HAVE_PRI
- struct dahdi_pri *pri = (struct dahdi_pri*) data;
-#endif
-#ifdef HAVE_SS7
- struct dahdi_ss7 *ss7 = (struct dahdi_ss7*) data;
-#endif
- /* We must unlock the PRI to avoid the possibility of a deadlock */
-#if defined(HAVE_PRI) || defined(HAVE_SS7)
- if (data) {
- switch (p->sig) {
-#ifdef HAVE_PRI
- case SIG_BRI:
- case SIG_BRI_PTMP:
- case SIG_PRI:
- ast_mutex_unlock(&pri->lock);
- break;
-#endif
-#ifdef HAVE_SS7
- case SIG_SS7:
- ast_mutex_unlock(&ss7->lock);
- break;
-#endif
- default:
- break;
- }
- }
-#endif
- for (;;) {
- if (p->owner) {
- if (ast_channel_trylock(p->owner)) {
- DEADLOCK_AVOIDANCE(&p->lock);
- } else {
- ast_queue_frame(p->owner, f);
- ast_channel_unlock(p->owner);
- break;
- }
- } else
- break;
- }
-#if defined(HAVE_PRI) || defined(HAVE_SS7)
- if (data) {
- switch (p->sig) {
-#ifdef HAVE_PRI
- case SIG_BRI:
- case SIG_BRI_PTMP:
- case SIG_PRI:
- ast_mutex_lock(&pri->lock);
- break;
-#endif
-#ifdef HAVE_SS7
- case SIG_SS7:
- ast_mutex_lock(&ss7->lock);
- break;
-#endif
- default:
- break;
- }
- }
-#endif
+ if (p->owner) {
+ ast_queue_frame(p->owner, f);
+ }
}
static struct ast_channel *dahdi_new(struct dahdi_pvt *, int, int, int, int, int);
Modified: team/wedhorn/readq-locking/channels/chan_iax2.c
URL: http://svn.digium.com/svn-view/asterisk/team/wedhorn/readq-locking/channels/chan_iax2.c?view=diff&rev=189269&r1=189268&r2=189269
==============================================================================
--- team/wedhorn/readq-locking/channels/chan_iax2.c (original)
+++ team/wedhorn/readq-locking/channels/chan_iax2.c Sat Apr 18 22:33:53 2009
@@ -2012,27 +2012,15 @@
/*!
* \brief Queue a frame to a call's owning asterisk channel
*
- * \pre This function assumes that iaxsl[callno] is locked when called.
+ * \pre This function assumes that iaxsl[callno] is locked when called. No longer applicable.
*
- * \note IMPORTANT NOTE!!! Any time this function is used, even if iaxs[callno]
- * was valid before calling it, it may no longer be valid after calling it.
- * This function may unlock and lock the mutex associated with this callno,
- * meaning that another thread may grab it and destroy the call.
+ * \note IMPORTANT NOTE!!! Behaviour changed, iaxs[callno] will exist when this
+ * function ends assuming lock was held before this was called.
*/
static int iax2_queue_frame(int callno, struct ast_frame *f)
{
- for (;;) {
- if (iaxs[callno] && iaxs[callno]->owner) {
- if (ast_channel_trylock(iaxs[callno]->owner)) {
- /* Avoid deadlock by pausing and trying again */
- DEADLOCK_AVOIDANCE(&iaxsl[callno]);
- } else {
- ast_queue_frame(iaxs[callno]->owner, f);
- ast_channel_unlock(iaxs[callno]->owner);
- break;
- }
- } else
- break;
+ if (iaxs[callno] && iaxs[callno]->owner) {
+ ast_queue_frame(iaxs[callno]->owner, f);
}
return 0;
}
@@ -2054,6 +2042,8 @@
{
for (;;) {
if (iaxs[callno] && iaxs[callno]->owner) {
+ /* XXX Locking only required to set chan->_softhangup, not to queue frame */
+ /* wedhorn: planning on changing _softhangup to atomic at which stage we can remove this locking */
if (ast_channel_trylock(iaxs[callno]->owner)) {
/* Avoid deadlock by pausing and trying again */
DEADLOCK_AVOIDANCE(&iaxsl[callno]);
@@ -2074,28 +2064,16 @@
* This function queues a control frame on the owner of the IAX2 pvt struct that
* is active for the given call number.
*
- * \pre Assumes lock for callno is already held.
+ * \pre Assumes lock for callno is already held. No longer applicable.
*
- * \note IMPORTANT NOTE!!! Any time this function is used, even if iaxs[callno]
- * was valid before calling it, it may no longer be valid after calling it.
- * This function may unlock and lock the mutex associated with this callno,
- * meaning that another thread may grab it and destroy the call.
+ * \note IMPORTANT NOTE!!! Behaviour changed, iaxs[callno] will exist when this
+ * function ends assuming lock was held before this was called.
*/
static int iax2_queue_control_data(int callno,
enum ast_control_frame_type control, const void *data, size_t datalen)
{
- for (;;) {
- if (iaxs[callno] && iaxs[callno]->owner) {
- if (ast_channel_trylock(iaxs[callno]->owner)) {
- /* Avoid deadlock by pausing and trying again */
- DEADLOCK_AVOIDANCE(&iaxsl[callno]);
- } else {
- ast_queue_control_data(iaxs[callno]->owner, control, data, datalen);
- ast_channel_unlock(iaxs[callno]->owner);
- break;
- }
- } else
- break;
+ if (iaxs[callno] && iaxs[callno]->owner) {
+ ast_queue_control_data(iaxs[callno]->owner, control, data, datalen);
}
return 0;
}
Modified: team/wedhorn/readq-locking/channels/chan_sip.c
URL: http://svn.digium.com/svn-view/asterisk/team/wedhorn/readq-locking/channels/chan_sip.c?view=diff&rev=189269&r1=189268&r2=189269
==============================================================================
--- team/wedhorn/readq-locking/channels/chan_sip.c (original)
+++ team/wedhorn/readq-locking/channels/chan_sip.c Sat Apr 18 22:33:53 2009
@@ -5064,12 +5064,8 @@
sip_pvt_lock(p);
p->initid = -1; /* event gone, will not be rescheduled */
if (p->owner) {
- /* XXX fails on possible deadlock */
- if (!ast_channel_trylock(p->owner)) {
- append_history(p, "Cong", "Auto-congesting (timer)");
- ast_queue_control(p->owner, AST_CONTROL_CONGESTION);
- ast_channel_unlock(p->owner);
- }
+ append_history(p, "Cong", "Auto-congesting (timer)");
+ ast_queue_control(p->owner, AST_CONTROL_CONGESTION);
}
sip_pvt_unlock(p);
dialog_unref(p, "unreffing arg passed into auto_congest callback (p->initid)");
Modified: team/wedhorn/readq-locking/include/asterisk/channel.h
URL: http://svn.digium.com/svn-view/asterisk/team/wedhorn/readq-locking/include/asterisk/channel.h?view=diff&rev=189269&r1=189268&r2=189269
==============================================================================
--- team/wedhorn/readq-locking/include/asterisk/channel.h (original)
+++ team/wedhorn/readq-locking/include/asterisk/channel.h Sat Apr 18 22:33:53 2009
@@ -596,7 +596,7 @@
struct varshead varshead; /*!< A linked list for channel variables. See \ref AstChanVar */
ast_group_t callgroup; /*!< Call group for call pickups */
ast_group_t pickupgroup; /*!< Pickup group - which calls groups can be picked up? */
- AST_LIST_HEAD_NOLOCK(, ast_frame) readq;
+ AST_LIST_HEAD(, ast_frame) readq;
AST_LIST_ENTRY(ast_channel) chan_list; /*!< For easy linking */
struct ast_jb jb; /*!< The jitterbuffer state */
struct timeval dtmf_tv; /*!< The time that an in process digit began, or the last digit ended */
Modified: team/wedhorn/readq-locking/main/channel.c
URL: http://svn.digium.com/svn-view/asterisk/team/wedhorn/readq-locking/main/channel.c?view=diff&rev=189269&r1=189268&r2=189269
==============================================================================
--- team/wedhorn/readq-locking/main/channel.c (original)
+++ team/wedhorn/readq-locking/main/channel.c Sat Apr 18 22:33:53 2009
@@ -934,6 +934,8 @@
ast_mutex_init(&tmp->lock_dont_use);
+ AST_LIST_HEAD_INIT(&tmp->readq);
+
AST_LIST_HEAD_INIT_NOLOCK(&tmp->datastores);
ast_string_field_set(tmp, language, defaultlanguage);
@@ -988,12 +990,12 @@
return -1;
}
- ast_channel_lock(chan);
+ AST_LIST_LOCK(&chan->readq);
/* See if the last frame on the queue is a hangup, if so don't queue anything */
if ((cur = AST_LIST_LAST(&chan->readq)) && (cur->frametype == AST_FRAME_CONTROL) && (cur->subclass == AST_CONTROL_HANGUP)) {
ast_frfree(f);
- ast_channel_unlock(chan);
+ AST_LIST_UNLOCK(&chan->readq);
return 0;
}
@@ -1010,7 +1012,7 @@
} else {
ast_debug(1, "Dropping voice to exceptionally long queue on %s\n", chan->name);
ast_frfree(f);
- ast_channel_unlock(chan);
+ AST_LIST_UNLOCK(&chan->readq);
return 0;
}
}
@@ -1032,7 +1034,7 @@
pthread_kill(chan->blocker, SIGURG);
}
- ast_channel_unlock(chan);
+ AST_LIST_UNLOCK(&chan->readq);
return 0;
}
@@ -1682,8 +1684,10 @@
}
close(chan->epfd);
#endif
+ AST_LIST_LOCK(&chan->readq);
while ((f = AST_LIST_REMOVE_HEAD(&chan->readq, frame_list)))
ast_frfree(f);
+ AST_LIST_UNLOCK(&chan->readq);
/* loop over the variables list, freeing all data and deleting list items */
/* no need to lock the list, as the channel is already locked */
@@ -2834,6 +2838,8 @@
usleep(1);
}
+ AST_LIST_LOCK(&chan->readq);
+
if (chan->masq) {
if (ast_do_masquerade(chan))
ast_log(LOG_WARNING, "Failed to perform masquerade\n");
@@ -2896,11 +2902,13 @@
int (*func)(const void *) = chan->timingfunc;
void *data = chan->timingdata;
chan->fdno = -1;
+ AST_LIST_UNLOCK(&chan->readq);
ast_channel_unlock(chan);
func(data);
} else {
ast_timer_set_rate(chan->timer, 0);
chan->fdno = -1;
+ AST_LIST_UNLOCK(&chan->readq);
ast_channel_unlock(chan);
}
@@ -2996,6 +3004,7 @@
the channel driver and f would be only a single frame)
*/
if (AST_LIST_NEXT(f, frame_list)) {
+ /* Using AST_LIST_HEAD_SET_NOLOCK so we don't screw with the list lock */
AST_LIST_HEAD_SET_NOLOCK(&chan->readq, AST_LIST_NEXT(f, frame_list));
AST_LIST_NEXT(f, frame_list) = NULL;
}
@@ -3246,6 +3255,7 @@
if (chan->music_state && chan->generator && chan->generator->digit && f && f->frametype == AST_FRAME_DTMF_END)
chan->generator->digit(chan, f->subclass);
+ AST_LIST_UNLOCK(&chan->readq);
ast_channel_unlock(chan);
return f;
}
@@ -4541,6 +4551,8 @@
AST_LIST_HEAD_NOLOCK(, ast_frame) tmp_readq;
AST_LIST_HEAD_SET_NOLOCK(&tmp_readq, NULL);
+ AST_LIST_LOCK(&original->readq);
+
AST_LIST_APPEND_LIST(&tmp_readq, &original->readq, frame_list);
AST_LIST_APPEND_LIST(&original->readq, &clonechan->readq, frame_list);
@@ -4554,6 +4566,7 @@
}
}
}
+ AST_LIST_UNLOCK(&original->readq);
}
/* Swap the raw formats */
More information about the asterisk-commits
mailing list