From: Scott Griepentrog Date: Thu, 22 Jan 2015 18:10:13 +0000 (+0000) Subject: stasis transfer: fix a race condition on stasis bridge push X-Git-Tag: 14.0.0-beta1~1303 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=49f405fe4c4c758a0d99d05ed2d35f55dd76b732;p=thirdparty%2Fasterisk.git stasis transfer: fix a race condition on stasis bridge push After a bridge transfer completes where a local replacement channel is used, a stasis transfer message with the details of the transfer is sent. This is processed by stasis which then sets the stasis app name and replaced channel snapshot on the replacement channel. However, since a separate thread was already started to run stasis on the new replacement channel, a race was on to see if the message processing would be completed before the app name was needed, otherwise the channel would be hung up. This change moves the calls used to set the stasis app name and the replace snapshot to the bridge_stasis_push function callback from the bridge transfer logic, allowing the steps to be completed earlier and more deterministically, and the race elimianted. NOTE: the swap channel parameter to bridge_stasis_push (and thus all bridge push callbacks) must always be present when performing a swap with another channel. ASTERISK-24649 #close Reported by: John Bigelow Review: https://reviewboard.asterisk.org/r/4341/ ........ Merged revisions 430939 from http://svn.asterisk.org/svn/asterisk/branches/13 git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@430940 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- diff --git a/res/stasis/app.c b/res/stasis/app.c index 1cc4fb5115..330e711e68 100644 --- a/res/stasis/app.c +++ b/res/stasis/app.c @@ -724,23 +724,6 @@ static int bridge_app_subscribed_involved(struct stasis_app *app, struct ast_bri return subscribed; } -static void set_replacement_channel(struct ast_channel_snapshot *to_be_replaced, - struct ast_channel_snapshot *replacing) -{ - struct stasis_app_control *control = stasis_app_control_find_by_channel_id( - to_be_replaced->uniqueid); - struct ast_channel *chan = ast_channel_get_by_name(replacing->uniqueid); - - if (control && chan) { - ast_channel_lock(chan); - app_set_replace_channel_app(chan, app_name(control_app(control))); - app_set_replace_channel_snapshot(chan, to_be_replaced); - ast_channel_unlock(chan); - } - ast_channel_cleanup(chan); - ao2_cleanup(control); -} - static void bridge_blind_transfer_handler(void *data, struct stasis_subscription *sub, struct stasis_message *message) { @@ -748,10 +731,6 @@ static void bridge_blind_transfer_handler(void *data, struct stasis_subscription struct ast_blind_transfer_message *transfer_msg = stasis_message_data(message); struct ast_bridge_snapshot *bridge = transfer_msg->bridge; - if (transfer_msg->replace_channel) { - set_replacement_channel(transfer_msg->transferer, transfer_msg->replace_channel); - } - if (bridge_app_subscribed(app, transfer_msg->transferer->uniqueid) || (bridge && bridge_app_subscribed_involved(app, bridge))) { stasis_publish(app->topic, message); @@ -802,18 +781,6 @@ static void bridge_attended_transfer_handler(void *data, struct stasis_subscript if (subscribed) { stasis_publish(app->topic, message); } - - if (transfer_msg->replace_channel) { - set_replacement_channel(transfer_msg->to_transferee.channel_snapshot, - transfer_msg->replace_channel); - } - - if (transfer_msg->dest_type == AST_ATTENDED_TRANSFER_DEST_LINK) { - set_replacement_channel(transfer_msg->to_transferee.channel_snapshot, - transfer_msg->dest.links[0]); - set_replacement_channel(transfer_msg->to_transfer_target.channel_snapshot, - transfer_msg->dest.links[1]); - } } static void bridge_default_handler(void *data, struct stasis_subscription *sub, diff --git a/res/stasis/stasis_bridge.c b/res/stasis/stasis_bridge.c index 646b3062a6..657238417c 100644 --- a/res/stasis/stasis_bridge.c +++ b/res/stasis/stasis_bridge.c @@ -115,6 +115,27 @@ static int bridge_stasis_push(struct ast_bridge *self, struct ast_bridge_channel { 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 */ /* Attach after-bridge callback and pass ownership of swap_app to it */ @@ -125,8 +146,15 @@ static int bridge_stasis_push(struct ast_bridge *self, struct ast_bridge_channel } bridge_stasis_queue_join_action(self, bridge_channel); + if (swap) { + /* nudge the swap channel out of the bridge */ + ast_bridge_channel_leave_bridge(swap, BRIDGE_CHANNEL_STATE_END_NO_DISSOLVE, 0); + } - /* Return -1 so the push fails and the after-bridge callback gets called */ + /* Return -1 so the push fails and the after-bridge callback gets called + * This keeps the bridging framework from putting the channel into the bridge + * until the Stasis thread gets started, and then the channel is put into the bridge. + */ return -1; }