[svn-commits] russell: branch 1.4 r88805 - in /branches/1.4: include/asterisk/pbx.h main/pbx.c
SVN commits to the Digium repositories
svn-commits at lists.digium.com
Mon Nov 5 16:07:55 CST 2007
Author: russell
Date: Mon Nov 5 16:07:54 2007
New Revision: 88805
URL: http://svn.digium.com/view/asterisk?view=rev&rev=88805
Log:
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:
branches/1.4/include/asterisk/pbx.h
branches/1.4/main/pbx.c
Modified: branches/1.4/include/asterisk/pbx.h
URL: http://svn.digium.com/view/asterisk/branches/1.4/include/asterisk/pbx.h?view=diff&rev=88805&r1=88804&r2=88805
==============================================================================
--- branches/1.4/include/asterisk/pbx.h (original)
+++ branches/1.4/include/asterisk/pbx.h Mon Nov 5 16:07:54 2007
@@ -764,13 +764,37 @@
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, char *buf, size_t size);
+
+/*!
+ * \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);
+
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: branches/1.4/main/pbx.c
URL: http://svn.digium.com/view/asterisk/branches/1.4/main/pbx.c?view=diff&rev=88805&r1=88804&r2=88805
==============================================================================
--- branches/1.4/main/pbx.c (original)
+++ branches/1.4/main/pbx.c Mon Nov 5 16:07:54 2007
@@ -1135,6 +1135,7 @@
struct varshead *places[2] = { headp, &globals }; /* list of places where we may look */
if (c) {
+ ast_channel_lock(c);
places[0] = &c->varshead;
}
/*
@@ -1232,6 +1233,9 @@
if (need_substring)
*ret = substring(*ret, offset, length, workspace, workspacelen);
}
+
+ if (c)
+ ast_channel_unlock(c);
}
/*! \brief CLI function to show installed custom functions
@@ -5722,6 +5726,8 @@
memset(buf, 0, size);
+ 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) */
@@ -5735,6 +5741,8 @@
break;
}
+ ast_channel_unlock(chan);
+
return total;
}
@@ -5747,8 +5755,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])
@@ -5767,6 +5778,9 @@
break;
}
+ if (chan)
+ ast_channel_unlock(chan);
+
return ret;
}
@@ -5783,18 +5797,25 @@
return;
}
- headp = (chan) ? &chan->varshead : &globals;
+ if (chan) {
+ ast_channel_lock(chan);
+ headp = &chan->varshead;
+ } else {
+ ast_mutex_lock(&globalslock);
+ headp = &globals;
+ }
if (value) {
if ((option_verbose > 1) && (headp == &globals))
ast_verbose(VERBOSE_PREFIX_2 "Setting global variable '%s' to '%s'\n", name, value);
newvariable = ast_var_assign(name, value);
- if (headp == &globals)
- ast_mutex_lock(&globalslock);
AST_LIST_INSERT_HEAD(headp, newvariable, entries);
- if (headp == &globals)
- ast_mutex_unlock(&globalslock);
- }
+ }
+
+ if (chan)
+ ast_channel_unlock(chan);
+ else
+ ast_mutex_unlock(&globalslock);
}
void pbx_builtin_setvar_helper(struct ast_channel *chan, const char *name, const char *value)
@@ -5803,7 +5824,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);
@@ -5811,7 +5831,13 @@
return;
}
- headp = (chan) ? &chan->varshead : &globals;
+ if (chan) {
+ ast_channel_lock(chan);
+ headp = &chan->varshead;
+ } else {
+ ast_mutex_lock(&globalslock);
+ headp = &globals;
+ }
/* For comparison purposes, we have to strip leading underscores */
if (*nametail == '_') {
@@ -5820,8 +5846,6 @@
nametail++;
}
- if (headp == &globals)
- ast_mutex_lock(&globalslock);
AST_LIST_TRAVERSE (headp, newvariable, entries) {
if (strcasecmp(ast_var_name(newvariable), nametail) == 0) {
/* there is already such a variable, delete it */
@@ -5838,7 +5862,9 @@
AST_LIST_INSERT_HEAD(headp, newvariable, entries);
}
- if (headp == &globals)
+ if (chan)
+ ast_channel_unlock(chan);
+ else
ast_mutex_unlock(&globalslock);
}
More information about the svn-commits
mailing list