Fix Dial I option ignored if dial forked and one fork redirects.

The Dial and Queue I option is intended to block connected line updates
and redirecting updates.  However, it is a feature that when a call is
locally redirected, the I option is disabled if the redirected call runs
as a local channel so the administrator can have an opportunity to setup
new connected line information.  Unfortunately, the Dial and Queue I
option is disabled for *all* forked calls if one of those calls is
redirected.

* Make the Dial and Queue I option apply to each outgoing call leg
independently.  Now if one outgoing call leg is locally redirected, the
other outgoing calls are not affected.

* Made Dial not pass any redirecting updates when forking calls.
Redirecting updates do not make sense for this scenario.

* Made Queue not pass any redirecting updates when using the ringall
strategy.  Redirecting updates do not make sense for this scenario.

* Fixed deadlock potential with chan_local when Dial and Queue send
redirecting updates for a local redirect.

* Converted the Queue stillgoing flag to a boolean bitfield.

(closes issue ASTERISK-19511)
Reported by: rmudgett
Tested by: rmudgett

Review: https://reviewboard.asterisk.org/r/1920/
........

Merged revisions 367678 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........

Merged revisions 367679 from http://svn.asterisk.org/svn/asterisk/branches/10


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@367693 65c4cc65-6c06-0410-ace0-fbb531ad65f3

1 files changed, 115 insertions, 51 deletions

