[Asterisk-code-review] bridge: Fix returning to dialplan when executing Bridge() fr... (asterisk[master])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/5569 )

Change subject: bridge: Fix returning to dialplan when executing Bridge() from AMI.
......................................................................


bridge: Fix returning to dialplan when executing Bridge() from AMI.

When using the Bridge AMI action on the same channel multiple times
it was possible for the channel to return to the wrong location in
the dialplan if the other party hung up. This happened because the
priority of the channel was not preserved across each action
invocation and it would fail to move on to the next priority in
other cases.

This change makes it so that the priority of a channel is preserved
when taking control of it from another thread and it is incremented
as appropriate such that the priority reflects where the channel
should next be executed in the dialplan, not where it may or may not
currently be.

The Bridge AMI action was also changed to ensure that it too
starts the channels at the next location in the dialplan.

ASTERISK-24529

Change-Id: I52406669cf64208aef7252a65b63ade31fbf7a5a
---
M include/asterisk/channel.h
M main/channel.c
M main/features.c
3 files changed, 19 insertions(+), 4 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Jenkins2: Verified
  Joshua Colp: Approved for Submit



diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h
index 889d3ff..fd339d9 100644
--- a/include/asterisk/channel.h
+++ b/include/asterisk/channel.h
@@ -4640,6 +4640,9 @@
  *
  * \note absolutely _NO_ channel locks should be held before calling this function.
  *
+ * \note The dialplan location on the returned channel is where the channel
+ * should be started in the dialplan if it is returned to it.
+ *
  * \param yankee The channel to gain control of
  * \retval NULL Could not gain control of the channel
  * \retval non-NULL The channel
diff --git a/main/channel.c b/main/channel.c
index dbf2354..099e6f6 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -10566,6 +10566,7 @@
 		char *context;
 		char *name;
 		int amaflags;
+		int priority;
 		struct ast_format *readformat;
 		struct ast_format *writeformat;
 	} my_vars = { 0, };
@@ -10576,6 +10577,16 @@
 	my_vars.context = ast_strdupa(ast_channel_context(yankee));
 	my_vars.name = ast_strdupa(ast_channel_name(yankee));
 	my_vars.amaflags = ast_channel_amaflags(yankee);
+	my_vars.priority = ast_channel_priority(yankee);
+	/* The priority as returned by ast_channel_yank is where the channel
+	 * should go if the dialplan is executed on it. If the channel is
+	 * already executing dialplan then the priority currently set is
+	 * where it is currently. We increment it so it becomes where it should
+	 * execute.
+	 */
+	if (ast_test_flag(ast_channel_flags(yankee), AST_FLAG_IN_AUTOLOOP)) {
+		my_vars.priority++;
+	}
 	my_vars.writeformat = ao2_bump(ast_channel_writeformat(yankee));
 	my_vars.readformat = ao2_bump(ast_channel_readformat(yankee));
 	ast_channel_unlock(yankee);
@@ -10595,6 +10606,7 @@
 	ast_channel_set_writeformat(yanked_chan, my_vars.writeformat);
 	ao2_cleanup(my_vars.readformat);
 	ao2_cleanup(my_vars.writeformat);
+	ast_channel_priority_set(yanked_chan, my_vars.priority);
 
 	ast_channel_unlock(yanked_chan);
 
diff --git a/main/features.c b/main/features.c
index ae16374..61d9e51 100644
--- a/main/features.c
+++ b/main/features.c
@@ -782,7 +782,7 @@
 	chana_exten = ast_strdupa(ast_channel_exten(chana));
 	chana_context = ast_strdupa(ast_channel_context(chana));
 	chana_priority = ast_channel_priority(chana);
-	if (!ast_test_flag(ast_channel_flags(chana), AST_FLAG_IN_AUTOLOOP)) {
+	if (ast_test_flag(ast_channel_flags(chana), AST_FLAG_IN_AUTOLOOP)) {
 		chana_priority++;
 	}
 	ast_channel_unlock(chana);
@@ -798,7 +798,7 @@
 	chanb_exten = ast_strdupa(ast_channel_exten(chanb));
 	chanb_context = ast_strdupa(ast_channel_context(chanb));
 	chanb_priority = ast_channel_priority(chanb);
-	if (!ast_test_flag(ast_channel_flags(chanb), AST_FLAG_IN_AUTOLOOP)) {
+	if (ast_test_flag(ast_channel_flags(chanb), AST_FLAG_IN_AUTOLOOP)) {
 		chanb_priority++;
 	}
 	ast_channel_unlock(chanb);
@@ -809,7 +809,7 @@
 		return 0;
 	}
 
-	ast_bridge_set_after_go_on(chana, chana_context, chana_exten, chana_priority, NULL);
+	ast_bridge_set_after_goto(chana, chana_context, chana_exten, chana_priority);
 	if (ast_bridge_add_channel(bridge, chana, NULL, playtone & PLAYTONE_CHANNEL1, xfer_cfg_a ? xfer_cfg_a->xfersound : NULL)) {
 		snprintf(buf, sizeof(buf), "Unable to add Channel1 to bridge: %s", ast_channel_name(chana));
 		astman_send_error(s, m, buf);
@@ -817,7 +817,7 @@
 		return 0;
 	}
 
-	ast_bridge_set_after_go_on(chanb, chanb_context, chanb_exten, chanb_priority, NULL);
+	ast_bridge_set_after_goto(chanb, chanb_context, chanb_exten, chanb_priority);
 	if (ast_bridge_add_channel(bridge, chanb, NULL, playtone & PLAYTONE_CHANNEL2, xfer_cfg_b ? xfer_cfg_b->xfersound : NULL)) {
 		snprintf(buf, sizeof(buf), "Unable to add Channel2 to bridge: %s", ast_channel_name(chanb));
 		astman_send_error(s, m, buf);

-- 
To view, visit https://gerrit.asterisk.org/5569
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I52406669cf64208aef7252a65b63ade31fbf7a5a
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>