[svn-commits] dvossel: trunk r292869 - in /trunk: ./ channels/chan_local.c

SVN commits to the Digium repositories svn-commits at lists.digium.com
Mon Oct 25 14:11:46 CDT 2010


Author: dvossel
Date: Mon Oct 25 14:11:42 2010
New Revision: 292869

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=292869
Log:
Merged revisions 292868 via svnmerge from 
https://origsvn.digium.com/svn/asterisk/branches/1.8

................
  r292868 | dvossel | 2010-10-25 14:07:50 -0500 (Mon, 25 Oct 2010) | 39 lines
  
  Merged revisions 292867 via svnmerge from 
  https://origsvn.digium.com/svn/asterisk/branches/1.6.2
  
  ................
    r292867 | dvossel | 2010-10-25 14:06:21 -0500 (Mon, 25 Oct 2010) | 32 lines
    
    Merged revisions 292866 via svnmerge from 
    https://origsvn.digium.com/svn/asterisk/branches/1.4
    
    ........
      r292866 | dvossel | 2010-10-25 14:05:07 -0500 (Mon, 25 Oct 2010) | 27 lines
      
      This patch turns chan_local pvts into astobj2 objects.
      
      chan_local does some dangerous things involving deadlock avoidance.
      tech_pvt functions like hangup and queue_frame are provided with a
      locked channel upon entry.  Those functions are completely safe as
      long as you don't attempt to give up that channel lock, but that is
      impossible to guarantee due to the required deadlock avoidance necessary
      to lock both the tech_pvt and both channels involved.
      
      In the past, we have tried to account for this by doing things like
      setting a "glare" flag that indicates what function should destroy the
      pvt.  This was used in local_hangup and local_queue_frame to decided
      who should destroy the pvt if they collided in separate threads.  I
      have removed the need to do this by converting all chan_local tech_pvts
      to astobj2.  This means we can ref a pvt before deadlock avoidance
      and not have to worry about that pvt possibly getting destroyed under
      us.  It also cleans up where we destroy the tech_pvt.  The only unlink
      from the tech_pvt container occurs in local_hangup now, which is where
      it should occur.
      
      Since there still may be thread collisions on some functions like
      local_hangup after deadlock avoidance, I have added some checks to detect
      those collisions and exit appropriately.  I think this patch is going to
      solve quite a bit of weirdness we have had with local channels in the past.
    ........
  ................
................

Modified:
    trunk/   (props changed)
    trunk/channels/chan_local.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-1.8-merged' - no diff available.

Modified: trunk/channels/chan_local.c
URL: http://svnview.digium.com/svn/asterisk/trunk/channels/chan_local.c?view=diff&rev=292869&r1=292868&r2=292869
==============================================================================
--- trunk/channels/chan_local.c (original)
+++ trunk/channels/chan_local.c Mon Oct 25 14:11:42 2010
@@ -48,6 +48,7 @@
 #include "asterisk/manager.h"
 #include "asterisk/stringfields.h"
 #include "asterisk/devicestate.h"
