[asterisk-commits] russell: trunk r88934 - in /trunk: ./ channels/ include/asterisk/ main/

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Tue Nov 6 08:08:54 CST 2007


Author: russell
Date: Tue Nov  6 08:08:54 2007
New Revision: 88934

URL: http://svn.digium.com/view/asterisk?view=rev&rev=88934
Log:
Merged revisions 88805 via svnmerge from 
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r88805 | russell | 2007-11-05 16:07:54 -0600 (Mon, 05 Nov 2007) | 12 lines

After seeing crashes related to channel variables, I went looking around at the
ways that channel variables are handled.  In general, they were not handled in
a thread-safe way.  The channel _must_ be locked when reading or writing from/to
the channel variable list.

What I have done to improve this situation is to make pbx_builtin_setvar_helper()
and friends lock the channel when doing their thing.  Asterisk API calls almost 
all lock the channel for you as necessary, but this family of functions did not.

(closes issue #10923, reported by atis)
(closes issue #11159, reported by 850t)

........

Modified:
    trunk/   (props changed)
    trunk/channels/busy.h
    trunk/channels/ringtone.h
    trunk/include/asterisk/pbx.h
    trunk/main/pbx.c

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

Modified: trunk/channels/busy.h
URL: http://svn.digium.com/view/asterisk/trunk/channels/busy.h?view=diff&rev=88934&r1=88933&r2=88934
==============================================================================
--- trunk/channels/busy.h (original)
+++ trunk/channels/busy.h Tue Nov  6 08:08:54 2007
@@ -1,21 +1,3 @@
-/*
- * Asterisk -- An open source telephony toolkit.
- *
- * Copyright (C) 1999 - 2007, Digium, Inc.
- *
- * Mark Spencer <markster at digium.com>
- *
- * See http://www.asterisk.org for more information about
- * the Asterisk project. Please do not directly contact
- * any of the maintainers of this project for assistance;
- * the project provides a web site, mailing lists and IRC
- * channels for your use.
- *
- * This program is free software, distributed under the terms of
- * the GNU General Public License Version 2. See the LICENSE file
- * at the top of the source tree.
- */
-
 /* busy.h: Generated from frequencies 480 and 620 
    by gentone.  400 samples  */
 static short busy[400] = {

Modified: trunk/channels/ringtone.h
URL: http://svn.digium.com/view/asterisk/trunk/channels/ringtone.h?view=diff&rev=88934&r1=88933&r2=88934
==============================================================================
--- trunk/channels/ringtone.h (original)
+++ trunk/channels/ringtone.h Tue Nov  6 08:08:54 2007
@@ -1,21 +1,3 @@
-/*
- * Asterisk -- An open source telephony toolkit.
- *
- * Copyright (C) 1999 - 2007, Digium, Inc.
- *
- * Mark Spencer <markster at digium.com>
- *
- * See http://www.asterisk.org for more information about
- * the Asterisk project. Please do not directly contact
- * any of the maintainers of this project for assistance;
- * the project provides a web site, mailing lists and IRC
- * channels for your use.
- *
- * This program is free software, distributed under the terms of
- * the GNU General Public License Version 2. See the LICENSE file
- * at the top of the source tree.
- */
-
 /* ringtone.h: Generated from frequencies 440 and 480 
    by gentone.  200 samples  */
 static short ringtone[200] = {

Modified: trunk/include/asterisk/pbx.h
URL: http://svn.digium.com/view/asterisk/trunk/include/asterisk/pbx.h?view=diff&rev=88934&r1=88933&r2=88934
==============================================================================
--- trunk/include/asterisk/pbx.h (original)
+++ trunk/include/asterisk/pbx.h Tue Nov  6 08:08:54 2007
@@ -850,14 +850,39 @@
 	struct ast_ignorepat *ip);
 struct ast_sw *ast_walk_context_switches(struct ast_context *con, struct ast_sw *sw);
 
+/*!
+ * \note Will lock the channel.
+ */
 int pbx_builtin_serialize_variables(struct ast_channel *chan, struct ast_str **buf);
+
+/*!
+ * \note Will lock the channel.
+ */
 const char *pbx_builtin_getvar_helper(struct ast_channel *chan, const char *name);
+
+/*!
+ * \note Will lock the channel.
+ */
 void pbx_builtin_pushvar_helper(struct ast_channel *chan, const char *name, const char *value);
+
+/*!
+ * \note Will lock the channel.
+ */
 void pbx_builtin_setvar_helper(struct ast_channel *chan, const char *name, const char *value);
+
+/*!
+ * \note Will lock the channel.
+ */
 void pbx_retrieve_variable(struct ast_channel *c, const char *var, char **ret, char *workspace, int workspacelen, struct varshead *headp);
 void pbx_builtin_clear_globals(void);
+
+/*!
+ * \note Will lock the channel.
+ */
 int pbx_builtin_setvar(struct ast_channel *chan, void *data);
+
 int pbx_builtin_raise_exception(struct ast_channel *chan, void *data);
+
 void pbx_substitute_variables_helper(struct ast_channel *c,const char *cp1,char *cp2,int count);
 void pbx_substitute_variables_varshead(struct varshead *headp, const char *cp1, char *cp2, int count);
 

Modified: trunk/main/pbx.c
URL: http://svn.digium.com/view/asterisk/trunk/main/pbx.c?view=diff&rev=88934&r1=88933&r2=88934
==============================================================================
--- trunk/main/pbx.c (original)
+++ trunk/main/pbx.c Tue Nov  6 08:08:54 2007
@@ -1187,6 +1187,7 @@
 	struct varshead *places[2] = { headp, &globals };	/* list of places where we may look */
 
 	if (c) {
+		ast_channel_lock(c);
 		places[0] = &c->varshead;
 	}
 	/*
@@ -1284,6 +1285,9 @@
 		if (need_substring)
 			*ret = substring(*ret, offset, length, workspace, workspacelen);
 	}
+
+	if (c)
+		ast_channel_unlock(c);
 }
 
 static void exception_store_free(void *data)
@@ -5913,6 +5917,8 @@
 	(*buf)->used = 0;
 	(*buf)->str[0] = '\0';
 
+	ast_channel_lock(chan);
+
 	AST_LIST_TRAVERSE(&chan->varshead, variables, entries) {
 		if ((var = ast_var_name(variables)) && (val = ast_var_value(variables))
 		   /* && !ast_strlen_zero(var) && !ast_strlen_zero(val) */
@@ -5926,6 +5932,8 @@
 			break;
 	}
 
+	ast_channel_unlock(chan);
+
 	return total;
 }
 
@@ -5938,8 +5946,11 @@
 
 	if (!name)
 		return NULL;
-	if (chan)
+
+	if (chan) {
+		ast_channel_lock(chan);
 		places[0] = &chan->varshead;
+	}
 
 	for (i = 0; i < 2; i++) {
 		if (!places[i])
@@ -5958,6 +5969,9 @@
 			break;
 	}
 
+	if (chan)
+		ast_channel_unlock(chan);
+
 	return ret;
 }
 
