[asterisk-commits] stasis transfer: fix stasis bridge push race part two (asterisk[certified/13.1])
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Fri Apr 22 17:21:43 CDT 2016
Joshua Colp has submitted this change and it was merged.
Change subject: stasis transfer: fix stasis bridge push race part two
......................................................................
stasis transfer: fix stasis bridge push race part two
When swapping a Local channel in place of one already
in a bridge (to complete a bridge attended transfer),
the channel that was swapped out can actually be hung
up before the stasis bridge push callback executes on
the independant transfer thread. This results in the
stasis app loop dropping out and removing the control
that has the the app name which the local replacement
channel needs so it can re-enter stasis.
To avoid this race condition a new push_peek callback
has been added, and called from the ast_bridge_impart
thread before it launches the independant thread that
will complete the transfer. Now the stasis push_peek
callback can copy the stasis app name before the swap
channel can hang up.
ASTERISK-24649
Review: https://reviewboard.asterisk.org/r/4382/
This patch is a remedial cherry-pick from v13.
Change-Id: I307c3b506af5af80ec506f73e8b78a91d79999e0
---
M include/asterisk/bridge.h
M main/bridge.c
M res/stasis/stasis_bridge.c
3 files changed, 98 insertions(+), 21 deletions(-)
Approvals:
Mark Michelson: Looks good to me, approved
Joshua Colp: Looks good to me, but someone else must approve; Verified
diff --git a/include/asterisk/bridge.h b/include/asterisk/bridge.h
index e5d4820..423ae04 100644
--- a/include/asterisk/bridge.h
+++ b/include/asterisk/bridge.h
@@ -241,6 +241,8 @@
ast_bridge_notify_masquerade_fn notify_masquerade;
/*! Get the bridge merge priority. */
ast_bridge_merge_priority_fn get_merge_priority;
+ /*! Peek at swap channel before it can hang up, prior to push. */
+ ast_bridge_push_channel_fn push_peek;
};
/*! Softmix technology parameters. */
diff --git a/main/bridge.c b/main/bridge.c
index bd5f63c..c1729e5 100644
--- a/main/bridge.c
+++ b/main/bridge.c
@@ -904,6 +904,26 @@
return 0;
}
+/*!
+ * \internal
+ * \brief ast_bridge base push_peek method.
+ * \since 13.2.0
+ *
+ * \param self Bridge to operate upon.
+ * \param bridge_channel Bridge channel to push.
+ * \param swap Bridge channel to swap places with if not NULL.
+ *
+ * \note On entry, self is already locked.
+ * \note Stub because of nothing to do.
+ *
+ * \retval 0 on success
+ * \retval -1 on failure
+ */
+static int bridge_base_push_peek(struct ast_bridge *self, struct ast_bridge_channel *bridge_channel, struct ast_bridge_channel *swap)
+{
+ return 0;
+}
+
struct ast_bridge_methods ast_bridge_base_v_table = {
.name = "base",
.destroy = bridge_base_destroy,
@@ -912,6 +932,7 @@
.pull = bridge_base_pull,
.notify_masquerade = bridge_base_notify_masquerade,
.get_merge_priority = bridge_base_get_merge_priority,
+ .push_peek = bridge_base_push_peek,
};
struct ast_bridge *ast_bridge_base_new(uint32_t capabilities, unsigned int flags, const char *creator, const char *name, const char *id)
@@ -1589,6 +1610,18 @@
bridge_channel->features = features;
bridge_channel->inhibit_colp = !!(flags & AST_BRIDGE_JOIN_INHIBIT_JOIN_COLP);
+ /* allow subclass to peek at upcoming push operation */
+ if (bridge->v_table->push_peek && !res) {
+ struct ast_bridge_channel *bcswap = NULL;
+
+ ast_bridge_lock(bridge);
+ if (bridge_channel->swap) {
+ bcswap = bridge_find_channel(bridge, bridge_channel->swap);
+ }
+ res = bridge->v_table->push_peek(bridge, bridge_channel, bcswap);
+ ast_bridge_unlock(bridge);
+ }
+
if (!res) {
res = bridge_channel_internal_join(bridge_channel);
}
@@ -1724,6 +1757,18 @@
(flags & AST_BRIDGE_IMPART_CHAN_MASK) == AST_BRIDGE_IMPART_CHAN_DEPARTABLE;
bridge_channel->callid = ast_read_threadstorage_callid();
+ /* allow subclass to peek at swap channel before it can hangup */
+ if (bridge->v_table->push_peek && !res) {
+ struct ast_bridge_channel *bcswap = NULL;
+
+ ast_bridge_lock(bridge);
+ if (bridge_channel->swap) {
+ bcswap = bridge_find_channel(bridge, bridge_channel->swap);
+ }
+ res = bridge->v_table->push_peek(bridge, bridge_channel, bcswap);
+ ast_bridge_unlock(bridge);
+ }
+
/* Actually create the thread that will handle the channel */
if (!res) {
if ((flags & AST_BRIDGE_IMPART_CHAN_MASK) == AST_BRIDGE_IMPART_CHAN_INDEPENDENT) {
diff --git a/res/stasis/stasis_bridge.c b/res/stasis/stasis_bridge.c
index 6572384..e410881 100644
--- a/res/stasis/stasis_bridge.c
+++ b/res/stasis/stasis_bridge.c
@@ -99,6 +99,56 @@
/*!
* \internal
+ * \brief Peek at channel before it is pushed into bridge
+ * \since 13.2.0
+ *
+ * \param self Bridge to operate upon.
+ * \param bridge_channel Bridge channel to push.
+ * \param swap Bridge channel to swap places with if not NULL.
+ *
+ * \note On entry, self is already locked.
+ *
+ * \retval 0 on success.
+ * \retval -1 on failure. The channel should not be pushed.
+ */
+static int bridge_stasis_push_peek(struct ast_bridge *self, struct ast_bridge_channel *bridge_channel, struct ast_bridge_channel *swap)
+{
+ struct stasis_app_control *swap_control;
+ struct ast_channel_snapshot *to_be_replaced;
+
+ if (!swap) {
+ goto done;
+ }
+
+ swap_control = stasis_app_control_find_by_channel(swap->chan);
+ if (!swap_control) {
+ ast_log(LOG_ERROR,"Failed to find stasis app control for swapped channel %s\n", ast_channel_name(swap->chan));
+ return -1;
+ }
+ to_be_replaced = ast_channel_snapshot_get_latest(ast_channel_uniqueid(swap->chan));
+
+ ast_debug(3, "Copying stasis app name %s from %s to %s\n", app_name(control_app(swap_control)),
+ ast_channel_name(swap->chan), ast_channel_name(bridge_channel->chan));
+
+ ast_channel_lock(bridge_channel->chan);
+
+ /* copy the app name from the swap channel */
+ app_set_replace_channel_app(bridge_channel->chan, app_name(control_app(swap_control)));
+
+ /* set the replace channel snapshot */
+ app_set_replace_channel_snapshot(bridge_channel->chan, to_be_replaced);
+
+ ast_channel_unlock(bridge_channel->chan);
+
+ ao2_ref(swap_control, -1);
+ ao2_cleanup(to_be_replaced);
+
+done:
+ return ast_bridge_base_v_table.push_peek(self, bridge_channel, swap);
+}
+
+/*!
+ * \internal
* \brief Push this channel into the Stasis bridge.
* \since 12.5.0
*
@@ -114,27 +164,6 @@
static int bridge_stasis_push(struct ast_bridge *self, struct ast_bridge_channel *bridge_channel, struct ast_bridge_channel *swap)
{
struct stasis_app_control *control = stasis_app_control_find_by_channel(bridge_channel->chan);
-
- if (swap) {
- struct ast_channel_snapshot *to_be_replaced = ast_channel_snapshot_get_latest(ast_channel_uniqueid(swap->chan));
- struct stasis_app_control *swap_control = stasis_app_control_find_by_channel(swap->chan);
-
- /* set the replace channel snapshot */
- ast_channel_lock(bridge_channel->chan);
- app_set_replace_channel_snapshot(bridge_channel->chan, to_be_replaced);
-
- /* copy the app name from the swap channel */
- if (swap_control) {
- ast_debug(3, "Copying stasis app name %s from %s to %s\n",
- app_name(control_app(swap_control)),
- ast_channel_name(swap->chan),
- ast_channel_name(bridge_channel->chan));
- app_set_replace_channel_app(bridge_channel->chan, app_name(control_app(swap_control)));
- }
- ast_channel_unlock(bridge_channel->chan);
- ao2_cleanup(to_be_replaced);
- ao2_cleanup(swap_control);
- }
if (!control && !stasis_app_channel_is_internal(bridge_channel->chan)) {
/* channel not in Stasis(), get it there */
@@ -256,4 +285,5 @@
bridge_stasis_v_table.name = "stasis";
bridge_stasis_v_table.push = bridge_stasis_push;
bridge_stasis_v_table.pull = bridge_stasis_pull;
+ bridge_stasis_v_table.push_peek = bridge_stasis_push_peek;
}
--
To view, visit https://gerrit.asterisk.org/2665
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I307c3b506af5af80ec506f73e8b78a91d79999e0
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: certified/13.1
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
More information about the asterisk-commits
mailing list