+#include "asterisk/astobj2.h"
 
 /*** DOCUMENTATION
 	<manager name="LocalOptimizeAway" language="en_US">
@@ -71,6 +72,13 @@
 static const char tdesc[] = "Local Proxy Channel Driver";
 
 #define IS_OUTBOUND(a,b) (a == b->chan ? 1 : 0)
+
+/* right now we are treating the locals astobj2 container as a
+ * list.  If there is ever a reason to make this more efficient
+ * increasing the bucket size would help. */
+static const int BUCKET_SIZE = 1;
+
+static struct ao2_container *locals;
 
 static struct ast_jb_conf g_jb_conf = {
 	.flags = 0,
@@ -130,7 +138,6 @@
 
 */
 struct local_pvt {
-	ast_mutex_t lock;			/*!< Channel private lock */
 	unsigned int flags;                     /*!< Private flags */
 	char context[AST_MAX_CONTEXT];		/*!< Context to call */
 	char exten[AST_MAX_EXTENSION];		/*!< Extension to call */
@@ -140,18 +147,13 @@
 	struct ast_channel *chan;		/*!< Outbound channel - PBX is run here */
 	struct ast_module_user *u_owner;	/*!< reference to keep the module loaded while in use */
 	struct ast_module_user *u_chan;		/*!< reference to keep the module loaded while in use */
-	AST_LIST_ENTRY(local_pvt) list;		/*!< Next entity */
 };
 
-#define LOCAL_GLARE_DETECT    (1 << 0) /*!< Detect glare on hangup */
-#define LOCAL_CANCEL_QUEUE    (1 << 1) /*!< Cancel queue */
-#define LOCAL_ALREADY_MASQED  (1 << 2) /*!< Already masqueraded */
-#define LOCAL_LAUNCHED_PBX    (1 << 3) /*!< PBX was launched */
-#define LOCAL_NO_OPTIMIZATION (1 << 4) /*!< Do not optimize using masquerading */
-#define LOCAL_BRIDGE          (1 << 5) /*!< Report back the "true" channel as being bridged to */
-#define LOCAL_MOH_PASSTHRU    (1 << 6) /*!< Pass through music on hold start/stop frames */
-
-static AST_LIST_HEAD_STATIC(locals, local_pvt);
+#define LOCAL_ALREADY_MASQED  (1 << 0) /*!< Already masqueraded */
+#define LOCAL_LAUNCHED_PBX    (1 << 1) /*!< PBX was launched */
+#define LOCAL_NO_OPTIMIZATION (1 << 2) /*!< Do not optimize using masquerading */
+#define LOCAL_BRIDGE          (1 << 3) /*!< Report back the "true" channel as being bridged to */
+#define LOCAL_MOH_PASSTHRU    (1 << 4) /*!< Pass through music on hold start/stop frames */
 
 static int local_setoption(struct ast_channel *chan, int option, void * data, int datalen)
 {
@@ -182,7 +184,7 @@
 		return -1;
 	}
 
-	while (ast_mutex_trylock(&p->lock)) {
+	while (ao2_trylock(p)) {
 		ast_channel_unlock(chan);
 		sched_yield();
 		ast_channel_lock(chan);
@@ -197,14 +199,14 @@
 	otherchan = (write_info->chan == p->owner) ? p->chan : p->owner;
 
 	if (!otherchan || otherchan == write_info->chan) {
-		ast_mutex_unlock(&p->lock);
+		ao2_unlock(p);
 		ast_channel_unlock(chan);
 		ast_log(LOG_WARNING, "Could not update other side of %s, other side went away.\n", chan->name);
 		return 0;
 	}
 
 	if (ast_channel_trylock(otherchan)) {
-		ast_mutex_unlock(&p->lock);
+		ao2_unlock(p);
 		ast_channel_unlock(chan);
 		goto startover;
 	}
@@ -212,7 +214,7 @@
 	res = write_info->write_fn(otherchan, write_info->function, write_info->data, write_info->value);
 
 	ast_channel_unlock(otherchan);
-	ast_mutex_unlock(&p->lock);
+	ao2_unlock(p);
 	ast_channel_unlock(chan);
 
 	return res;
@@ -225,6 +227,7 @@
 	char *context = NULL, *opts = NULL;
 	int res;
 	struct local_pvt *lp;
+	struct ao2_iterator it;
 
 	if (!(context = strchr(exten, '@'))) {
 		ast_log(LOG_WARNING, "Someone used Local/%s somewhere without a @context. This is bad.\n", exten);
@@ -244,26 +247,19 @@
 		return AST_DEVICE_INVALID;
 	
 	res = AST_DEVICE_NOT_INUSE;
-	AST_LIST_LOCK(&locals);
-	AST_LIST_TRAVERSE(&locals, lp, list) {
+
+	it = ao2_iterator_init(locals, 0);
+	while ((lp = ao2_iterator_next(&it))) {
 		if (!strcmp(exten, lp->exten) && !strcmp(context, lp->context) && lp->owner) {
 			res = AST_DEVICE_INUSE;
+			ao2_ref(lp, -1);
 			break;
 		}
-	}
-	AST_LIST_UNLOCK(&locals);
+		ao2_ref(lp, -1);
+	}
+	ao2_iterator_destroy(&it);
 
 	return res;
-}
-
-/*!
- * \note Assumes the pvt is no longer in the pvts list
- */
-static struct local_pvt *local_pvt_destroy(struct local_pvt *pvt)
-{
-	ast_mutex_destroy(&pvt->lock);
-	ast_free(pvt);
-	return NULL;
 }
 
 /*! \brief Return the bridged channel of a Local channel */
@@ -278,7 +274,7 @@
 		return NULL;
 	}
 
-	ast_mutex_lock(&p->lock);
+	ao2_lock(p);
 
 	if (ast_test_flag(p, LOCAL_BRIDGE)) {
 		/* Find the opposite channel */
@@ -292,7 +288,7 @@
 		}
 	}
 
-	ast_mutex_unlock(&p->lock);
+	ao2_unlock(p);
 
 	return bridged;
 }
