[asterisk-commits] mmichelson: branch mmichelson/atxfer_features r393126 - /team/mmichelson/atxf...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Fri Jun 28 09:39:15 CDT 2013


Author: mmichelson
Date: Fri Jun 28 09:39:14 2013
New Revision: 393126

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=393126
Log:
Some progress towards getting recalls more stable.

This divides the state machine into two superstates,
transferring, and recalling. The transferring state is
already stable and involves the typical transfer mechanics.
The recalling state is the one that is unstable at the moment,
and involves the logic for re-calling the transferer and
transfer target when atxferdropcall=no.

As part of this conversion, the pull method for atxfer bridges
has been divided in two. The logic is different depending on
which superstate the transfer is currently in.

This commit also contains a fix for a problem I would see sometimes
where a hangup hook on a transferer would trigger twice. The reason
was because when I detected a hangup on the transferer channel, I
would not include his bridge channel in the list of kicked channels
when merging bridges. By doing this, it could cause the hangup hook
to be triggered initially when the transferer hung up and again after
the transferer was pushed into the merge target. I now always include
the transferer's channel as a channel to kick when merging bridges because
of a completed or blond transfer. However, I have to check that the
transferer channel's bridge channel is not NULL or else I may trigger
a crash when attempting to merge.


Modified:
    team/mmichelson/atxfer_features/main/bridging_basic.c

Modified: team/mmichelson/atxfer_features/main/bridging_basic.c
URL: http://svnview.digium.com/svn/asterisk/team/mmichelson/atxfer_features/main/bridging_basic.c?view=diff&rev=393126&r1=393125&r2=393126
==============================================================================
--- team/mmichelson/atxfer_features/main/bridging_basic.c (original)
+++ team/mmichelson/atxfer_features/main/bridging_basic.c Fri Jun 28 09:39:14 2013
@@ -31,7 +31,6 @@
 
 ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 
-#define REF_DEBUG 1
 #include "asterisk/astobj2.h"
 #include "asterisk/channel.h"
 #include "asterisk/utils.h"
@@ -328,6 +327,11 @@
 	}
 	return "default";
 }
