[asterisk-commits] core: Cleanup some channel snapshot staging anomalies. (asterisk[14])
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Mon Feb 13 10:35:50 CST 2017
Anonymous Coward #1000019 has submitted this change and it was merged. ( https://gerrit.asterisk.org/4910 )
Change subject: core: Cleanup some channel snapshot staging anomalies.
......................................................................
core: Cleanup some channel snapshot staging anomalies.
We shouldn't unlock the channel after starting a snapshot staging because
another thread may interfere and do its own snapshot staging.
* app_dial.c:dial_exec_full() made hold the channel lock while setting up
the outgoing channel staging. Made hold the channel lock after the called
party answers while updating the caller channel staging.
* chan_sip.c:sip_new() completed the channel staging on off-nominal exit.
Also we need to use ast_hangup() instead of ast_channel_unref() at that
location.
* channel.c:__ast_channel_alloc_ap() added a comment about not needing to
complete the channel snapshot staging on off-nominal exit paths.
* rtp_engine.c:ast_rtp_instance_set_stats_vars() made hold the channel
locks while staging the channels for the stats channel variables.
Change-Id: Iefb6336893163f6447bad65568722ad5d5d8212a
---
M apps/app_dial.c
M channels/chan_sip.c
M main/channel.c
M main/rtp_engine.c
4 files changed, 36 insertions(+), 29 deletions(-)
Approvals:
George Joseph: Looks good to me, approved
Anonymous Coward #1000019: Verified
Joshua Colp: Looks good to me, but someone else must approve
diff --git a/apps/app_dial.c b/apps/app_dial.c
index c80caeb..5e0f172 100644
--- a/apps/app_dial.c
+++ b/apps/app_dial.c
@@ -2543,16 +2543,14 @@
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);
}
ast_channel_lock_both(tc, chan);
+ ast_channel_stage_snapshot(tc);
+
pbx_builtin_setvar_helper(tc, "DIALEDPEERNUMBER", tmp->number);
/* Setup outgoing SDP to match incoming one */
@@ -2568,7 +2566,6 @@
ast_channel_appl_set(tc, "AppDial");
ast_channel_data_set(tc, "(Outgoing Line)");
- ast_channel_publish_snapshot(tc);
memset(ast_channel_whentohangup(tc), 0, sizeof(*ast_channel_whentohangup(tc)));
@@ -2793,15 +2790,14 @@
}
} else {
const char *number;
+ const char *name;
int dial_end_raised = 0;
int cause = -1;
- if (ast_test_flag64(&opts, OPT_CALLER_ANSWER))
+ if (ast_test_flag64(&opts, OPT_CALLER_ANSWER)) {
ast_answer(chan);
+ }
- strcpy(pa.status, "ANSWER");
- ast_channel_stage_snapshot(chan);
- pbx_builtin_setvar_helper(chan, "DIALSTATUS", pa.status);
/* Ah ha! Someone answered within the desired timeframe. Of course after this
we will always return with -1 so that it is hung up properly after the
conversation. */
@@ -2823,10 +2819,10 @@
hanguptree(&out_chans, peer, cause >= 0 ? cause : AST_CAUSE_ANSWERED_ELSEWHERE);
/* If appropriate, log that we have a destination channel and set the answer time */
- if (ast_channel_name(peer))
- pbx_builtin_setvar_helper(chan, "DIALEDPEERNAME", ast_channel_name(peer));
ast_channel_lock(peer);
+ name = ast_strdupa(ast_channel_name(peer));
+
number = pbx_builtin_getvar_helper(peer, "DIALEDPEERNUMBER");
if (ast_strlen_zero(number)) {
number = NULL;
@@ -2834,8 +2830,16 @@
number = ast_strdupa(number);
}
ast_channel_unlock(peer);
+
ast_channel_lock(chan);
+ ast_channel_stage_snapshot(chan);
+
+ strcpy(pa.status, "ANSWER");
+ pbx_builtin_setvar_helper(chan, "DIALSTATUS", pa.status);
+
+ pbx_builtin_setvar_helper(chan, "DIALEDPEERNAME", name);
pbx_builtin_setvar_helper(chan, "DIALEDPEERNUMBER", number);
+
ast_channel_stage_snapshot_done(chan);
ast_channel_unlock(chan);
diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 709d5ab..0622a92 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -8148,7 +8148,9 @@
if (!fmt) {
ast_log(LOG_WARNING, "No compatible formats could be found for %s\n", ast_channel_name(tmp));
ao2_ref(caps, -1);
- tmp = ast_channel_unref(tmp);
+ ast_channel_stage_snapshot_done(tmp);
+ ast_channel_unlock(tmp);
+ ast_hangup(tmp);
return NULL;
}
}
diff --git a/main/channel.c b/main/channel.c
index 92763f9..798c35f 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -825,7 +825,12 @@
ast_channel_stage_snapshot(tmp);
if (!(nativeformats = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT))) {
- /* format capabilities structure allocation failure */
+ /*
+ * Aborting the channel creation. We do not need to complete staging
+ * the channel snapshot because the channel has not been finalized or
+ * linked into the channels container yet. Nobody else knows about
+ * this channel nor will anybody ever know about it.
+ */
return ast_channel_unref(tmp);
}
ast_format_cap_append(nativeformats, ast_format_none, 0);
@@ -851,6 +856,7 @@
if (!(schedctx = ast_sched_context_create())) {
ast_log(LOG_WARNING, "Channel allocation failed: Unable to create schedule context\n");
+ /* See earlier channel creation abort comment above. */
return ast_channel_unref(tmp);
}
ast_channel_sched_set(tmp, schedctx);
@@ -865,6 +871,7 @@
ast_channel_caller(tmp)->id.name.valid = 1;
ast_channel_caller(tmp)->id.name.str = ast_strdup(cid_name);
if (!ast_channel_caller(tmp)->id.name.str) {
+ /* See earlier channel creation abort comment above. */
return ast_channel_unref(tmp);
}
}
@@ -872,6 +879,7 @@
ast_channel_caller(tmp)->id.number.valid = 1;
ast_channel_caller(tmp)->id.number.str = ast_strdup(cid_num);
if (!ast_channel_caller(tmp)->id.number.str) {
+ /* See earlier channel creation abort comment above. */
return ast_channel_unref(tmp);
}
}
@@ -885,6 +893,7 @@
}
if (needqueue && ast_channel_internal_alertpipe_init(tmp)) {
+ /* See earlier channel creation abort comment above. */
return ast_channel_unref(tmp);
}
@@ -976,20 +985,14 @@
if (assignedids && (does_id_conflict(assignedids->uniqueid) || does_id_conflict(assignedids->uniqueid2))) {
ast_channel_internal_errno_set(AST_CHANNEL_ERROR_ID_EXISTS);
ao2_unlock(channels);
- /* This is a bit unorthodox, but we can't just call ast_channel_stage_snapshot_done()
- * because that will result in attempting to publish the channel snapshot. That causes
- * badness in some places, such as CDRs. So we need to manually clear the flag on the
- * channel that says that a snapshot is being cleared.
- */
- ast_clear_flag(ast_channel_flags(tmp), AST_FLAG_SNAPSHOT_STAGE);
ast_channel_unlock(tmp);
+ /* See earlier channel creation abort comment above. */
return ast_channel_unref(tmp);
}
+ /* Finalize and link into the channels container. */
ast_channel_internal_finalize(tmp);
-
ast_atomic_fetchadd_int(&chancount, +1);
-
ao2_link_flags(channels, tmp, OBJ_NOLOCK);
ao2_unlock(channels);
diff --git a/main/rtp_engine.c b/main/rtp_engine.c
index 9175a28..b5ac529 100644
--- a/main/rtp_engine.c
+++ b/main/rtp_engine.c
@@ -1901,16 +1901,16 @@
{
char quality_buf[AST_MAX_USER_FIELD];
char *quality;
- struct ast_channel *bridge = ast_channel_bridge_peer(chan);
+ struct ast_channel *bridge;
- ast_channel_lock(chan);
- ast_channel_stage_snapshot(chan);
- ast_channel_unlock(chan);
+ bridge = ast_channel_bridge_peer(chan);
if (bridge) {
- ast_channel_lock(bridge);
+ ast_channel_lock_both(chan, bridge);
ast_channel_stage_snapshot(bridge);
- ast_channel_unlock(bridge);
+ } else {
+ ast_channel_lock(chan);
}
+ ast_channel_stage_snapshot(chan);
quality = ast_rtp_instance_get_quality(instance, AST_RTP_INSTANCE_STAT_FIELD_QUALITY,
quality_buf, sizeof(quality_buf));
@@ -1948,11 +1948,9 @@
}
}
- ast_channel_lock(chan);
ast_channel_stage_snapshot_done(chan);
ast_channel_unlock(chan);
if (bridge) {
- ast_channel_lock(bridge);
ast_channel_stage_snapshot_done(bridge);
ast_channel_unlock(bridge);
ast_channel_unref(bridge);
--
To view, visit https://gerrit.asterisk.org/4910
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iefb6336893163f6447bad65568722ad5d5d8212a
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: 14
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
More information about the asterisk-commits
mailing list