@@ -312,17 +308,19 @@
 		return -1;
 	}
 
-	ast_mutex_lock(&p->lock);
+	ao2_lock(p);
 	chan = IS_OUTBOUND(ast, p) ? p->owner : p->chan;
 
 try_again:
 	if (!chan) {
-		ast_mutex_unlock(&p->lock);
+		ao2_unlock(p);
 		return -1;
 	}
 
 	if (ast_channel_trylock(chan)) {
-		DEADLOCK_AVOIDANCE(&p->lock);
+		ao2_unlock(p);
+		sched_yield();
+		ao2_lock(p);
 		chan = IS_OUTBOUND(ast, p) ? p->owner : p->chan;
 		goto try_again;
 	}
@@ -330,25 +328,35 @@
 	bridged = ast_bridged_channel(chan);
 	if (!bridged) {
 		/* can't query channel unless we are bridged */
-		ast_mutex_unlock(&p->lock);
+		ao2_unlock(p);
 		ast_channel_unlock(chan);
 		return -1;
 	}
 
 	if (ast_channel_trylock(bridged)) {
 		ast_channel_unlock(chan);
-		DEADLOCK_AVOIDANCE(&p->lock);
+		ao2_unlock(p);
+		sched_yield();
+		ao2_lock(p);
 		chan = IS_OUTBOUND(ast, p) ? p->owner : p->chan;
 		goto try_again;
 	}
 
 	res = ast_channel_queryoption(bridged, option, data, datalen, 0);
-	ast_mutex_unlock(&p->lock);
+	ao2_unlock(p);
 	ast_channel_unlock(chan);
 	ast_channel_unlock(bridged);
 	return res;
 }
 
+/*! \brief queue a frame on a to either the p->owner or p->chan
+ *
+ * \note the local_pvt MUST have it's ref count bumped before entering this function and
+ * decremented after this function is called.  This is a side effect of the deadlock
+ * avoidance that is necessary to lock 2 channels and a tech_pvt.  Without a ref counted
+ * local_pvt, it is impossible to guarantee it will not be destroyed by another thread
+ * during deadlock avoidance.
+ */
 static int local_queue_frame(struct local_pvt *p, int isoutbound, struct ast_frame *f, 
 	struct ast_channel *us, int us_locked)
 {
@@ -366,39 +374,22 @@
 		return 0;
 	}
 
-	/* Set glare detection */
-	ast_set_flag(p, LOCAL_GLARE_DETECT);
-
 	/* Ensure that we have both channels locked */
 	while (other && ast_channel_trylock(other)) {
 		int res;
-		if ((res = ast_mutex_unlock(&p->lock))) {
+		if ((res = ao2_unlock(p))) {
 			ast_log(LOG_ERROR, "chan_local bug! '&p->lock' was not locked when entering local_queue_frame! (%s)\n", strerror(res));
 			return -1;
 		}
 		if (us && us_locked) {
 			do {
 				CHANNEL_DEADLOCK_AVOIDANCE(us);
-			} while (ast_mutex_trylock(&p->lock));
+			} while (ao2_trylock(p));
 		} else {
 			usleep(1);
-			ast_mutex_lock(&p->lock);
+			ao2_lock(p);
 		}
 		other = isoutbound ? p->owner : p->chan;
-	}
-
-	/* Since glare detection only occurs within this function, and because
-	 * a pvt flag cannot be set without having the pvt lock, this is the only
-	 * location where we could detect a cancelling of the queue. */
-	if (ast_test_flag(p, LOCAL_CANCEL_QUEUE)) {
-		/* We had a glare on the hangup.  Forget all this business,
-		return and destroy p.  */
-		ast_mutex_unlock(&p->lock);
-		p = local_pvt_destroy(p);
-		if (other) {
-			ast_channel_unlock(other);
-		}
-		return -1;
 	}
 
 	if (other) {
@@ -409,8 +400,6 @@
 		ast_channel_unlock(other);
 	}
 
-	ast_clear_flag(p, LOCAL_GLARE_DETECT);
-
 	return 0;
 }
 
@@ -423,16 +412,18 @@
 	if (!p)
 		return -1;
 
