Skip to content

Commit

Permalink
channel locking: Add locking for channel snapshot creation
Browse files Browse the repository at this point in the history
Original commit message by mmichelson (asterisk 12 r403311):

"This adds channel locks around calls to create channel snapshots as well
as other functions which operate on a channel and then end up
creating a channel snapshot. Functions that expect the channel to be
locked prior to being called have had their documentation updated to
indicate such."

The above was initially committed and then reverted at r403398.  The problem
was found to be in core_local.c in the publish_local_bridge_message function.
The ast_unreal_lock_all function locks and adds a reference to the returned
channels and while they were being unlocked they were not being unreffed when
no longer needed.  Fixed by unreffing the channels.

Also in bridge.c a lock was obtained on "other->chan", but then an attempt was
made to unlock "other" and not the previously locked channel.  Fixed by
unlocking "other->chan"

(closes issue ASTERISK-22709)
Reported by: John Bigelow
........

Merged revisions 404237 from http://svn.asterisk.org/svn/asterisk/branches/12


git-svn-id: http://svn.asterisk.org/svn/asterisk/trunk@404260 f38db490-d61c-443f-a65b-d21fe96a405b
  • Loading branch information
kharwell committed Dec 18, 2013
1 parent 2eef66d commit 8d70015
Show file tree
Hide file tree
Showing 53 changed files with 340 additions and 38 deletions.
1 change: 0 additions & 1 deletion addons/chan_mobile.c
Expand Up @@ -878,7 +878,6 @@ static struct ast_channel *mbl_new(int state, struct mbl_pvt *pvt, char *cid_num
if (pvt->sco_socket != -1) {
ast_channel_set_fd(chn, 0, pvt->sco_socket);
}

ast_channel_unlock(chn);

return chn;
Expand Down
2 changes: 1 addition & 1 deletion addons/chan_ooh323.c
Expand Up @@ -2164,8 +2164,8 @@ int onCallEstablished(ooCallData *call)
}

ast_queue_control(c, AST_CONTROL_ANSWER);
ast_channel_unlock(p->owner);
ast_publish_channel_state(c);
ast_channel_unlock(p->owner);
}
ast_mutex_unlock(&p->lock);

Expand Down
4 changes: 4 additions & 0 deletions apps/app_agent_pool.c
Expand Up @@ -1465,7 +1465,9 @@ static void agent_logout(struct agent_pvt *agent)
ast_bridge_destroy(caller_bridge, AST_CAUSE_USER_BUSY);
}

ast_channel_lock(logged);
send_agent_logoff(logged, agent->username, time_logged_in);
ast_channel_unlock(logged);
ast_verb(2, "Agent '%s' logged out. Logged in for %ld seconds.\n",
agent->username, time_logged_in);
ast_channel_unref(logged);
Expand Down Expand Up @@ -2046,7 +2048,9 @@ static int agent_login_exec(struct ast_channel *chan, const char *data)
ast_verb(2, "Agent '%s' logged in (format %s/%s)\n", agent->username,
ast_getformatname(ast_channel_readformat(chan)),
ast_getformatname(ast_channel_writeformat(chan)));
ast_channel_lock(chan);
send_agent_login(chan, agent->username);
ast_channel_unlock(chan);

agent_run(agent, chan);
return -1;
Expand Down
2 changes: 2 additions & 0 deletions apps/app_confbridge.c
Expand Up @@ -1379,7 +1379,9 @@ static int alloc_playback_chan(struct confbridge_conference *conference)
}

/* To make sure playback_chan has the same language of that profile */
ast_channel_lock(conference->playback_chan);
ast_channel_language_set(conference->playback_chan, conference->b_profile.language);
ast_channel_unlock(conference->playback_chan);

