[asterisk-commits] rmudgett: branch 1.8 r348362 - in /branches/1.8: apps/ funcs/ include/asteris...
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Fri Dec 16 14:55:22 CST 2011
Author: rmudgett
Date: Fri Dec 16 14:55:17 2011
New Revision: 348362
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=348362
Log:
Fix crash during CDR update.
The ast_cdr_setcid() and ast_cdr_update() were shown in ASTERISK-18836 to
be called by different threads for the same channel. The channel driver
thread and the PBX thread running dialplan.
* Add lock protection around CDR API calls that access an ast_channel
pointer.
(closes issue ASTERISK-18836)
Reported by: gpluser
Review: https://reviewboard.asterisk.org/r/1628/
Modified:
branches/1.8/apps/app_authenticate.c
branches/1.8/apps/app_followme.c
branches/1.8/apps/app_queue.c
branches/1.8/funcs/func_cdr.c
branches/1.8/include/asterisk/cdr.h
branches/1.8/main/channel.c
branches/1.8/main/features.c
branches/1.8/main/pbx.c
branches/1.8/res/res_monitor.c
Modified: branches/1.8/apps/app_authenticate.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/apps/app_authenticate.c?view=diff&rev=348362&r1=348361&r2=348362
==============================================================================
--- branches/1.8/apps/app_authenticate.c (original)
+++ branches/1.8/apps/app_authenticate.c Fri Dec 16 14:55:17 2011
@@ -213,14 +213,20 @@
continue;
ast_md5_hash(md5passwd, passwd);
if (!strcmp(md5passwd, md5secret)) {
- if (ast_test_flag(&flags,OPT_ACCOUNT))
+ if (ast_test_flag(&flags,OPT_ACCOUNT)) {
+ ast_channel_lock(chan);
ast_cdr_setaccount(chan, buf);
+ ast_channel_unlock(chan);
+ }
break;
}
} else {
if (!strcmp(passwd, buf)) {
- if (ast_test_flag(&flags, OPT_ACCOUNT))
+ if (ast_test_flag(&flags, OPT_ACCOUNT)) {
+ ast_channel_lock(chan);
ast_cdr_setaccount(chan, buf);
+ ast_channel_unlock(chan);
+ }
break;
}
}
@@ -242,8 +248,11 @@
}
if ((retries < 3) && !res) {
- if (ast_test_flag(&flags,OPT_ACCOUNT) && !ast_test_flag(&flags,OPT_MULTIPLE))
+ if (ast_test_flag(&flags,OPT_ACCOUNT) && !ast_test_flag(&flags,OPT_MULTIPLE)) {
+ ast_channel_lock(chan);
ast_cdr_setaccount(chan, passwd);
+ ast_channel_unlock(chan);
+ }
if (!(res = ast_streamfile(chan, "auth-thankyou", chan->language)))
res = ast_waitstream(chan, "");
} else {
Modified: branches/1.8/apps/app_followme.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/apps/app_followme.c?view=diff&rev=348362&r1=348361&r2=348362
==============================================================================
--- branches/1.8/apps/app_followme.c (original)
+++ branches/1.8/apps/app_followme.c Fri Dec 16 14:55:17 2011
@@ -478,10 +478,12 @@
if (tmpuser && tmpuser->ochan && tmpuser->state >= 0) {
outbound = tmpuser->ochan;
+ ast_channel_lock(outbound);
if (!outbound->cdr) {
outbound->cdr = ast_cdr_alloc();
- if (outbound->cdr)
+ if (outbound->cdr) {
ast_cdr_init(outbound->cdr, outbound);
+ }
}
if (outbound->cdr) {
char tmp[256];
@@ -492,11 +494,15 @@
ast_cdr_start(outbound->cdr);
ast_cdr_end(outbound->cdr);
/* If the cause wasn't handled properly */
- if (ast_cdr_disposition(outbound->cdr, outbound->hangupcause))
+ if (ast_cdr_disposition(outbound->cdr, outbound->hangupcause)) {
ast_cdr_failed(outbound->cdr);
- } else
+ }
+ } else {
ast_log(LOG_WARNING, "Unable to create Call Detail Record\n");
- ast_hangup(tmpuser->ochan);
+ }
+ ast_channel_unlock(outbound);
+ ast_hangup(outbound);
+ tmpuser->ochan = NULL;
}
}
@@ -876,6 +882,7 @@
AST_LIST_INSERT_TAIL(findme_user_list, tmpuser, entry);
} else {
ast_verb(3, "couldn't reach at this number.\n");
+ ast_channel_lock(outbound);
if (!outbound->cdr) {
outbound->cdr = ast_cdr_alloc();
}
@@ -895,6 +902,7 @@
} else {
ast_log(LOG_ERROR, "Unable to create Call Detail Record\n");
}
+ ast_channel_unlock(outbound);
ast_hangup(outbound);
ast_free(tmpuser);
}
Modified: branches/1.8/apps/app_queue.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/apps/app_queue.c?view=diff&rev=348362&r1=348361&r2=348362
==============================================================================
--- branches/1.8/apps/app_queue.c (original)
+++ branches/1.8/apps/app_queue.c Fri Dec 16 14:55:17 2011
@@ -4988,9 +4988,9 @@
if ((strcasecmp(cdr->uniqueid, qe->chan->uniqueid)) &&
(strcasecmp(cdr->linkedid, qe->chan->uniqueid)) &&
(newcdr = ast_cdr_dup(cdr))) {
+ ast_channel_lock(qe->chan);
ast_cdr_init(newcdr, qe->chan);
ast_cdr_reset(newcdr, 0);
- ast_channel_lock(qe->chan);
cdr = ast_cdr_append(cdr, newcdr);
cdr = cdr->next;
ast_channel_unlock(qe->chan);
Modified: branches/1.8/funcs/func_cdr.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/funcs/func_cdr.c?view=diff&rev=348362&r1=348361&r2=348362
==============================================================================
--- branches/1.8/funcs/func_cdr.c (original)
+++ branches/1.8/funcs/func_cdr.c Fri Dec 16 14:55:17 2011
@@ -197,17 +197,21 @@
{
char *ret;
struct ast_flags flags = { 0 };
- struct ast_cdr *cdr = chan ? chan->cdr : NULL;
+ struct ast_cdr *cdr;
AST_DECLARE_APP_ARGS(args,
AST_APP_ARG(variable);
AST_APP_ARG(options);
);
- if (ast_strlen_zero(parse))
+ if (ast_strlen_zero(parse) || !chan)
return -1;
- if (!cdr)
+ ast_channel_lock(chan);
+ cdr = chan->cdr;
+ if (!cdr) {
+ ast_channel_unlock(chan);
return -1;
+ }
AST_STANDARD_APP_ARGS(args, parse);
@@ -255,13 +259,14 @@
ast_test_flag(&flags, OPT_UNPARSED));
}
+ ast_channel_unlock(chan);
return ret ? 0 : -1;
}
static int cdr_write(struct ast_channel *chan, const char *cmd, char *parse,
const char *value)
{
- struct ast_cdr *cdr = chan ? chan->cdr : NULL;
+ struct ast_cdr *cdr;
struct ast_flags flags = { 0 };
AST_DECLARE_APP_ARGS(args,
AST_APP_ARG(variable);
@@ -271,8 +276,12 @@
if (ast_strlen_zero(parse) || !value || !chan)
return -1;
- if (!cdr)
+ ast_channel_lock(chan);
+ cdr = chan->cdr;
+ if (!cdr) {
+ ast_channel_unlock(chan);
return -1;
+ }
AST_STANDARD_APP_ARGS(args, parse);
@@ -296,6 +305,7 @@
/* No need to worry about the u flag, as all fields for which setting
* 'u' would do anything are marked as readonly. */
+ ast_channel_unlock(chan);
return 0;
}
Modified: branches/1.8/include/asterisk/cdr.h
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/include/asterisk/cdr.h?view=diff&rev=348362&r1=348361&r2=348362
==============================================================================
--- branches/1.8/include/asterisk/cdr.h (original)
+++ branches/1.8/include/asterisk/cdr.h Fri Dec 16 14:55:17 2011
@@ -211,6 +211,7 @@
* \param cdr Call Detail Record to use for channel
* \param chan Channel to bind CDR with
* Initializes a CDR and associates it with a particular channel
+ * \note The channel should be locked before calling.
* \return 0 by default
*/
int ast_cdr_init(struct ast_cdr *cdr, struct ast_channel *chan);
@@ -220,6 +221,7 @@
* \param cdr Call Detail Record to use for channel
* \param chan Channel to bind CDR with
* Initializes a CDR and associates it with a particular channel
+ * \note The channel should be locked before calling.
* \return 0 by default
*/
int ast_cdr_setcid(struct ast_cdr *cdr, struct ast_channel *chan);
@@ -403,22 +405,40 @@
*/
void ast_cdr_merge(struct ast_cdr *to, struct ast_cdr *from);
-/*! \brief Set account code, will generate AMI event */
+/*!
+ * \brief Set account code, will generate AMI event
+ * \note The channel should be locked before calling.
+ */
int ast_cdr_setaccount(struct ast_channel *chan, const char *account);
-/*! \brief Set the peer account */
+/*!
+ * \brief Set the peer account
+ * \note The channel should be locked before calling.
+ */
int ast_cdr_setpeeraccount(struct ast_channel *chan, const char *account);
-/*! \brief Set AMA flags for channel */
+/*!
+ * \brief Set AMA flags for channel
+ * \note The channel should be locked before calling.
+ */
int ast_cdr_setamaflags(struct ast_channel *chan, const char *amaflags);
-/*! \brief Set CDR user field for channel (stored in CDR) */
+/*!
+ * \brief Set CDR user field for channel (stored in CDR)
+ * \note The channel should be locked before calling.
+ */
int ast_cdr_setuserfield(struct ast_channel *chan, const char *userfield);
-/*! \brief Append to CDR user field for channel (stored in CDR) */
+/*!
+ * \brief Append to CDR user field for channel (stored in CDR)
+ * \note The channel should be locked before calling.
+ */
int ast_cdr_appenduserfield(struct ast_channel *chan, const char *userfield);
-/*! Update CDR on a channel */
+/*!
+ * \brief Update CDR on a channel
+ * \note The channel should be locked before calling.
+ */
int ast_cdr_update(struct ast_channel *chan);
Modified: branches/1.8/main/channel.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/main/channel.c?view=diff&rev=348362&r1=348361&r2=348362
==============================================================================
--- branches/1.8/main/channel.c (original)
+++ branches/1.8/main/channel.c Fri Dec 16 14:55:17 2011
@@ -5242,7 +5242,9 @@
ast_channel_datastore_inherit(oh->parent_channel, new);
}
if (oh->account) {
+ ast_channel_lock(new);
ast_cdr_setaccount(new, oh->account);
+ ast_channel_lock(new);
}
} else if (caller) { /* no outgoing helper so use caller if avaliable */
ast_channel_update_redirecting(caller, apr, NULL);
@@ -5308,8 +5310,11 @@
ast_channel_inherit_variables(oh->parent_channel, chan);
ast_channel_datastore_inherit(oh->parent_channel, chan);
}
- if (oh->account)
- ast_cdr_setaccount(chan, oh->account);
+ if (oh->account) {
+ ast_channel_lock(chan);
+ ast_cdr_setaccount(chan, oh->account);
+ ast_channel_unlock(chan);
+ }
}
ast_set_callerid(chan, cid_num, cid_name, cid_num);
@@ -5421,21 +5426,27 @@
*outstate = AST_CONTROL_ANSWER;
if (res <= 0) {
- if ( AST_CONTROL_RINGING == last_subclass )
+ ast_channel_lock(chan);
+ if (AST_CONTROL_RINGING == last_subclass) {
chan->hangupcause = AST_CAUSE_NO_ANSWER;
- if (!chan->cdr && (chan->cdr = ast_cdr_alloc()))
+ }
+ if (!chan->cdr && (chan->cdr = ast_cdr_alloc())) {
ast_cdr_init(chan->cdr, chan);
+ }
if (chan->cdr) {
char tmp[256];
+
snprintf(tmp, sizeof(tmp), "%s/%s", type, (char *)data);
- ast_cdr_setapp(chan->cdr,"Dial",tmp);
+ ast_cdr_setapp(chan->cdr, "Dial", tmp);
ast_cdr_update(chan);
ast_cdr_start(chan->cdr);
ast_cdr_end(chan->cdr);
/* If the cause wasn't handled properly */
- if (ast_cdr_disposition(chan->cdr,chan->hangupcause))
+ if (ast_cdr_disposition(chan->cdr, chan->hangupcause)) {
ast_cdr_failed(chan->cdr);
- }
+ }
+ }
+ ast_channel_unlock(chan);
ast_hangup(chan);
chan = NULL;
}
Modified: branches/1.8/main/features.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/main/features.c?view=diff&rev=348362&r1=348361&r2=348362
==============================================================================
--- branches/1.8/main/features.c (original)
+++ branches/1.8/main/features.c Fri Dec 16 14:55:17 2011
@@ -2303,6 +2303,7 @@
pbx_builtin_setvar_helper(transferer, "BLINDTRANSFER", transferee->name);
pbx_builtin_setvar_helper(transferee, "BLINDTRANSFER", transferer->name);
finishup(transferee);
+ ast_channel_lock(transferer);
if (!transferer->cdr) { /* this code should never get called (in a perfect world) */
transferer->cdr = ast_cdr_alloc();
if (transferer->cdr) {
@@ -2310,6 +2311,7 @@
ast_cdr_start(transferer->cdr);
}
}
+ ast_channel_unlock(transferer);
if (transferer->cdr) {
struct ast_cdr *swap = transferer->cdr;
@@ -3897,11 +3899,15 @@
/* copy the userfield from the B-leg to A-leg if applicable */
if (chan->cdr && peer->cdr && !ast_strlen_zero(peer->cdr->userfield)) {
char tmp[256];
+
+ ast_channel_lock(chan);
if (!ast_strlen_zero(chan->cdr->userfield)) {
snprintf(tmp, sizeof(tmp), "%s;%s", chan->cdr->userfield, peer->cdr->userfield);
ast_cdr_appenduserfield(chan, tmp);
- } else
+ } else {
ast_cdr_setuserfield(chan, peer->cdr->userfield);
+ }
+ ast_channel_unlock(chan);
/* Don't delete the CDR; just disable it. */
ast_set_flag(peer->cdr, AST_CDR_FLAG_POST_DISABLED);
we_disabled_peer_cdr = 1;
@@ -3910,7 +3916,7 @@
ast_copy_string(orig_peername,peer->name,sizeof(orig_peername));
if (!chan_cdr || (chan_cdr && !ast_test_flag(chan_cdr, AST_CDR_FLAG_POST_DISABLED))) {
-
+ ast_channel_lock_both(chan, peer);
if (chan_cdr) {
ast_set_flag(chan_cdr, AST_CDR_FLAG_MAIN);
ast_cdr_update(chan);
@@ -3925,7 +3931,6 @@
ast_copy_string(bridge_cdr->userfield, peer_cdr->userfield, sizeof(bridge_cdr->userfield));
}
ast_cdr_setaccount(peer, chan->accountcode);
-
} else {
/* better yet, in a xfer situation, find out why the chan cdr got zapped (pun unintentional) */
bridge_cdr = ast_cdr_alloc(); /* this should be really, really rare/impossible? */
@@ -3948,6 +3953,9 @@
ast_cdr_start(bridge_cdr);
}
}
+ ast_channel_unlock(chan);
+ ast_channel_unlock(peer);
+
ast_debug(4,"bridge answer set, chan answer set\n");
/* peer_cdr->answer will be set when a macro runs on the peer;
in that case, the bridge answer will be delayed while the
Modified: branches/1.8/main/pbx.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/main/pbx.c?view=diff&rev=348362&r1=348361&r2=348362
==============================================================================
--- branches/1.8/main/pbx.c (original)
+++ branches/1.8/main/pbx.c Fri Dec 16 14:55:17 2011
@@ -4952,10 +4952,12 @@
ast_copy_string(c->context, "default", sizeof(c->context));
}
}
+ ast_channel_lock(c);
if (c->cdr) {
/* allow CDR variables that have been collected after channel was created to be visible during call */
ast_cdr_update(c);
}
+ ast_channel_unlock(c);
for (;;) {
char dst_exten[256]; /* buffer to accumulate digits */
int pos = 0; /* XXX should check bounds */
@@ -5062,8 +5064,11 @@
}
/* Call timed out with no special extension to jump to. */
}
- if (c->cdr)
+ ast_channel_lock(c);
+ if (c->cdr) {
ast_cdr_update(c);
+ }
+ ast_channel_unlock(c);
error = 1;
break;
}
@@ -5167,10 +5172,12 @@
}
}
}
+ ast_channel_lock(c);
if (c->cdr) {
ast_verb(2, "CDR updated on %s\n",c->name);
ast_cdr_update(c);
}
+ ast_channel_unlock(c);
}
}
@@ -9409,7 +9416,9 @@
static int pbx_builtin_setamaflags(struct ast_channel *chan, const char *data)
{
/* Copy the AMA Flags as specified */
+ ast_channel_lock(chan);
ast_cdr_setamaflags(chan, data ? data : "");
+ ast_channel_unlock(chan);
return 0;
}
Modified: branches/1.8/res/res_monitor.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/res/res_monitor.c?view=diff&rev=348362&r1=348361&r2=348362
==============================================================================
--- branches/1.8/res/res_monitor.c (original)
+++ branches/1.8/res/res_monitor.c Fri Dec 16 14:55:17 2011
@@ -684,9 +684,13 @@
if (urlprefix) {
snprintf(tmp, sizeof(tmp), "%s/%s.%s", urlprefix, args.fname_base,
((strcmp(args.format, "gsm")) ? "wav" : "gsm"));
- if (!chan->cdr && !(chan->cdr = ast_cdr_alloc()))
+ ast_channel_lock(chan);
+ if (!chan->cdr && !(chan->cdr = ast_cdr_alloc())) {
+ ast_channel_unlock(chan);
return -1;
+ }
ast_cdr_setuserfield(chan, tmp);
+ ast_channel_unlock(chan);
}
if (waitforbridge) {
/* We must remove the "b" option if listed. In principle none of
More information about the asterisk-commits
mailing list