-	ast_mutex_lock(&p->lock);
+	ao2_lock(p);
+	ao2_ref(p, 1);
 	isoutbound = IS_OUTBOUND(ast, p);
 	if (isoutbound) {
 		/* Pass along answer since somebody answered us */
 		struct ast_frame answer = { AST_FRAME_CONTROL, { AST_CONTROL_ANSWER } };
 		res = local_queue_frame(p, isoutbound, &answer, ast, 1);
-	} else
+	} else {
 		ast_log(LOG_WARNING, "Huh?  Local is being asked to answer?\n");
-	if (!res)
-		ast_mutex_unlock(&p->lock);
+	}
+	ao2_unlock(p);
+	ao2_ref(p, -1);
 	return res;
 }
 
@@ -535,7 +526,8 @@
 		return -1;
 
 	/* Just queue for delivery to the other side */
-	ast_mutex_lock(&p->lock);
+	ao2_lock(p);
+	ao2_ref(p, 1); /* ref for local_queue_frame */
 	isoutbound = IS_OUTBOUND(ast, p);
 	if (isoutbound && f && (f->frametype == AST_FRAME_VOICE || f->frametype == AST_FRAME_VIDEO))
 		check_bridge(p);
@@ -545,8 +537,9 @@
 		ast_debug(1, "Not posting to queue since already masked on '%s'\n", ast->name);
 		res = 0;
 	}
-	if (!res)
-		ast_mutex_unlock(&p->lock);
+	ao2_unlock(p);
+	ao2_ref(p, -1);
+
 	return res;
 }
 
@@ -557,11 +550,11 @@
 	if (!p)
 		return -1;
 
-	ast_mutex_lock(&p->lock);
+	ao2_lock(p);
 
 	if ((p->owner != oldchan) && (p->chan != oldchan)) {
 		ast_log(LOG_WARNING, "Old channel wasn't %p but was %p/%p\n", oldchan, p->owner, p->chan);
-		ast_mutex_unlock(&p->lock);
+		ao2_unlock(p);
 		return -1;
 	}
 	if (p->owner == oldchan)
@@ -572,12 +565,12 @@
 	/* Do not let a masquerade cause a Local channel to be bridged to itself! */
 	if (!ast_check_hangup(newchan) && (p->owner->_bridge == p->chan || p->chan->_bridge == p->owner)) {
 		ast_log(LOG_WARNING, "You can not bridge a Local channel to itself!\n");
-		ast_mutex_unlock(&p->lock);
+		ao2_unlock(p);
 		ast_queue_hangup(newchan);
 		return -1;
 	}
 
-	ast_mutex_unlock(&p->lock);
+	ao2_unlock(p);
 	return 0;
 }
 
@@ -590,6 +583,8 @@
 
 	if (!p)
 		return -1;
+
+	ao2_ref(p, 1); /* ref for local_queue_frame */
 
 	/* If this is an MOH hold or unhold, do it on the Local channel versus real channel */
 	if (!ast_test_flag(p, LOCAL_MOH_PASSTHRU) && condition == AST_CONTROL_HOLD) {
@@ -606,7 +601,7 @@
 		 * we need to transmit the collected connected line information instead of whatever
 		 * happens to be in this control frame. The same applies for redirecting information, which
 		 * is why it is handled here as well.*/
-		ast_mutex_lock(&p->lock);
+		ao2_lock(p);
 		isoutbound = IS_OUTBOUND(ast, p);
 		if (isoutbound) {
 			this_channel = p->chan;
@@ -627,23 +622,21 @@
 			}
 			f.subclass.integer = condition;
 			f.data.ptr = frame_data;
-			if (!(res = local_queue_frame(p, isoutbound, &f, ast, 1))) {
-				ast_mutex_unlock(&p->lock);
-			}
-		} else {
-			ast_mutex_unlock(&p->lock);
-		}
+			res = local_queue_frame(p, isoutbound, &f, ast, 1);
+		}
+		ao2_unlock(p);
 	} else {
 		/* Queue up a frame representing the indication as a control frame */
-		ast_mutex_lock(&p->lock);
+		ao2_lock(p);
 		isoutbound = IS_OUTBOUND(ast, p);
 		f.subclass.integer = condition;
 		f.data.ptr = (void*)data;
 		f.datalen = datalen;
-		if (!(res = local_queue_frame(p, isoutbound, &f, ast, 1)))
-			ast_mutex_unlock(&p->lock);
-	}
-
+		res = local_queue_frame(p, isoutbound, &f, ast, 1);
+		ao2_unlock(p);
+	}
+
+	ao2_ref(p, -1);
 	return res;
 }
 