+
+enum attended_transfer_superstate {
+	STATE_TRANSFER,
+	STATE_RECALL,
+};
 
 enum attended_transfer_state {
 	/* Transfer target is ringing, Transferer is in target bridge */
@@ -398,10 +402,12 @@
 	struct ast_bridge *target_bridge;
 	struct ast_channel *transferer;
 	struct ast_channel *transfer_target;
+	struct ast_channel *recall_target;
 	struct timeval start;
 	struct timeval timeout;
 	AST_LIST_HEAD(,stimulus_list) stimulus_queue;
 	enum attended_transfer_state state;
+	enum attended_transfer_superstate superstate;
 	int atxferdropcall;
 	int atxfercallbackretries;
 	int retry_attempts;
@@ -418,13 +424,14 @@
 {
 	struct attended_transfer_properties *props = obj;
 
-	ast_log(LOG_NOTICE, "What about now? Can I be a happy man again?\n");
+	ast_log(LOG_NOTICE, "Attended transfer properties destructor\n");
 
 	ao2_cleanup(props->target_bridge);
 	ao2_cleanup(props->transferee_bridge);
 	/* Use ao2_cleanup() instead of ast_channel_unref() for channels since they may be NULL */
 	ao2_cleanup(props->transferer);
 	ao2_cleanup(props->transfer_target);
+	ao2_cleanup(props->recall_target);
 	ast_cond_destroy(&props->cond);
 	ast_mutex_destroy(&props->lock);
 	AST_LIST_HEAD_DESTROY(&props->stimulus_queue);
@@ -599,20 +606,33 @@
 	struct ast_bridge_channel **kick_bridge_channels = num_kick ?
 		ast_alloca(num_kick * sizeof(*kick_bridge_channels)) : NULL;
 	int i;
+	int actual_num_kick = 0;
 
 	ast_bridge_lock_both(dest, src);
 
 	for (i = 0; i < num_kick; ++i) {
+		struct ast_bridge_channel *kick_channel;
+
 		ast_channel_lock(kick_me[i]);
-		kick_bridge_channels[i] = ast_channel_get_bridge_channel(kick_me[i]);
+		kick_channel = ast_channel_get_bridge_channel(kick_me[i]);
 		ast_channel_unlock(kick_me[i]);
+
+		/* It's possible (and fine) for the bridge channel to be NULL at this point if the
+		 * channel has hung up already. If that happens, we can just remove it from the list
+		 * of bridge channels to kick from the bridge
+		 */
+		if (!kick_channel) {
+			continue;
+		}
+
+		kick_bridge_channels[actual_num_kick++] = kick_channel;
 	}
 
 	bridge_merge_do(dest, src, kick_bridge_channels, num_kick);
 	ast_bridge_unlock(dest);
 	ast_bridge_unlock(src);
 
-	for (i = 0; i < num_kick; ++i) {
+	for (i = 0; i < actual_num_kick; ++i) {
 		ao2_cleanup(kick_bridge_channels[i]);
 	}
 }
@@ -826,24 +846,21 @@
 
 static int complete_enter(struct attended_transfer_properties *props)
 {
-	if (ast_check_hangup(props->transferer)) {
-		bridge_merge(props->transferee_bridge, props->target_bridge, NULL, 0);
-	} else {
-		bridge_merge(props->transferee_bridge, props->target_bridge, &props->transferer, 1);
-	}
-
+	bridge_merge(props->transferee_bridge, props->target_bridge, &props->transferer, 1);
 	return 0;
 }
 
 static int blond_enter(struct attended_transfer_properties *props)
 {
-	if (ast_check_hangup(props->transferer)) {
-		bridge_merge(props->transferee_bridge, props->target_bridge, NULL, 0);
-	} else {
-		bridge_merge(props->transferee_bridge, props->target_bridge, &props->transferer, 1);
-	}
+	bridge_merge(props->transferee_bridge, props->target_bridge, &props->transferer, 1);
 	ringing(props->transfer_target);
 	return 0;
+}
+
+static int blond_nonfinal_enter(struct attended_transfer_properties *props)
+{
+	props->superstate = STATE_RECALL;
+	return blond_enter(props);
 }
 
 static enum attended_transfer_state blond_nonfinal_next(struct attended_transfer_properties *props,
@@ -863,8 +880,9 @@
 		return TRANSFER_FAIL;
 	case STIMULUS_TARGET_ANSWER:
 		return TRANSFER_RESUME;
+	case STIMULUS_TIMEOUT:
+		ast_softhangup(props->transfer_target, AST_SOFTHANGUP_EXPLICIT);
 	case STIMULUS_TARGET_HANGUP:
-	case STIMULUS_TIMEOUT:
 		return TRANSFER_RECALLING;
 	}
 }
@@ -888,11 +906,6 @@
 	 * have to use the dialing API because the channel is not local.
 	 */
 
-	/* First, let's lose our reference to the transferer. We're going to
-	 * replace it anyway.
-	 */
-	props->transferer = ast_channel_unref(props->transferer);
-
 	props->dial = ast_dial_create();
 	if (!props->dial) {
 		return -1;
@@ -906,8 +919,6 @@
 		return -1;
 	}
 
-	props->transferer = ast_channel_ref(ast_dial_get_channel(props->dial, 0));
-
 	ast_dial_set_state_callback(props->dial, &recall_callback);
 
 	ao2_ref(props, +1);
@@ -925,6 +936,7 @@
 		enum attended_transfer_stimulus stimulus)
 {
 	/* No matter what the outcome was, we need to kill off the dial */
+	ast_dial_join(props->dial);
 	ast_dial_destroy(props->dial);
 	props->dial = NULL;
 
@@ -940,15 +952,15 @@
 		ast_assert(0);
 	case STIMULUS_TRANSFEREE_HANGUP:
 		return TRANSFER_FAIL;
+	case STIMULUS_TIMEOUT:
 	case STIMULUS_TRANSFERER_HANGUP:
-	case STIMULUS_TIMEOUT:
 		++props->retry_attempts;
 		if (props->retry_attempts >= props->atxfercallbackretries) {
 			return TRANSFER_FAIL;
 		}
 		return TRANSFER_RETRANSFER;
 	case STIMULUS_TRANSFERER_ANSWER:
-		ast_bridge_impart(props->transferee_bridge, props->transferer, NULL, NULL, 1);
+		ast_bridge_impart(props->transferee_bridge, props->recall_target, NULL, NULL, 1);
 		return TRANSFER_RESUME;
 	}
 }
@@ -1040,7 +1052,7 @@
 	},
 	[TRANSFER_BLOND_NONFINAL] = {
 		.state_name = "Blond Non-Final",
-		.enter = blond_enter,
+		.enter = blond_nonfinal_enter,
 		.next = blond_nonfinal_next,
 		.flags = TRANSFER_STATE_IS_TIMED,
 	},