diff --git a/apps/app_queue.c b/apps/app_queue.c
index ae82bd936..c59d9c14a 100644
--- a/apps/app_queue.c
+++ b/apps/app_queue.c
@@ -893,7 +893,7 @@ enum {
 	OPT_CALLEE_HANGUP =          (1 << 4),
 	OPT_CALLER_HANGUP =          (1 << 5),
 	OPT_IGNORE_CALL_FW =         (1 << 6),
-	OPT_UPDATE_CONNECTED =       (1 << 7),
+	OPT_IGNORE_CONNECTEDLINE =   (1 << 7),
 	OPT_CALLEE_PARK =            (1 << 8),
 	OPT_CALLER_PARK =            (1 << 9),
 	OPT_NO_RETRY =               (1 << 10),
@@ -921,7 +921,7 @@ AST_APP_OPTIONS(queue_exec_options, BEGIN_OPTIONS
 	AST_APP_OPTION('h', OPT_CALLEE_HANGUP),
 	AST_APP_OPTION('H', OPT_CALLER_HANGUP),
 	AST_APP_OPTION('i', OPT_IGNORE_CALL_FW),
-	AST_APP_OPTION('I', OPT_UPDATE_CONNECTED),
+	AST_APP_OPTION('I', OPT_IGNORE_CONNECTEDLINE),
 	AST_APP_OPTION('k', OPT_CALLEE_PARK),
 	AST_APP_OPTION('K', OPT_CALLER_PARK),
 	AST_APP_OPTION('n', OPT_NO_RETRY),
@@ -1097,7 +1097,6 @@ struct callattempt {
 	struct callattempt *call_next;
 	struct ast_channel *chan;
 	char interface[256];			/*!< An Asterisk dial string (not a channel name) */
-	int stillgoing;
 	int metric;
 	time_t lastcall;
 	struct call_queue *lastqueue;
@@ -1106,8 +1105,12 @@ struct callattempt {
 	struct ast_party_connected_line connected;
 	/*! TRUE if an AST_CONTROL_CONNECTED_LINE update was saved to the connected element. */
 	unsigned int pending_connected_update:1;
+	/*! TRUE if the connected line update is blocked. */
+	unsigned int block_connected_update:1;
 	/*! TRUE if caller id is not available for connected line */
 	unsigned int dial_callerid_absent:1;
+	/*! TRUE if the call is still active */
+	unsigned int stillgoing:1;
 	struct ast_aoc_decoded *aoc_s_rate_list;
 };
 
@@ -3725,15 +3728,13 @@ static void rna(int rnatime, struct queue_ent *qe, char *interface, char *member
  * \param[in] prebusies number of busy members calculated prior to calling wait_for_answer
  * \param[in] caller_disconnect if the 'H' option is used when calling Queue(), this is used to detect if the caller pressed * to disconnect the call
  * \param[in] forwardsallowed used to detect if we should allow call forwarding, based on the 'i' option to Queue()
- * \param[in] update_connectedline Allow connected line and redirecting updates to pass through.
  *
  * \todo eventually all call forward logic should be intergerated into and replaced by ast_call_forward()
  */
-static struct callattempt *wait_for_answer(struct queue_ent *qe, struct callattempt *outgoing, int *to, char *digit, int prebusies, int caller_disconnect, int forwardsallowed, int update_connectedline)
+static struct callattempt *wait_for_answer(struct queue_ent *qe, struct callattempt *outgoing, int *to, char *digit, int prebusies, int caller_disconnect, int forwardsallowed)
 {
 	const char *queue = qe->parent->name;
 	struct callattempt *o, *start = NULL, *prev = NULL;
-	int res;
 	int status;
 	int numbusies = prebusies;
 	int numnochan = 0;
@@ -3817,10 +3818,11 @@ static struct callattempt *wait_for_answer(struct queue_ent *qe, struct callatte
 
 		/* Service all of the outgoing channels */
 		for (o = start; o; o = o->call_next) {
-			/* We go with a static buffer here instead of using ast_strdupa. Using
+			/* We go with a fixed buffer here instead of using ast_strdupa. Using
 			 * ast_strdupa in a loop like this one can cause a stack overflow
 			 */
 			char ochan_name[AST_CHANNEL_NAME];
+
 			if (o->chan) {
 				ast_channel_lock(o->chan);
 				ast_copy_string(ochan_name, ast_channel_name(o->chan), sizeof(ochan_name));
@@ -3829,7 +3831,7 @@ static struct callattempt *wait_for_answer(struct queue_ent *qe, struct callatte
 			if (o->stillgoing && (o->chan) &&  (ast_channel_state(o->chan) == AST_STATE_UP)) {
 				if (!peer) {
 					ast_verb(3, "%s answered %s\n", ochan_name, inchan_name);
-					if (update_connectedline) {
+					if (!o->block_connected_update) {
 						if (o->pending_connected_update) {
 							if (ast_channel_connected_line_sub(o->chan, in, &o->connected, 0) &&
 								ast_channel_connected_line_macro(o->chan, in, &o->connected, 1, 0)) {
@@ -3862,6 +3864,7 @@ static struct callattempt *wait_for_answer(struct queue_ent *qe, struct callatte
 				ast_copy_string(on, o->member->interface, sizeof(on));
 				ast_copy_string(membername, o->member->membername, sizeof(membername));
 
+				/* Before processing channel, go ahead and check for forwarding */
 				if (!ast_strlen_zero(ast_channel_call_forward(o->chan)) && !forwardsallowed) {
 					ast_verb(3, "Forwarding %s to '%s' prevented.\n", inchan_name, ast_channel_call_forward(o->chan));
 					numnochan++;
@@ -3883,10 +3886,17 @@ static struct callattempt *wait_for_answer(struct queue_ent *qe, struct callatte
 						stuff = tmpchan;
 						tech = "Local";
 					}
+					if (!strcasecmp(tech, "Local")) {
+						/*
+						 * Drop the connected line update block for local channels since
+						 * this is going to run dialplan and the user can change his
+						 * mind about what connected line information he wants to send.
+						 */
+						o->block_connected_update = 0;
+					}
 
 					ast_cel_report_event(in, AST_CEL_FORWARD, NULL, ast_channel_call_forward(o->chan), NULL);
 
-					/* Before processing channel, go ahead and check for forwarding */
 					ast_verb(3, "Now forwarding %s to '%s/%s' (thanks to %s)\n", inchan_name, tech, stuff, ochan_name);
 					/* Setup parameters */
 					o->chan = ast_request(tech, ast_channel_nativeformats(in), in, stuff, &status);
@@ -3897,15 +3907,29 @@ static struct callattempt *wait_for_answer(struct queue_ent *qe, struct callatte
 						o->stillgoing = 0;
 						numnochan++;
 					} else {
-						struct ast_party_redirecting redirecting;
+						ast_channel_lock_both(o->chan, original);
+						ast_party_redirecting_copy(ast_channel_redirecting(o->chan),
+							ast_channel_redirecting(original));
+						ast_channel_unlock(o->chan);
+						ast_channel_unlock(original);
 
 						ast_channel_lock_both(o->chan, in);
 						ast_channel_inherit_variables(in, o->chan);
 						ast_channel_datastore_inherit(in, o->chan);
 
+						if (o->pending_connected_update) {
+							/*
+							 * Re-seed the callattempt's connected line information with
+							 * previously acquired connected line info from the queued
+							 * channel.  The previously acquired connected line info could
+							 * have been set through the CONNECTED_LINE dialplan function.
+							 */
+							o->pending_connected_update = 0;
+							ast_party_connected_line_copy(&o->connected, ast_channel_connected(in));
+						}
+
 						ast_channel_accountcode_set(o->chan, ast_channel_accountcode(in));
 
-						ast_channel_set_redirecting(o->chan, ast_channel_redirecting(original), NULL);
 						if (!ast_channel_redirecting(o->chan)->from.number.valid
 							|| ast_strlen_zero(ast_channel_redirecting(o->chan)->from.number.str)) {
 							/*
@@ -3921,27 +3945,37 @@ static struct callattempt *wait_for_answer(struct queue_ent *qe, struct callatte
 
 						ast_channel_dialed(o->chan)->transit_network_select = ast_channel_dialed(in)->transit_network_select;
 
-						ast_party_caller_copy(ast_channel_caller(o->chan), ast_channel_caller(in));
-						ast_party_connected_line_copy(ast_channel_connected(o->chan), ast_channel_connected(original));
+						o->dial_callerid_absent = !ast_channel_caller(o->chan)->id.number.valid
+							|| ast_strlen_zero(ast_channel_caller(o->chan)->id.number.str);
+						ast_connected_line_copy_from_caller(ast_channel_connected(o->chan),
+							ast_channel_caller(in));
 
-						/*
-						 * We must unlock o->chan before calling
-						 * ast_channel_redirecting_macro, because we put o->chan into
-						 * autoservice there.  That is pretty much a guaranteed
-						 * deadlock.  This is why the handling of o->chan's lock may
-						 * seem a bit unusual here.
-						 */
-						ast_party_redirecting_init(&redirecting);
-						ast_party_redirecting_copy(&redirecting, ast_channel_redirecting(o->chan));
-						ast_channel_unlock(o->chan);
-						if ((res = ast_channel_redirecting_sub(o->chan, in, &redirecting, 0)) &&
-							(res = ast_channel_redirecting_macro(o->chan, in, &redirecting, 1, 0))) {
-							ast_channel_update_redirecting(in, &redirecting, NULL);
-						}
-						ast_party_redirecting_free(&redirecting);
 						ast_channel_unlock(in);
+						if (qe->parent->strategy != QUEUE_STRATEGY_RINGALL
+							&& !o->block_connected_update) {
+							struct ast_party_redirecting redirecting;
 
-						update_connectedline = 1;
+							/*
+							 * Redirecting updates to the caller make sense only on single
+							 * call at a time strategies.
+							 *
+							 * We must unlock o->chan before calling
+							 * ast_channel_redirecting_macro, because we put o->chan into
+							 * autoservice there.  That is pretty much a guaranteed
+							 * deadlock.  This is why the handling of o->chan's lock may
+							 * seem a bit unusual here.
+							 */
+							ast_party_redirecting_init(&redirecting);
+							ast_party_redirecting_copy(&redirecting, ast_channel_redirecting(o->chan));
+							ast_channel_unlock(o->chan);
+							if (ast_channel_redirecting_sub(o->chan, in, &redirecting, 0) &&
+								ast_channel_redirecting_macro(o->chan, in, &redirecting, 1, 0)) {
+								ast_channel_update_redirecting(in, &redirecting, NULL);
+							}
+							ast_party_redirecting_free(&redirecting);
+						} else {
+							ast_channel_unlock(o->chan);
+						}
 
 						if (ast_call(o->chan, stuff, 0)) {
 							ast_log(LOG_NOTICE, "Forwarding failed to dial '%s/%s'\n",
@@ -3962,7 +3996,7 @@ static struct callattempt *wait_for_answer(struct queue_ent *qe, struct callatte
 							/* This is our guy if someone answered. */
 							if (!peer) {
 								ast_verb(3, "%s answered %s\n", ochan_name, inchan_name);
-								if (update_connectedline) {
+								if (!o->block_connected_update) {
 									if (o->pending_connected_update) {
 										if (ast_channel_connected_line_sub(o->chan, in, &o->connected, 0) &&
 											ast_channel_connected_line_macro(o->chan, in, &o->connected, 1, 0)) {
@@ -4045,21 +4079,31 @@ static struct callattempt *wait_for_answer(struct queue_ent *qe, struct callatte
 							/* Ignore going off hook */
 							break;
 						case AST_CONTROL_CONNECTED_LINE:
-							if (!update_connectedline) {
+							if (o->block_connected_update) {
 								ast_verb(3, "Connected line update to %s prevented.\n", inchan_name);
-							} else if (qe->parent->strategy == QUEUE_STRATEGY_RINGALL) {
+								break;
+							}
+							if (qe->parent->strategy == QUEUE_STRATEGY_RINGALL) {
 								struct ast_party_connected_line connected;
+
 								ast_verb(3, "%s connected line has changed. Saving it until answer for %s\n", ochan_name, inchan_name);
 								ast_party_connected_line_set_init(&connected, &o->connected);
 								ast_connected_line_parse_data(f->data.ptr, f->datalen, &connected);
 								ast_party_connected_line_set(&o->connected, &connected, NULL);
 								ast_party_connected_line_free(&connected);
 								o->pending_connected_update = 1;
-							} else {
-								if (ast_channel_connected_line_sub(o->chan, in, f, 1) &&
-									ast_channel_connected_line_macro(o->chan, in, f, 1, 1)) {
-									ast_indicate_data(in, AST_CONTROL_CONNECTED_LINE, f->data.ptr, f->datalen);
-								}
+								break;
+							}
+
+							/*
+							 * Prevent using the CallerID from the outgoing channel since we
+							 * got a connected line update from it.
+							 */
+							o->dial_callerid_absent = 1;
+
+							if (ast_channel_connected_line_sub(o->chan, in, f, 1) &&
+								ast_channel_connected_line_macro(o->chan, in, f, 1, 1)) {
+								ast_indicate_data(in, AST_CONTROL_CONNECTED_LINE, f->data.ptr, f->datalen);
 							}
 							break;
 						case AST_CONTROL_AOC:
@@ -4074,14 +4118,23 @@ static struct callattempt *wait_for_answer(struct queue_ent *qe, struct callatte
 							}
 							break;
 						case AST_CONTROL_REDIRECTING:
-							if (!update_connectedline) {
-								ast_verb(3, "Redirecting update to %s prevented\n", inchan_name);
-							} else if (qe->parent->strategy != QUEUE_STRATEGY_RINGALL) {
-								ast_verb(3, "%s redirecting info has changed, passing it to %s\n", ochan_name, inchan_name);
-								if (ast_channel_redirecting_sub(o->chan, in, f, 1) &&
-									ast_channel_redirecting_macro(o->chan, in, f, 1, 1)) {
-									ast_indicate_data(in, AST_CONTROL_REDIRECTING, f->data.ptr, f->datalen);
-								}
+							if (qe->parent->strategy == QUEUE_STRATEGY_RINGALL) {
+								/*
+								 * Redirecting updates to the caller make sense only on single
+								 * call at a time strategies.
+								 */
+								break;
+							}
+							if (o->block_connected_update) {
+								ast_verb(3, "Redirecting update to %s prevented\n",
+									inchan_name);
+								break;
+							}
+							ast_verb(3, "%s redirecting info has changed, passing it to %s\n",
+								ochan_name, inchan_name);
+							if (ast_channel_redirecting_sub(o->chan, in, f, 1) &&
+								ast_channel_redirecting_macro(o->chan, in, f, 1, 1)) {
+								ast_indicate_data(in, AST_CONTROL_REDIRECTING, f->data.ptr, f->datalen);
 							}
 							break;
 						case AST_CONTROL_PVT_CAUSE_CODE:
@@ -4124,6 +4177,14 @@ static struct callattempt *wait_for_answer(struct queue_ent *qe, struct callatte
 				}
 				return NULL;
 			}
+
+			/*!
+			 * \todo
+			 * XXX Queue like Dial really should send any connected line
+			 * updates (AST_CONTROL_CONNECTED_LINE) from the caller to each
+			 * ringing queue member.
+			 */
+
 			if ((f->frametype == AST_FRAME_DTMF) && caller_disconnect && (f->subclass.integer == '*')) {
 				ast_verb(3, "User hit %c to disconnect call.\n", f->subclass.integer);
 				*to = 0;
@@ -4679,7 +4740,7 @@ static int try_calling(struct queue_ent *qe, const struct ast_flags opts, char *
 	char *p;
 	char vars[2048];
 	int forwardsallowed = 1;
-	int update_connectedline = 1;
+	int block_connected_line = 0;
 	int callcompletedinsl;
 	struct ao2_iterator memi;
 	struct ast_datastore *datastore, *transfer_ds;
@@ -4745,8 +4806,8 @@ static int try_calling(struct queue_ent *qe, const struct ast_flags opts, char *
 	if (ast_test_flag(&opts, OPT_IGNORE_CALL_FW)) {
 		forwardsallowed = 0;
 	}
-	if (ast_test_flag(&opts, OPT_UPDATE_CONNECTED)) {
-		update_connectedline = 0;
+	if (ast_test_flag(&opts, OPT_IGNORE_CONNECTEDLINE)) {
+		block_connected_line = 1;
 	}
 	if (ast_test_flag(&opts, OPT_CALLEE_AUTOMIXMON)) {
 		ast_set_flag(&(bridge_config.features_callee), AST_FEATURE_AUTOMIXMON);
@@ -4847,17 +4908,18 @@ static int try_calling(struct queue_ent *qe, const struct ast_flags opts, char *
 			AST_LIST_UNLOCK(dialed_interfaces);
 		}
 
-		ast_channel_lock(qe->chan);
 		/*
 		 * Seed the callattempt's connected line information with previously
 		 * acquired connected line info from the queued channel.  The
 		 * previously acquired connected line info could have been set
 		 * through the CONNECTED_LINE dialplan function.
 		 */
+		ast_channel_lock(qe->chan);
 		ast_party_connected_line_copy(&tmp->connected, ast_channel_connected(qe->chan));
 		ast_channel_unlock(qe->chan);
 
-		tmp->stillgoing = -1;
+		tmp->block_connected_update = block_connected_line;
+		tmp->stillgoing = 1;
 		tmp->member = cur;/* Place the reference for cur into callattempt. */
 		tmp->lastcall = cur->lastcall;
 		tmp->lastqueue = cur->lastqueue;
@@ -4900,7 +4962,9 @@ static int try_calling(struct queue_ent *qe, const struct ast_flags opts, char *
 	++qe->pending;
 	ao2_unlock(qe->parent);
 	ring_one(qe, outgoing, &numbusies);
-	lpeer = wait_for_answer(qe, outgoing, &to, &digit, numbusies, ast_test_flag(&(bridge_config.features_caller), AST_FEATURE_DISCONNECT), forwardsallowed, update_connectedline);
+	lpeer = wait_for_answer(qe, outgoing, &to, &digit, numbusies,
+		ast_test_flag(&(bridge_config.features_caller), AST_FEATURE_DISCONNECT),
+		forwardsallowed);
 	/* The ast_channel_datastore_remove() function could fail here if the
 	 * datastore was moved to another channel during a masquerade. If this is
 	 * the case, don't free the datastore here because later, when the channel