@@ -657,11 +650,13 @@
 	if (!p)
 		return -1;
 
-	ast_mutex_lock(&p->lock);
+	ao2_ref(p, 1); /* ref for local_queue_frame */
+	ao2_lock(p);
 	isoutbound = IS_OUTBOUND(ast, p);
 	f.subclass.integer = digit;
-	if (!(res = local_queue_frame(p, isoutbound, &f, ast, 0)))
-		ast_mutex_unlock(&p->lock);
+	res = local_queue_frame(p, isoutbound, &f, ast, 0);
+	ao2_unlock(p);
+	ao2_ref(p, -1);
 
 	return res;
 }
@@ -676,12 +671,14 @@
 	if (!p)
 		return -1;
 
-	ast_mutex_lock(&p->lock);
+	ao2_ref(p, 1); /* ref for local_queue_frame */
+	ao2_lock(p);
 	isoutbound = IS_OUTBOUND(ast, p);
 	f.subclass.integer = digit;
 	f.len = duration;
-	if (!(res = local_queue_frame(p, isoutbound, &f, ast, 0)))
-		ast_mutex_unlock(&p->lock);
+	res = local_queue_frame(p, isoutbound, &f, ast, 0);
+	ao2_unlock(p);
+	ao2_ref(p, -1);
 
 	return res;
 }
@@ -696,12 +693,14 @@
 	if (!p)
 		return -1;
 
-	ast_mutex_lock(&p->lock);
+	ao2_lock(p);
+	ao2_ref(p, 1); /* ref for local_queue_frame */
 	isoutbound = IS_OUTBOUND(ast, p);
 	f.data.ptr = (char *) text;
 	f.datalen = strlen(text) + 1;
-	if (!(res = local_queue_frame(p, isoutbound, &f, ast, 0)))
-		ast_mutex_unlock(&p->lock);
+	res = local_queue_frame(p, isoutbound, &f, ast, 0);
+	ao2_unlock(p);
+	ao2_ref(p, -1);
 	return res;
 }
 
@@ -714,14 +713,17 @@
 
 	if (!p)
 		return -1;
-	
-	ast_mutex_lock(&p->lock);
+
+	ao2_lock(p);
+	ao2_ref(p, 1); /* ref for local_queue_frame */
 	isoutbound = IS_OUTBOUND(ast, p);
 	f.subclass.integer = subclass;
 	f.data.ptr = (char *)data;
 	f.datalen = datalen;
-	if (!(res = local_queue_frame(p, isoutbound, &f, ast, 0)))
-		ast_mutex_unlock(&p->lock);
+	res = local_queue_frame(p, isoutbound, &f, ast, 0);
+	ao2_unlock(p);
+	ao2_ref(p, -1);
+
 	return res;
 }
 
@@ -736,25 +738,40 @@
 	char *reduced_dest = ast_strdupa(dest);
 	char *slash;
 