@@ -5974,18 +5988,25 @@
 		return;
 	}
 
-	headp = (chan) ? &chan->varshead : &globals;
+	if (chan) {
+		ast_channel_lock(chan);
+		headp = &chan->varshead;
+	} else {
+		ast_rwlock_wrlock(&globalslock);
+		headp = &globals;
+	}
 
 	if (value) {
 		if (headp == &globals)
 			ast_verb(2, "Setting global variable '%s' to '%s'\n", name, value);
 		newvariable = ast_var_assign(name, value);
-		if (headp == &globals)
-			ast_rwlock_wrlock(&globalslock);
 		AST_LIST_INSERT_HEAD(headp, newvariable, entries);
-		if (headp == &globals)
-			ast_rwlock_unlock(&globalslock);
-	}
+	}
+
+	if (chan)
+		ast_channel_unlock(chan);
+	else
+		ast_rwlock_unlock(&globalslock);
 }
 
 void pbx_builtin_setvar_helper(struct ast_channel *chan, const char *name, const char *value)
@@ -5994,7 +6015,6 @@
 	struct varshead *headp;
 	const char *nametail = name;
 
-	/* XXX may need locking on the channel ? */
 	if (name[strlen(name) - 1] == ')') {
 		char *function = ast_strdupa(name);
 
@@ -6002,7 +6022,13 @@
 		return;
 	}
 
-	headp = (chan) ? &chan->varshead : &globals;
+	if (chan) {
+		ast_channel_lock(chan);
+		headp = &chan->varshead;
+	} else {
+		ast_rwlock_wrlock(&globalslock);
+		headp = &globals;
+	}
 
 	/* For comparison purposes, we have to strip leading underscores */
 	if (*nametail == '_') {
@@ -6011,8 +6037,6 @@
 			nametail++;
 	}
 
-	if (headp == &globals)
-		ast_rwlock_wrlock(&globalslock);
 	AST_LIST_TRAVERSE (headp, newvariable, entries) {
 		if (strcasecmp(ast_var_name(newvariable), nametail) == 0) {
 			/* there is already such a variable, delete it */
@@ -6036,7 +6060,9 @@
 			chan ? chan->uniqueid : "none");
 	}
 
-	if (headp == &globals)
+	if (chan)
+		ast_channel_unlock(chan);
+	else
 		ast_rwlock_unlock(&globalslock);
 }
 




More information about the asterisk-commits mailing list