[svn-commits] twilson: branch 10 r365083 - in /branches/10: ./ channels/ main/

SVN commits to the Digium repositories svn-commits at lists.digium.com
Wed May 2 12:30:01 CDT 2012


Author: twilson
Date: Wed May  2 12:29:54 2012
New Revision: 365083

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=365083
Log:
Multiple revisions 365006,365068

........
  r365006 | twilson | 2012-05-02 10:49:03 -0500 (Wed, 02 May 2012) | 12 lines
  
  Fix a CEL LINKEDID_END race and local channel linkedids
  
  This patch has the ;2 channel inherit the linkedid of the ;1 channel and fixes
  the race condition by no longer scanning the channel list for "other" channels
  with the same linkedid. Instead, cel.c has an ao2 container of linkedid strings
  and uses the refcount of the string as a counter of how many channels with the
  linkedid exist. Not only does this eliminate the race condition, but it also
  allows us to look up the linkedid by the hashed key instead of traversing the
  entire channel list.
  
  Review: https://reviewboard.asterisk.org/r/1895/
........
  r365068 | twilson | 2012-05-02 12:02:39 -0500 (Wed, 02 May 2012) | 11 lines
  
  Don't leak a ref if out of memory and can't link the linkedid
  
  If the ao2_link fails, we are most likely out of memory and bad things
  are going to happen. Before those bad things happen, make sure to clean
  up the linkedid references.
  
  This patch also adds a comment explaining why linkedid can't be passed
  to both local channel allocations and combines two ao2_ref calls into 1.
  
  Review: https://reviewboard.asterisk.org/r/1895/
........

Merged revisions 365006,365068 from http://svn.asterisk.org/svn/asterisk/branches/1.8

Modified:
    branches/10/   (props changed)
    branches/10/channels/chan_local.c
    branches/10/main/cel.c

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

Modified: branches/10/channels/chan_local.c
URL: http://svnview.digium.com/svn/asterisk/branches/10/channels/chan_local.c?view=diff&rev=365083&r1=365082&r2=365083
==============================================================================
--- branches/10/channels/chan_local.c (original)
+++ branches/10/channels/chan_local.c Wed May  2 12:29:54 2012
@@ -1119,8 +1119,11 @@
 		ama = p->owner->amaflags;
 	else
 		ama = 0;