-	if (!p)
-		return -1;
-
-	/* If you value your sanity, please don't look at this code */
-start_over:
-	while (ast_channel_trylock(p->chan)) {
-		ast_channel_unlock(p->owner);
-		usleep(1);
-		ast_channel_lock(p->owner);
-	}
-
-	/* p->owner and p->chan are locked now. Let's get p locked */
-	if (ast_mutex_trylock(&p->lock)) {
-		/* @#$&$@ */
-		ast_channel_unlock(p->chan);
-		ast_channel_unlock(p->owner);
-		usleep(1);
-		ast_channel_lock(p->owner);
-		goto start_over;
+	if (!p || p->owner != ast) {
+		return -1;
+	}
+
+	/* since we are letting go of channel locks that were locked coming into
+	 * this function, then we need to give the tech pvt a ref */
+	ao2_ref(p, 1);
+
+	while (ao2_trylock(p)) {
+		ast_channel_unlock(ast);
+		sched_yield();
+		ast_channel_lock(ast);
+	}
+	while ((p->chan && p->owner) && ast_channel_trylock(p->chan)) {
+		ao2_unlock(p);
+		if (p->owner) {
+			ast_channel_unlock(p->owner);
+		}
+		sched_yield();
+		if (p->owner) {
+			ast_channel_lock(p->owner);
+		}
+		ao2_lock(p);
+	}
+
+	if (!p->owner || !p->chan) {
+		/* someone went away during the locking madness.
+		 * time to bail. */
+		if (p->chan) {
+			ast_channel_unlock(p->chan);
+		}
+		ao2_unlock(p);
+		ao2_ref(p,-1);
+		return -1;
 	}
 
 	/*
@@ -781,8 +798,9 @@
 	if (!ast_exists_extension(NULL, p->chan->context, p->chan->exten, 1,
 		S_COR(p->owner->caller.id.number.valid, p->owner->caller.id.number.str, NULL))) {
 		ast_log(LOG_NOTICE, "No such extension/context %s@%s while calling Local channel\n", p->chan->exten, p->chan->context);
-		ast_mutex_unlock(&p->lock);
+		ao2_unlock(p);
 		ast_channel_unlock(p->chan);
+		ao2_ref(p, -1);
 		return -1;
 	}
 
@@ -816,8 +834,9 @@
 	if (!(res = ast_pbx_start(p->chan)))
 		ast_set_flag(p, LOCAL_LAUNCHED_PBX);
 
-	ast_mutex_unlock(&p->lock);
+	ao2_unlock(p);
 	ast_channel_unlock(p->chan);
+	ao2_ref(p, -1);
 	return res;
 }
 
@@ -828,12 +847,15 @@
 	int isoutbound;
 	struct ast_frame f = { AST_FRAME_CONTROL, { AST_CONTROL_HANGUP }, .data.uint32 = ast->hangupcause };
 	struct ast_channel *ochan = NULL;
-	int glaredetect = 0, res = 0;
 
 	if (!p)
 		return -1;
 
-	ast_mutex_lock(&p->lock);
+	/* we MUST give the tech_pvt a ref here since we are unlocking the
+	 * channel during deadlock avoidance. */
+	ao2_ref(p, 1);
+
+	ao2_lock(p);
 
 	isoutbound = IS_OUTBOUND(ast, p);
 
@@ -847,20 +869,30 @@
 		if ((status) && (p->owner)) {
 			/* Deadlock avoidance */
 			while (p->owner && ast_channel_trylock(p->owner)) {
-				ast_mutex_unlock(&p->lock);
+				ao2_unlock(p);
 				if (p->chan) {
 					ast_channel_unlock(p->chan);
 				}
-				usleep(1);
+				sched_yield();
 				if (p->chan) {
 					ast_channel_lock(p->chan);
 				}
-				ast_mutex_lock(&p->lock);
+				ao2_lock(p);
 			}
 			if (p->owner) {
 				pbx_builtin_setvar_helper(p->owner, "CHANLOCALSTATUS", status);
 				ast_channel_unlock(p->owner);
 			}
+		}
+		if (!p->chan) {
+			/* chan was == to ast and was !NULL before deadlock avoidance started, if chan
+			 * is NULL now, then we should bail because that channel
+			 * hungup already. This is possible because we let go of the
+			 * lock given to the ast channel passed to this function during
+			 * deadlock avoidance. */
+			ao2_unlock(p);
+			ao2_ref(p, -1);
+			return 0;
 		}
 		p->chan = NULL;
 		ast_clear_flag(p, LOCAL_LAUNCHED_PBX);
@@ -868,53 +900,57 @@
 	} else {
 		ast_module_user_remove(p->u_owner);
 		while (p->chan && ast_channel_trylock(p->chan)) {
-				ast_mutex_unlock(&p->lock);
+				ao2_unlock(p);
 				if (p->owner) {
 					ast_channel_unlock(p->owner);
 				}
-				usleep(1);
+				sched_yield();
 				if (p->owner) {
 					ast_channel_lock(p->owner);
 				}
-				ast_mutex_lock(&p->lock);
-		}
-
-		p->owner = NULL;
+				ao2_lock(p);
+		}
 		if (p->chan) {
 			ast_queue_hangup(p->chan);
 			ast_channel_unlock(p->chan);
 		}
-	}
-	
+
+		if (!p->owner) {
+			/* owner was == to ast and was !NULL before deadlock avoidance started, if
+			 * owner is NULL now, then we should bail because that channel
+			 * hungup already. This is possible because we let go of the
+			 * lock given to the ast channel passed to this function during
+			 * deadlock avoidance. */
+			ao2_unlock(p);
+			ao2_ref(p, -1);
+			return 0;
+		}
+		p->owner = NULL;
+	}
+
 	ast->tech_pvt = NULL;