@@ -1117,7 +1129,6 @@
 {
 	struct attended_transfer_properties *props = hook_pvt;
 
-	ast_log(LOG_NOTICE, "Coming from hangup hook?\n");
 	stimulate_attended_transfer(props, STIMULUS_TRANSFERER_HANGUP);
 	return 0;
 }
@@ -1156,7 +1167,6 @@
 	case AST_DIAL_RESULT_HANGUP:
 	case AST_DIAL_RESULT_UNANSWERED:
 		/* Failure cases */
-		ast_log(LOG_NOTICE, "Coming from recall callback? dial state == %d\n", ast_dial_state(dial));
 		stimulate_attended_transfer(props, STIMULUS_TRANSFERER_HANGUP);
 		ao2_ref(props, -1);
 		break;
@@ -1168,7 +1178,11 @@
 		break;
 	case AST_DIAL_RESULT_ANSWERED:
 		/* We struck gold! */
-		props->transferer = ast_dial_answered_steal(dial);
+		props->recall_target = ast_dial_answered_steal(dial);
+		/* We need this reference because when we impart the channel into a bridge later, the impart will
+		 * steal a reference
+		 */
+		ast_channel_ref(props->recall_target);
 		stimulate_attended_transfer(props, STIMULUS_TRANSFERER_ANSWER);
 		ao2_ref(props, -1);
 		break;
@@ -1222,11 +1236,8 @@
 	return 0;
 }
 
-static void bridge_personality_atxfer_pull(struct ast_bridge *self, struct ast_bridge_channel *bridge_channel)
-{
-	struct bridge_basic_personality *personality = self->personality;
-	struct attended_transfer_properties *props = personality->pvt;
-
+static void transfer_pull(struct ast_bridge *self, struct ast_bridge_channel *bridge_channel, struct attended_transfer_properties *props)
+{
 	if (self->num_channels > 1 || bridge_channel->state == AST_BRIDGE_CHANNEL_STATE_WAIT) {
 		return;
 	}
@@ -1260,6 +1271,62 @@
 	}
 }
 
+static void recall_pull(struct ast_bridge *self, struct ast_bridge_channel *bridge_channel, struct attended_transfer_properties *props)
+{
+	if (self == props->target_bridge) {
+		/* Once we're in the recall superstate, we no longer care about this bridge */
+		return;
+	}
+
+	if (bridge_channel->chan == props->recall_target) {
+		stimulate_attended_transfer(props, STIMULUS_TARGET_HANGUP);
+		return;
+	}
+
+	if (self->num_channels == 0) {
+		/* Empty bridge means all transferees are gone for sure */
+		stimulate_attended_transfer(props, STIMULUS_TRANSFEREE_HANGUP);
+		return;
+	}
+
+	if (self->num_channels == 1) {
+		RAII_VAR(struct ast_bridge_channel *, target_bridge_channel, NULL, ao2_cleanup);
+		if (!props->recall_target) {
+			/* No recall target means that the pull happened on a transferee. If there's still
+			 * a channel left in the bridge, we don't need to send a stimulus
+			 */
+			return;
+		}
+
+		ast_channel_lock(props->recall_target);
+		target_bridge_channel = ast_channel_get_bridge_channel(props->recall_target);
+		ast_channel_unlock(props->recall_target);
+
+		if (!target_bridge_channel) {
+			return;
+		}
+
+		if (AST_LIST_FIRST(&self->channels) == target_bridge_channel) {
+			stimulate_attended_transfer(props, STIMULUS_TRANSFEREE_HANGUP);
+		}
+	}
+}
+
+static void bridge_personality_atxfer_pull(struct ast_bridge *self, struct ast_bridge_channel *bridge_channel)
+{
+	struct bridge_basic_personality *personality = self->personality;
+	struct attended_transfer_properties *props = personality->pvt;
+
+	switch (props->superstate) {
+	case STATE_TRANSFER:
+		transfer_pull(self, bridge_channel, props);
+		break;
+	case STATE_RECALL:
+		recall_pull(self, bridge_channel, props);
+		break;
+	}
+}
+
 static enum attended_transfer_stimulus wait_for_stimulus(struct attended_transfer_properties *props)
 {
 	RAII_VAR(struct stimulus_list *, list, NULL, ast_free_ptr);




More information about the asterisk-commits mailing list