+
+	/* Make sure that the ;2 channel gets the same linkedid as ;1. You can't pass linkedid to both
+	 * allocations since if linkedid isn't set, then each channel will generate its own linkedid. */
 	if (!(tmp = ast_channel_alloc(1, state, 0, 0, t, p->exten, p->context, linkedid, ama, "Local/%s@%s-%04x;1", p->exten, p->context, randnum)) 
-		|| !(tmp2 = ast_channel_alloc(1, AST_STATE_RING, 0, 0, t, p->exten, p->context, linkedid, ama, "Local/%s@%s-%04x;2", p->exten, p->context, randnum))) {
+		|| !(tmp2 = ast_channel_alloc(1, AST_STATE_RING, 0, 0, t, p->exten, p->context, tmp->linkedid, ama, "Local/%s@%s-%04x;2", p->exten, p->context, randnum))) {
 		if (tmp) {
 			tmp = ast_channel_release(tmp);
 		}

Modified: branches/10/main/cel.c
URL: http://svnview.digium.com/svn/asterisk/branches/10/main/cel.c?view=diff&rev=365083&r1=365082&r2=365083
==============================================================================
--- branches/10/main/cel.c (original)
+++ branches/10/main/cel.c Wed May  2 12:29:54 2012
@@ -80,6 +80,7 @@
  * for when they start and end on a channel.
  */
 static struct ao2_container *appset;
+static struct ao2_container *linkedids;
 
 /*!
  * \brief Configured date format for event timestamps
@@ -360,42 +361,29 @@
 
 /* called whenever a channel is destroyed or a linkedid is changed to
  * potentially emit a CEL_LINKEDID_END event */
-
-struct channel_find_data {
-	const struct ast_channel *chan;
-	const char *linkedid;
-};
-
-static int linkedid_match(void *obj, void *arg, void *data, int flags)
-{
-	struct ast_channel *c = obj;
-	struct channel_find_data *find_dat = data;
-	int res;
-
-	ast_channel_lock(c);
-	res = (c != find_dat->chan && c->linkedid && !strcmp(find_dat->linkedid, c->linkedid));
-	ast_channel_unlock(c);
-
-	return res ? CMP_MATCH | CMP_STOP : 0;
-}
-
 void ast_cel_check_retire_linkedid(struct ast_channel *chan)
 {
 	const char *linkedid = chan->linkedid;
-	struct channel_find_data find_dat;
+	char *lid;
 
 	/* make sure we need to do all this work */
 
-	if (!ast_strlen_zero(linkedid) && ast_cel_track_event(AST_CEL_LINKEDID_END)) {
-		struct ast_channel *tmp = NULL;
-		find_dat.chan = chan;
-		find_dat.linkedid = linkedid;
-		if ((tmp = ast_channel_callback(linkedid_match, NULL, &find_dat, 0))) {
-			tmp = ast_channel_unref(tmp);
-		} else {
-			ast_cel_report_event(chan, AST_CEL_LINKEDID_END, NULL, NULL, NULL);
-		}
-	}
+	if (ast_strlen_zero(linkedid) || !ast_cel_track_event(AST_CEL_LINKEDID_END)) {
+		return;
+	}
+
+	if (!(lid = ao2_find(linkedids, (void *) linkedid, OBJ_POINTER))) {
+		ast_log(LOG_ERROR, "Something weird happened, couldn't find linkedid %s\n", linkedid);
+		return;
+	}
+
+	/* We have a ref for each channel with this linkedid, the link and the above find, so if
+	 * before unreffing the channel we have a refcount of 3, we're done. Unlink and report. */
+	if (ao2_ref(lid, -1) == 3) {
+		ao2_unlink(linkedids, lid);
+		ast_cel_report_event(chan, AST_CEL_LINKEDID_END, NULL, NULL, NULL);
+	}
+	ao2_ref(lid, -1);
 }
 
 struct ast_channel *ast_cel_fabricate_channel_from_event(const struct ast_event *event)
@@ -484,39 +472,54 @@
 	const char *peername = "";
 	struct ast_channel *peer;
 
+	/* Make sure a reload is not occurring while we're checking to see if this
+	 * is an event that we care about.  We could lose an important event in this
+	 * process otherwise. */
+	ast_mutex_lock(&reload_lock);
+
+	/* Record the linkedid of new channels if we are tracking LINKEDID_END even if we aren't
+	 * reporting on CHANNEL_START so we can track when to send LINKEDID_END */
+	if (cel_enabled && ast_cel_track_event(AST_CEL_LINKEDID_END) && event_type == AST_CEL_CHANNEL_START && chan->linkedid) {
+		char *lid;
+		if (!(lid = ao2_find(linkedids, (void *) chan->linkedid, OBJ_POINTER))) {
+			if (!(lid = ao2_alloc(strlen(chan->linkedid) + 1, NULL))) {
+				ast_mutex_unlock(&reload_lock);
+				return -1;
+			}
+			strcpy(lid, chan->linkedid);
+			if (!ao2_link(linkedids, lid)) {
+				ao2_ref(lid, -1);
+				ast_mutex_unlock(&reload_lock);
+				return -1;
+			}
+			/* Leave both the link and the alloc refs to show a count of 1 + the link */
+		}
+		/* If we've found, go ahead and keep the ref to increment count of how many channels
+		 * have this linkedid. We'll clean it up in check_retire */
+	}
+
+	if (!cel_enabled || !ast_cel_track_event(event_type)) {
+		ast_mutex_unlock(&reload_lock);
+		return 0;
+	}
+
+	if (event_type == AST_CEL_APP_START || event_type == AST_CEL_APP_END) {
+		char *app;
+		if (!(app = ao2_find(appset, (char *) chan->appl, OBJ_POINTER))) {
+			ast_mutex_unlock(&reload_lock);
+			return 0;
+		}
+		ao2_ref(app, -1);
+	}
+
+	ast_mutex_unlock(&reload_lock);
+
 	ast_channel_lock(chan);
 	peer = ast_bridged_channel(chan);
 	if (peer) {
 		ast_channel_ref(peer);
 	}
 	ast_channel_unlock(chan);
-
-	/* Make sure a reload is not occurring while we're checking to see if this
-	 * is an event that we care about.  We could lose an important event in this
-	 * process otherwise. */
-	ast_mutex_lock(&reload_lock);
-
-	if (!cel_enabled || !ast_cel_track_event(event_type)) {
-		ast_mutex_unlock(&reload_lock);
-		if (peer) {
-			ast_channel_unref(peer);
-		}
-		return 0;
-	}
-
-	if (event_type == AST_CEL_APP_START || event_type == AST_CEL_APP_END) {
-		char *app;
-		if (!(app = ao2_find(appset, (char *) chan->appl, OBJ_POINTER))) {
-			ast_mutex_unlock(&reload_lock);
-			if (peer) {
-				ast_channel_unref(peer);
-			}
-			return 0;
-		}
-		ao2_ref(app, -1);
-	}
-
-	ast_mutex_unlock(&reload_lock);
 
 	if (peer) {
 		ast_channel_lock(peer);
@@ -641,6 +644,9 @@
 	return !strcasecmp(app1, app2) ? CMP_MATCH | CMP_STOP : 0;
 }
 
+#define lid_hash app_hash
+#define lid_cmp app_cmp
+
 static void ast_cel_engine_term(void)
 {
 	if (appset) {
@@ -654,19 +660,19 @@
 	if (!(appset = ao2_container_alloc(NUM_APP_BUCKETS, app_hash, app_cmp))) {
 		return -1;
 	}
-
-	if (do_reload()) {
+	if (!(linkedids = ao2_container_alloc(NUM_APP_BUCKETS, lid_hash, lid_cmp))) {
+		ao2_ref(appset, -1);
+		return -1;
+	}
+
+	if (do_reload() || ast_cli_register(&cli_status)) {
 		ao2_ref(appset, -1);
 		appset = NULL;
+		ao2_ref(linkedids, -1);
+		linkedids = NULL;
 		return -1;
 	}
 
-	if (ast_cli_register(&cli_status)) {
-		ao2_ref(appset, -1);
-		appset = NULL;
-		return -1;
-	}
-
 	ast_register_atexit(ast_cel_engine_term);
 
 	return 0;




More information about the svn-commits mailing list