-	
+
 	if (!p->owner && !p->chan) {
-		/* Okay, done with the private part now, too. */
-		glaredetect = ast_test_flag(p, LOCAL_GLARE_DETECT);
-		/* If we have a queue holding, don't actually destroy p yet, but
-		   let local_queue do it. */
-		if (glaredetect)
-			ast_set_flag(p, LOCAL_CANCEL_QUEUE);
+		ao2_unlock(p);
+
 		/* Remove from list */
-		AST_LIST_LOCK(&locals);
-		AST_LIST_REMOVE(&locals, p, list);
-		AST_LIST_UNLOCK(&locals);
-		ast_mutex_unlock(&p->lock);
-		/* And destroy */
-		if (!glaredetect) {
-			p = local_pvt_destroy(p);
-		}
+		ao2_unlink(locals, p);
+		ao2_ref(p, -1);
 		return 0;
 	}
-	if (p->chan && !ast_test_flag(p, LOCAL_LAUNCHED_PBX))
+	if (p->chan && !ast_test_flag(p, LOCAL_LAUNCHED_PBX)) {
 		/* Need to actually hangup since there is no PBX */
 		ochan = p->chan;
-	else
-		res = local_queue_frame(p, isoutbound, &f, NULL, 1);
-	if (!res)
-		ast_mutex_unlock(&p->lock);
-	if (ochan)
+	} else {
+		local_queue_frame(p, isoutbound, &f, NULL, 1);
+	}
+
+	ao2_unlock(p);
+	if (ochan) {
 		ast_hangup(ochan);
+	}
+
+	ao2_ref(p, -1);
 	return 0;
 }
 
@@ -924,11 +960,11 @@
 	struct local_pvt *tmp = NULL;
 	char *c = NULL, *opts = NULL;
 
-	if (!(tmp = ast_calloc(1, sizeof(*tmp))))
+	if (!(tmp = ao2_alloc(sizeof(*tmp), NULL))) {
 		return NULL;
+	}
 
 	/* Initialize private structure information */
-	ast_mutex_init(&tmp->lock);
 	ast_copy_string(tmp->exten, data, sizeof(tmp->exten));
 
 	memcpy(&tmp->jb_conf, &g_jb_conf, sizeof(tmp->jb_conf));
@@ -972,14 +1008,11 @@
 	} else {
 #endif
 		/* Add to list */
-		AST_LIST_LOCK(&locals);
-		AST_LIST_INSERT_HEAD(&locals, tmp, list);
-		AST_LIST_UNLOCK(&locals);
+		ao2_link(locals, tmp);
 #if 0
 	}
 #endif
-	
-	return tmp;
+	return tmp; /* this is returned with a ref */
 }
 
 /*! \brief Start new local channel */
@@ -1054,15 +1087,13 @@
 	/* Allocate a new private structure and then Asterisk channel */
 	if ((p = local_alloc(data, format))) {
 		if (!(chan = local_new(p, AST_STATE_DOWN, requestor ? requestor->linkedid : NULL))) {
-			AST_LIST_LOCK(&locals);
-			AST_LIST_REMOVE(&locals, p, list);
-			AST_LIST_UNLOCK(&locals);
-			p = local_pvt_destroy(p);
+			ao2_unlink(locals, p);
 		}
 		if (chan && ast_channel_cc_params_init(chan, requestor ? ast_channel_get_cc_config_params((struct ast_channel *)requestor) : NULL)) {
 			chan = ast_channel_release(chan);
-			p = local_pvt_destroy(p);
-		}
+			ao2_unlink(locals, p);
+		}
+		ao2_ref(p, -1); /* kill the ref from the alloc */
 	}
 
 	return chan;