ast_debug(1, "Created announcer channel '%s' to conference bridge '%s'\n",
ast_channel_name(conference->playback_chan), conference->name);
Expand Down
16 changes: 13 additions & 3 deletions apps/app_dial.c
Expand Up @@ -2104,13 +2104,15 @@ static int dial_exec_full(struct ast_channel *chan, const char *data, struct ast
struct ast_party_caller caller;

/* Reset all DIAL variables back to blank, to prevent confusion (in case we don't reset all of them). */
ast_channel_lock(chan);
ast_channel_stage_snapshot(chan);
pbx_builtin_setvar_helper(chan, "DIALSTATUS", "");
pbx_builtin_setvar_helper(chan, "DIALEDPEERNUMBER", "");
pbx_builtin_setvar_helper(chan, "DIALEDPEERNAME", "");
pbx_builtin_setvar_helper(chan, "ANSWEREDTIME", "");
pbx_builtin_setvar_helper(chan, "DIALEDTIME", "");
ast_channel_stage_snapshot_done(chan);
ast_channel_unlock(chan);

if (ast_strlen_zero(data)) {
ast_log(LOG_WARNING, "Dial requires an argument (technology/resource)\n");
Expand Down Expand Up @@ -2443,15 +2445,17 @@ static int dial_exec_full(struct ast_channel *chan, const char *data, struct ast
continue;
}

ast_channel_lock(tc);
ast_channel_stage_snapshot(tc);
ast_channel_unlock(tc);

ast_channel_get_device_name(tc, device_name, sizeof(device_name));
if (!ignore_cc) {
ast_cc_extension_monitor_add_dialstring(chan, tmp->interface, device_name);
}
pbx_builtin_setvar_helper(tc, "DIALEDPEERNUMBER", tmp->number);

ast_channel_lock_both(tc, chan);
pbx_builtin_setvar_helper(tc, "DIALEDPEERNUMBER", tmp->number);

/* Setup outgoing SDP to match incoming one */
if (!AST_LIST_FIRST(&out_chans) && !rest && CAN_EARLY_BRIDGE(peerflags, chan, tc)) {
Expand Down Expand Up @@ -2724,8 +2728,10 @@ static int dial_exec_full(struct ast_channel *chan, const char *data, struct ast
number = ast_strdupa(number);
}
ast_channel_unlock(peer);
ast_channel_lock(chan);
pbx_builtin_setvar_helper(chan, "DIALEDPEERNUMBER", number);
ast_channel_stage_snapshot_done(chan);
ast_channel_unlock(chan);

if (!ast_strlen_zero(args.url) && ast_channel_supports_html(peer) ) {
ast_debug(1, "app_dial: sendurl=%s.\n", args.url);
Expand Down Expand Up @@ -2811,16 +2817,18 @@ static int dial_exec_full(struct ast_channel *chan, const char *data, struct ast
/* chan and peer are going into the PBX; as such neither are considered
* outgoing channels any longer */
ast_clear_flag(ast_channel_flags(chan), AST_FLAG_OUTGOING);
ast_channel_stage_snapshot(peer);
ast_clear_flag(ast_channel_flags(peer), AST_FLAG_OUTGOING);

ast_replace_subargument_delimiter(opt_args[OPT_ARG_GOTO]);
ast_parseable_goto(chan, opt_args[OPT_ARG_GOTO]);
/* peer goes to the same context and extension as chan, so just copy info from chan*/
ast_channel_lock(peer);
ast_channel_stage_snapshot(peer);
ast_clear_flag(ast_channel_flags(peer), AST_FLAG_OUTGOING);
ast_channel_context_set(peer, ast_channel_context(chan));
ast_channel_exten_set(peer, ast_channel_exten(chan));
ast_channel_priority_set(peer, ast_channel_priority(chan) + 2);
ast_channel_stage_snapshot_done(peer);
ast_channel_unlock(peer);
if (ast_pbx_start(peer)) {
ast_autoservice_chan_hangup_peer(chan, peer);
}
Expand Down Expand Up @@ -2970,7 +2978,9 @@ static int dial_exec_full(struct ast_channel *chan, const char *data, struct ast
if (!res) {
if (!ast_tvzero(calldurationlimit)) {
struct timeval whentohangup = ast_tvadd(ast_tvnow(), calldurationlimit);
ast_channel_lock(peer);
ast_channel_whentohangup_set(peer, &whentohangup);
ast_channel_unlock(peer);
}
if (!ast_strlen_zero(dtmfcalled)) {
ast_verb(3, "Sending DTMF '%s' to the called party.\n", dtmfcalled);
Expand Down
5 changes: 4 additions & 1 deletion apps/app_disa.c
Expand Up @@ -381,8 +381,11 @@ static int disa_exec(struct ast_channel *chan, const char *data)
ast_set_callerid(chan, ourcidnum, ourcidname, ourcidnum);
}

if (!ast_strlen_zero(acctcode))
if (!ast_strlen_zero(acctcode)) {
ast_channel_lock(chan);
ast_channel_accountcode_set(chan, acctcode);
ast_channel_unlock(chan);
}

if (special_noanswer) {
ast_clear_flag(&cdr_flags, AST_CDR_FLAG_DISABLE);
Expand Down
2 changes: 2 additions & 0 deletions apps/app_meetme.c
Expand Up @@ -1384,7 +1384,9 @@ static void meetme_stasis_generate_msg(struct ast_conference *meetme_conference,
}
}

ast_channel_lock(chan);
msg = ast_channel_blob_create(chan, message_type, json_object);
ast_channel_unlock(chan);

if (!msg) {
return;
Expand Down
13 changes: 12 additions & 1 deletion apps/app_queue.c
Expand Up @@ -2040,8 +2040,12 @@ static void queue_publish_multi_channel_blob(struct ast_channel *caller, struct
RAII_VAR(struct ast_channel_snapshot *, caller_snapshot, NULL, ao2_cleanup);
RAII_VAR(struct ast_channel_snapshot *, agent_snapshot, NULL, ao2_cleanup);

ast_channel_lock(caller);
caller_snapshot = ast_channel_snapshot_create(caller);
ast_channel_unlock(caller);
ast_channel_lock(agent);
agent_snapshot = ast_channel_snapshot_create(agent);
ast_channel_unlock(agent);

if (!caller_snapshot || !agent_snapshot) {
return;
Expand Down Expand Up @@ -3452,7 +3456,9 @@ static int join_queue(char *queuename, struct queue_ent *qe, enum queue_result *
"Queue", q->name,
"Position", qe->pos,
"Count", q->count);
ast_channel_lock(qe->chan);
ast_channel_publish_blob(qe->chan, queue_caller_join_type(), blob);
ast_channel_unlock(qe->chan);
ast_debug(1, "Queue '%s' Join, Channel '%s', Position '%d'\n", q->name, ast_channel_name(qe->chan), qe->pos );
}
ao2_unlock(q);
Expand Down Expand Up @@ -3731,7 +3737,9 @@ static void leave_queue(struct queue_ent *qe)
"Queue", q->name,
"Position", qe->pos,
"Count", q->count);
ast_channel_lock(qe->chan);
ast_channel_publish_blob(qe->chan, queue_caller_leave_type(), blob);
ast_channel_unlock(qe->chan);
ast_debug(1, "Queue '%s' Leave, Channel '%s'\n", q->name, ast_channel_name(qe->chan));
/* Take us out of the queue */
if (prev) {
Expand Down Expand Up @@ -4329,10 +4337,13 @@ static void record_abandoned(struct queue_ent *qe)
"Position", qe->pos,
"OriginalPosition", qe->opos,
"HoldTime", (int)(time(NULL) - qe->start));
ast_channel_publish_blob(qe->chan, queue_caller_abandon_type(), blob);

qe->parent->callsabandoned++;
ao2_unlock(qe->parent);

ast_channel_lock(qe->chan);
ast_channel_publish_blob(qe->chan, queue_caller_abandon_type(), blob);
ast_channel_unlock(qe->chan);
}

/*! \brief RNA == Ring No Answer. Common code that is executed when we try a queue member and they don't answer. */
Expand Down
2 changes: 2 additions & 0 deletions apps/app_userevent.c
Expand Up @@ -114,7 +114,9 @@ static int userevent_exec(struct ast_channel *chan, const char *data)
}
}

ast_channel_lock(chan);
ast_channel_publish_blob(chan, ast_channel_user_event_type(), blob);
ast_channel_unlock(chan);
return 0;
}

Expand Down
5 changes: 4 additions & 1 deletion apps/app_voicemail.c
Expand Up @@ -10958,8 +10958,11 @@ static int vm_execmain(struct ast_channel *chan, const char *data)
#endif

/* Set language from config to override channel language */
if (!ast_strlen_zero(vmu->language))
if (!ast_strlen_zero(vmu->language)) {
ast_channel_lock(chan);
ast_channel_language_set(chan, vmu->language);
ast_channel_unlock(chan);
}

/* Retrieve urgent, old and new message counts */
ast_debug(1, "Before open_mailbox\n");
Expand Down
1 change: 0 additions & 1 deletion channels/chan_alsa.c
Expand Up @@ -601,7 +601,6 @@ static struct ast_channel *alsa_new(struct chan_alsa_pvt *p, int state, const ch
ast_jb_configure(tmp, &global_jbconf);

ast_channel_stage_snapshot_done(tmp);

ast_channel_unlock(tmp);

if (state != AST_STATE_DOWN) {
Expand Down
1 change: 0 additions & 1 deletion channels/chan_console.c
Expand Up @@ -444,7 +444,6 @@ static struct ast_channel *console_new(struct console_pvt *pvt, const char *ext,
ast_jb_configure(chan, &global_jbconf);

ast_channel_stage_snapshot_done(chan);

ast_channel_unlock(chan);

if (state != AST_STATE_DOWN) {
Expand Down
6 changes: 6 additions & 0 deletions channels/chan_dahdi.c
Expand Up @@ -1667,7 +1667,9 @@ static void publish_dahdichannel(struct ast_channel *chan, int span, const char
return;
}

ast_channel_lock(chan);
ast_channel_publish_blob(chan, dahdichannel_type(), blob);
ast_channel_unlock(chan);
}

/*!
Expand Down Expand Up @@ -9616,6 +9618,7 @@ static void *analog_ss_thread(void *data)
getforward = 0;
} else {
res = tone_zone_play_tone(p->subs[idx].dfd, -1);
ast_channel_lock(chan);
ast_channel_exten_set(chan, exten);
if (!ast_strlen_zero(p->cid_num)) {
if (!p->hidecallerid)
Expand All @@ -9628,6 +9631,7 @@ static void *analog_ss_thread(void *data)
ast_set_callerid(chan, NULL, p->cid_name, NULL);
}
ast_setstate(chan, AST_STATE_RING);
ast_channel_unlock(chan);
dahdi_ec_enable(p);
res = ast_pbx_run(chan);
if (res) {
Expand Down Expand Up @@ -10391,8 +10395,10 @@ static void *analog_ss_thread(void *data)

my_handle_notify_message(chan, p, flags, -1);

ast_channel_lock(chan);
ast_setstate(chan, AST_STATE_RING);
ast_channel_rings_set(chan, 1);
ast_channel_unlock(chan);
p->ringt = p->ringt_base;
res = ast_pbx_run(chan);
if (res) {
Expand Down
3 changes: 2 additions & 1 deletion channels/chan_gtalk.c
Expand Up @@ -1226,7 +1226,6 @@ static struct ast_channel *gtalk_new(struct gtalk *client, struct gtalk_pvt *i,
ast_jb_configure(tmp, &global_jbconf);

ast_channel_stage_snapshot_done(tmp);

ast_channel_unlock(tmp);

if (state != AST_STATE_DOWN && ast_pbx_start(tmp)) {
Expand Down Expand Up @@ -1421,7 +1420,9 @@ static int gtalk_newcall(struct gtalk *client, ikspak *pak)
ast_format_cap_joint_copy(p->cap, p->peercap, p->jointcap);
ast_mutex_unlock(&p->lock);

ast_channel_lock(chan);
ast_setstate(chan, AST_STATE_RING);
ast_channel_unlock(chan);
if (ast_format_cap_is_empty(p->jointcap)) {
ast_log(LOG_WARNING, "Capabilities don't match : us - %s, peer - %s, combined - %s \n", ast_getformatname_multiple(s1, BUFSIZ, p->cap),
ast_getformatname_multiple(s2, BUFSIZ, p->peercap),
Expand Down
3 changes: 2 additions & 1 deletion channels/chan_iax2.c
Expand Up @@ -5804,7 +5804,6 @@ static struct ast_channel *ast_iax2_new(int callno, int state, iax2_format capab
}

ast_channel_stage_snapshot_done(tmp);

ast_channel_unlock(tmp);

if (state != AST_STATE_DOWN) {
Expand Down Expand Up @@ -12237,7 +12236,9 @@ static struct ast_channel *iax2_request(const char *type, struct ast_format_cap
if (c) {
struct ast_format_cap *joint;
if (callid) {
ast_channel_lock(c);
ast_channel_callid_set(c, callid);
ast_channel_unlock(c);
}

/* Choose a format we can live with */
Expand Down
3 changes: 2 additions & 1 deletion channels/chan_jingle.c
Expand Up @@ -941,7 +941,6 @@ static struct ast_channel *jingle_new(struct jingle *client, struct jingle_pvt *
ast_jb_configure(tmp, &global_jbconf);

ast_channel_stage_snapshot_done(tmp);

ast_channel_unlock(tmp);

if (state != AST_STATE_DOWN && ast_pbx_start(tmp)) {
Expand Down Expand Up @@ -1117,7 +1116,9 @@ static int jingle_newcall(struct jingle *client, ikspak *pak)
}

ast_mutex_unlock(&p->lock);
ast_channel_lock(chan);
ast_setstate(chan, AST_STATE_RING);
ast_channel_unlock(chan);
res = ast_pbx_start(chan);

switch (res) {
Expand Down
3 changes: 2 additions & 1 deletion channels/chan_mgcp.c
Expand Up @@ -1570,7 +1570,6 @@ static struct ast_channel *mgcp_new(struct mgcp_subchannel *sub, int state, cons
}

ast_channel_stage_snapshot_done(tmp);

ast_channel_unlock(tmp);

if (state != AST_STATE_DOWN) {
Expand Down Expand Up @@ -3048,6 +3047,7 @@ static void *mgcp_ss(void *data)
} else {
/*res = tone_zone_play_tone(p->subs[index].zfd, -1);*/
ast_indicate(chan, -1);
ast_channel_lock(chan);
ast_channel_exten_set(chan, p->dtmf_buf);
ast_channel_dialed(chan)->number.str = ast_strdup(p->dtmf_buf);
memset(p->dtmf_buf, 0, sizeof(p->dtmf_buf));
Expand All @@ -3056,6 +3056,7 @@ static void *mgcp_ss(void *data)
p->hidecallerid ? "" : p->cid_name,
ast_channel_caller(chan)->ani.number.valid ? NULL : p->cid_num);
ast_setstate(chan, AST_STATE_RING);
ast_channel_unlock(chan);
if (p->dtmfmode & MGCP_DTMF_HYBRID) {
p->dtmfmode |= MGCP_DTMF_INBAND;
ast_indicate(chan, -1);
Expand Down

0 comments on commit 8d70015

Please sign in to comment.