Fix crash during CDR update.
authorRichard Mudgett <rmudgett@digium.com>
Fri, 16 Dec 2011 21:10:19 +0000 (21:10 +0000)
committerRichard Mudgett <rmudgett@digium.com>
Fri, 16 Dec 2011 21:10:19 +0000 (21:10 +0000)
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/
........

Merged revisions 348362 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........

Merged revisions 348363 from http://svn.asterisk.org/svn/asterisk/branches/10

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@348364 65c4cc65-6c06-0410-ace0-fbb531ad65f3

apps/app_authenticate.c
apps/app_followme.c
apps/app_queue.c
funcs/func_cdr.c
include/asterisk/cdr.h
main/channel.c
main/features.c
main/pbx.c
res/res_monitor.c

index 540e3df..3bdf483 100644 (file)
@@ -213,14 +213,20 @@ static int auth_exec(struct ast_channel *chan, const char *data)
                                                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 @@ static int auth_exec(struct ast_channel *chan, const char *data)
        }
 
        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 {
index bab4d1b..9ee2e33 100644 (file)
@@ -492,10 +492,12 @@ static void clear_caller(struct findme_user *tmpuser)
 
        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];
@@ -506,11 +508,15 @@ static void clear_caller(struct findme_user *tmpuser)
                        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;
        }
 }
 
@@ -890,6 +896,7 @@ static void findmeexec(struct fm_args *tpargs)
                                        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();
                                        }
@@ -909,6 +916,7 @@ static void findmeexec(struct fm_args *tpargs)
                                        } else {
                                                ast_log(LOG_ERROR, "Unable to create Call Detail Record\n");
                                        }
+                                       ast_channel_unlock(outbound);
                                        ast_hangup(outbound);
                                        ast_free(tmpuser);
                                }
index fe8723c..de88dca 100644 (file)
@@ -5116,9 +5116,9 @@ static int try_calling(struct queue_ent *qe, const char *options, char *announce
                        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);
index 34fdd67..dc71096 100644 (file)
@@ -197,17 +197,21 @@ static int cdr_read(struct ast_channel *chan, const char *cmd, char *parse,
 {
        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 @@ static int cdr_read(struct ast_channel *chan, const char *cmd, char *parse,
                                   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 @@ static int cdr_write(struct ast_channel *chan, const char *cmd, char *parse,
        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 @@ static int cdr_write(struct ast_channel *chan, const char *cmd, char *parse,
                /* 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;
 }
 
index e755fe3..c2268e3 100644 (file)
@@ -213,6 +213,7 @@ void ast_cdr_discard(struct ast_cdr *cdr);
  * \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);
@@ -222,6 +223,7 @@ int ast_cdr_init(struct ast_cdr *cdr, struct ast_channel *chan);
  * \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);
@@ -414,22 +416,40 @@ char *ast_cdr_flags2str(int flags);
  */
 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);
 
 
index bb1bd4e..57788b5 100644 (file)
@@ -5435,7 +5435,9 @@ struct ast_channel *ast_call_forward(struct ast_channel *caller, struct ast_chan
                        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);
@@ -5501,8 +5503,11 @@ struct ast_channel *__ast_request_and_dial(const char *type, struct ast_format_c
                        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);
@@ -5614,21 +5619,27 @@ struct ast_channel *__ast_request_and_dial(const char *type, struct ast_format_c
                *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;
        }
index 6f23bd3..bb925f7 100644 (file)
@@ -2303,6 +2303,7 @@ static int builtin_blindtransfer(struct ast_channel *chan, struct ast_channel *p
        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 @@ static int builtin_blindtransfer(struct ast_channel *chan, struct ast_channel *p
                        ast_cdr_start(transferer->cdr);
                }
        }
+       ast_channel_unlock(transferer);
        if (transferer->cdr) {
                struct ast_cdr *swap = transferer->cdr;
 
@@ -3911,11 +3913,15 @@ int ast_bridge_call(struct ast_channel *chan, struct ast_channel *peer, struct a
        /* 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;
@@ -3924,7 +3930,7 @@ int ast_bridge_call(struct ast_channel *chan, struct ast_channel *peer, struct a
        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);
@@ -3939,7 +3945,6 @@ int ast_bridge_call(struct ast_channel *chan, struct ast_channel *peer, struct a
                                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? */
@@ -3962,6 +3967,9 @@ int ast_bridge_call(struct ast_channel *chan, struct ast_channel *peer, struct a
                                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
index fc4b3d9..3ae294f 100644 (file)
@@ -5112,10 +5112,12 @@ static enum ast_pbx_result __ast_pbx_run(struct ast_channel *c,
                set_ext_pri(c, "s", 1);
        }
 
+       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 */
@@ -5222,8 +5224,11 @@ static enum ast_pbx_result __ast_pbx_run(struct ast_channel *c,
                                        }
                                        /* 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;
                        }
@@ -5327,10 +5332,12 @@ static enum ast_pbx_result __ast_pbx_run(struct ast_channel *c,
                                        }
                                }
                        }
+                       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);
                }
        }
 
@@ -9571,7 +9578,9 @@ static int pbx_builtin_resetcdr(struct ast_channel *chan, const char *data)
 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;
 }
 
index 61dc4a2..9dac051 100644 (file)
@@ -684,9 +684,13 @@ static int start_monitor_exec(struct ast_channel *chan, const char *data)
        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