@@ -1072,6 +1103,7 @@
 static char *locals_show(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)
 {
 	struct local_pvt *p = NULL;
+	struct ao2_iterator it;
 
 	switch (cmd) {
 	case CLI_INIT:
@@ -1087,16 +1119,19 @@
 	if (a->argc != 3)
 		return CLI_SHOWUSAGE;
 
-	AST_LIST_LOCK(&locals);
-	if (!AST_LIST_EMPTY(&locals)) {
-		AST_LIST_TRAVERSE(&locals, p, list) {
-			ast_mutex_lock(&p->lock);
-			ast_cli(a->fd, "%s -- %s@%s\n", p->owner ? p->owner->name : "<unowned>", p->exten, p->context);
-			ast_mutex_unlock(&p->lock);
-		}
-	} else
+	if (ao2_container_count(locals) == 0) {
 		ast_cli(a->fd, "No local channels in use\n");
-	AST_LIST_UNLOCK(&locals);
+		return RESULT_SUCCESS;
+	}
+
+	it = ao2_iterator_init(locals, 0);
+	while ((p = ao2_iterator_next(&it))) {
+		ao2_lock(p);
+		ast_cli(a->fd, "%s -- %s@%s\n", p->owner ? p->owner->name : "<unowned>", p->exten, p->context);
+		ao2_unlock(p);
+		ao2_ref(p, -1);
+	}
+	ao2_iterator_destroy(&it);
 
 	return CLI_SUCCESS;
 }
@@ -1111,6 +1146,7 @@
 	struct local_pvt *p, *tmp = NULL;
 	struct ast_channel *c;
 	int found = 0;
+	struct ao2_iterator it;
 
 	channel = astman_get_header(m, "Channel");
 
@@ -1129,22 +1165,19 @@
 	ast_channel_unref(c);
 	c = NULL;
 
-	if (AST_LIST_LOCK(&locals)) {
-		astman_send_error(s, m, "Unable to lock the monitor");
-		return 0;
-	}
-
-
-	AST_LIST_TRAVERSE(&locals, tmp, list) {
+	it = ao2_iterator_init(locals, 0);
+	while ((tmp = ao2_iterator_next(&it))) {
 		if (tmp == p) {
-			ast_mutex_lock(&tmp->lock);
+			ao2_lock(tmp);
 			found = 1;
 			ast_clear_flag(tmp, LOCAL_NO_OPTIMIZATION);
-			ast_mutex_unlock(&tmp->lock);
+			ao2_unlock(tmp);
+			ao2_ref(tmp, -1);
 			break;
 		}
-	}
-	AST_LIST_UNLOCK(&locals);
+		ao2_ref(tmp, -1);
+	}
+	ao2_iterator_destroy(&it);
 
 	if (found) {
 		astman_send_ack(s, m, "Queued channel to be optimized away");
@@ -1156,12 +1189,22 @@
 }
 
 
+static int locals_cmp_cb(void *obj, void *arg, int flags)
+{
+	return (obj == arg) ? CMP_MATCH : 0;
+}
+
 /*! \brief Load module into PBX, register channel */
 static int load_module(void)
 {
+	if (!(locals = ao2_container_alloc(BUCKET_SIZE, NULL, locals_cmp_cb))) {
+		return AST_MODULE_LOAD_FAILURE;
+	}
+
 	/* Make sure we can register our channel type */
 	if (ast_channel_register(&local_tech)) {
 		ast_log(LOG_ERROR, "Unable to register channel class 'Local'\n");
+		ao2_ref(locals, -1);
 		return AST_MODULE_LOAD_FAILURE;
 	}
 	ast_cli_register_multiple(cli_local, sizeof(cli_local) / sizeof(struct ast_cli_entry));
@@ -1174,22 +1217,23 @@
 static int unload_module(void)
 {
 	struct local_pvt *p = NULL;
+	struct ao2_iterator it;
 
 	/* First, take us out of the channel loop */
 	ast_cli_unregister_multiple(cli_local, sizeof(cli_local) / sizeof(struct ast_cli_entry));
 	ast_manager_unregister("LocalOptimizeAway");
 	ast_channel_unregister(&local_tech);
-	if (!AST_LIST_LOCK(&locals)) {
-		/* Hangup all interfaces if they have an owner */
-		AST_LIST_TRAVERSE(&locals, p, list) {
-			if (p->owner)
-				ast_softhangup(p->owner, AST_SOFTHANGUP_APPUNLOAD);
-		}
-		AST_LIST_UNLOCK(&locals);
-	} else {
-		ast_log(LOG_WARNING, "Unable to lock the monitor\n");
-		return -1;
-	}		
+
+	it = ao2_iterator_init(locals, 0);
+	while ((p = ao2_iterator_next(&it))) {
+		if (p->owner) {
+			ast_softhangup(p->owner, AST_SOFTHANGUP_APPUNLOAD);
+		}
+		ao2_ref(p, -1);
+	}
+	ao2_iterator_destroy(&it);
+	ao2_ref(locals, -1);
+
 	return 0;
 }
 




More information about the svn-commits mailing list