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

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Tue Jun 25 13:35:38 CDT 2013


Author: mmichelson
Date: Tue Jun 25 13:35:35 2013
New Revision: 392856

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=392856
Log:
Rearrange how things are allocated in feature_attended_transfer.

This makes it easier and more reliable to clean up on failure since everything
that is allocated gets allocated directly into the props structure. The new
shutdown function will also be useful when the transfer is completed in order
to get everything cleaned up.


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=392856&r1=392855&r2=392856
==============================================================================
--- team/mmichelson/atxfer_features/main/bridging_basic.c (original)
+++ team/mmichelson/atxfer_features/main/bridging_basic.c Tue Jun 25 13:35:35 2013
@@ -379,6 +379,9 @@
 	int atxfercallbackretries;
 	int retry_attempts;
 	int target_framehook_id;
+	char exten[AST_MAX_EXTENSION];
+	char context[AST_MAX_CONTEXT];
+	char failsound[PATH_MAX];
 };
 
 static void attended_transfer_properties_destructor(void *obj)
@@ -396,11 +399,11 @@
 }
 
 static struct attended_transfer_properties *attended_transfer_properties_alloc(
-		struct ast_bridge *transferee_bridge, struct ast_bridge *target_bridge,
-		struct ast_channel *transferer, struct ast_channel *transfer_target,
-		int atxferdropcall, int atxfernoanswertimeout, int atxfercallbackretries)
+		struct ast_bridge *transferee_bridge, struct ast_channel *transferer,
+		const char *context)
 {
 	struct attended_transfer_properties *props;
+	RAII_VAR(struct ast_features_xfer_config *, xfer_cfg, NULL, ao2_cleanup);
 
 	props = ao2_alloc(sizeof(*props), attended_transfer_properties_destructor);
 	if (!props) {
@@ -410,21 +413,52 @@
 	ast_mutex_init(&props->lock);
 	ast_cond_init(&props->cond, NULL);
 
-	ao2_ref(transferee_bridge, +1);
-	props->transferee_bridge = transferee_bridge;
-	ao2_ref(target_bridge, +1);
-	props->target_bridge = target_bridge;
-
+	props->target_framehook_id = -1;
 	props->transferer = ast_channel_ref(transferer);
-	props->transfer_target = ast_channel_ref(transfer_target);
-
-	props->timeout = ast_samp2tv(atxfernoanswertimeout, 1000);
+
 	AST_LIST_HEAD_INIT(&props->stimulus_queue);
 
-	props->atxferdropcall = atxferdropcall;
-	props->atxfercallbackretries = atxfercallbackretries;
+	ast_channel_lock(props->transferer);
+	xfer_cfg = ast_get_chan_features_xfer_config(props->transferer);
+	if (!xfer_cfg) {
+		ao2_ref(props, -1);
+		return NULL;
+	}
+	props->atxferdropcall = xfer_cfg->atxferdropcall;
+	props->atxfercallbackretries = xfer_cfg->atxfercallbackretries;
+	props->timeout = ast_samp2tv(xfer_cfg->atxfernoanswertimeout, 1000);
+	ast_copy_string(props->context, get_transfer_context(transferer, context), sizeof(props->context));
+	ast_copy_string(props->failsound, xfer_cfg->xferfailsound, sizeof(props->failsound));
+	ast_channel_unlock(props->transferer);
 
 	return props;
+}
+
+static void attended_transfer_properties_shutdown(struct attended_transfer_properties *props,
+		int hangup_target)
+{
+	if (props->transferee_bridge) {
+		ast_bridge_merge_inhibit(props->transferee_bridge, -1);
+	}
+
+	if (props->transfer_target) {
+		SCOPED_CHANNELLOCK(lock, props->transfer_target);
+		if (hangup_target) {
+			ast_softhangup_nolock(props->transfer_target, AST_SOFTHANGUP_EXPLICIT);
+		}
+
+		if (props->target_framehook_id != -1) {
+			ast_framehook_detach(props->transfer_target, props->target_framehook_id);
+		}
+	}
+
+	if (props->target_bridge) {
+		ast_bridge_destroy(props->target_bridge);
+	}
+	/* XXX There will be more here, such as removing transferer role
+	 * from transferer
+	 */
+	ao2_cleanup(props);
 }
 
 static void hold(struct ast_channel *chan)
@@ -972,6 +1006,8 @@
 			frame && frame->frametype == AST_FRAME_CONTROL &&
 			frame->subclass.integer == AST_CONTROL_ANSWER) {
 		stimulate_attended_transfer(props, STIMULUS_TARGET_ANSWER);
+		ast_framehook_detach(chan, props->target_framehook_id);
+		props->target_framehook_id = -1;
 	}
 
 	return frame;
@@ -1093,7 +1129,7 @@
 	return NULL;
 }
 
-static int attach_framehook(struct ast_channel *transfer_target, struct attended_transfer_properties *props)
+static int attach_framehook(struct attended_transfer_properties *props)
 {
 	struct ast_framehook_interface target_interface = {
 		.version = AST_FRAMEHOOK_INTERFACE_VERSION,
@@ -1104,7 +1140,7 @@
 	ao2_ref(props, +1);
 	target_interface.data = props;
 
-	props->target_framehook_id = ast_framehook_attach(transfer_target, &target_interface);
+	props->target_framehook_id = ast_framehook_attach(props->transfer_target, &target_interface);
 	if (props->target_framehook_id == -1) {
 		ao2_ref(props, -1);
 		return -1;
@@ -1112,51 +1148,18 @@
 	return 0;
 }
 
-static int add_transferer_role(struct ast_channel *chan, const char *abort, const char *complete, const char *threeway, const char *swap)
-{
-	return ast_channel_add_bridge_role(chan, TRANSFERER_ROLE_NAME) ||
-		ast_channel_set_bridge_role_option(chan, TRANSFERER_ROLE_NAME, "abort", abort) ||
-		ast_channel_set_bridge_role_option(chan, TRANSFERER_ROLE_NAME, "complete", complete) ||
-		ast_channel_set_bridge_role_option(chan, TRANSFERER_ROLE_NAME, "threeway", threeway) ||
-		ast_channel_set_bridge_role_option(chan, TRANSFERER_ROLE_NAME, "swap", swap);
-}
-
-/*! \brief Internal built in feature for attended transfers */
-static int feature_attended_transfer(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel, void *hook_pvt)
-{
-	char exten[AST_MAX_EXTENSION] = "";
-	struct ast_channel *peer;
-	struct ast_bridge *attended_bridge;
-	struct ast_bridge_features caller_features;
-	struct ast_bridge_features_attended_transfer *attended_transfer = hook_pvt;
-	const char *context;
+static int add_transferer_role(struct ast_channel *chan, struct ast_bridge_features_attended_transfer *attended_transfer)
+{
 	const char *atxfer_abort;
 	const char *atxfer_threeway;
 	const char *atxfer_complete;
 	const char *atxfer_swap;
-	const char *fail_sound;
 	RAII_VAR(struct ast_features_xfer_config *, xfer_cfg, NULL, ao2_cleanup);
-	int atxferdropcall;
-	int atxfercallbackretries;
-	int atxfernoanswertimeout;
-	struct attended_transfer_properties *props;
-	char destination[AST_MAX_EXTENSION + AST_MAX_CONTEXT + 1];
-	pthread_t thread;
-
-	ast_bridge_channel_write_hold(bridge_channel, NULL);
-
-	bridge = ast_bridge_channel_merge_inhibit(bridge_channel, +1);
-
-	ast_channel_lock(bridge_channel->chan);
-	context = ast_strdupa(get_transfer_context(bridge_channel->chan,
-		attended_transfer ? attended_transfer->context : NULL));
-	xfer_cfg = ast_get_chan_features_xfer_config(bridge_channel->chan);
+	SCOPED_CHANNELLOCK(lock, chan);
+
+	xfer_cfg = ast_get_chan_features_xfer_config(chan);
 	if (!xfer_cfg) {
-		ast_log(LOG_ERROR, "Unable to get transfer configuration options\n");
-		ast_channel_unlock(bridge_channel->chan);
-		ast_bridge_merge_inhibit(bridge, -1);
-		ao2_ref(bridge, -1);
-		return 0;
+		return -1;
 	}
 	if (attended_transfer) {
 		atxfer_abort = ast_strdupa(S_OR(attended_transfer->abort, xfer_cfg->atxferabort));
@@ -1169,183 +1172,113 @@
 		atxfer_complete = ast_strdupa(xfer_cfg->atxfercomplete);
 		atxfer_swap = ast_strdupa(xfer_cfg->atxferswap);
 	}
-	fail_sound = ast_strdupa(xfer_cfg->xferfailsound);
-	atxferdropcall = xfer_cfg->atxferdropcall;
-	atxfercallbackretries = xfer_cfg->atxfercallbackretries;
-	atxfernoanswertimeout = xfer_cfg->atxfernoanswertimeout;
-	if (add_transferer_role(bridge_channel->chan, atxfer_abort, atxfer_complete, atxfer_threeway, atxfer_swap)) {
-		ast_channel_unlock(bridge_channel->chan);
-		ast_bridge_merge_inhibit(bridge, -1);
-		ao2_ref(bridge, -1);
+
+	return ast_channel_add_bridge_role(chan, TRANSFERER_ROLE_NAME) ||
+		ast_channel_set_bridge_role_option(chan, TRANSFERER_ROLE_NAME, "abort", atxfer_abort) ||
+		ast_channel_set_bridge_role_option(chan, TRANSFERER_ROLE_NAME, "complete", atxfer_complete) ||
+		ast_channel_set_bridge_role_option(chan, TRANSFERER_ROLE_NAME, "threeway", atxfer_threeway) ||
+		ast_channel_set_bridge_role_option(chan, TRANSFERER_ROLE_NAME, "swap", atxfer_swap);
+}
+
+/*! \brief Internal built in feature for attended transfers */
+static int feature_attended_transfer(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel, void *hook_pvt)
+{
+	struct ast_bridge_features_attended_transfer *attended_transfer = hook_pvt;
+	struct attended_transfer_properties *props;
+	char destination[AST_MAX_EXTENSION + AST_MAX_CONTEXT + 1];
+	pthread_t thread;
+
+	props = attended_transfer_properties_alloc(bridge, bridge_channel->chan,
+			attended_transfer ? attended_transfer->context : NULL);
+
+	if (!props) {
+		ast_log(LOG_ERROR, "Unable to allocate control structure for performing attended transfer\n");
 		return 0;
 	}
-	ast_channel_unlock(bridge_channel->chan);
+
+	if (add_transferer_role(props->transferer, attended_transfer)) {
+		ast_log(LOG_ERROR, "Unable to set transferer bridge role properly\n");
+		attended_transfer_properties_shutdown(props, 0);
+		return 0;
+	}
+
+	ast_bridge_channel_write_hold(bridge_channel, NULL);
+	props->transferee_bridge = ast_bridge_channel_merge_inhibit(bridge_channel, +1);
 
 	/* Grab the extension to transfer to */
-	if (grab_transfer(bridge_channel->chan, exten, sizeof(exten), context)) {
-		ast_bridge_merge_inhibit(bridge, -1);
-		ao2_ref(bridge, -1);
+	if (grab_transfer(bridge_channel->chan, props->exten, sizeof(props->exten), props->context)) {
+		ast_log(LOG_WARNING, "Unable to acquire target extension for attended transfer\n");
 		ast_bridge_channel_write_unhold(bridge_channel);
+		attended_transfer_properties_shutdown(props, 0);
 		return 0;
 	}
 
 	/* Fill the variable with the extension and context we want to call */
-	snprintf(destination, sizeof(destination), "%s@%s", exten, context);
+	snprintf(destination, sizeof(destination), "%s@%s", props->exten, props->context);
+
+	ast_debug(1, "Attended transfer to '%s'\n", destination);
 
 	/* Get a channel that is the destination we wish to call */
-	peer = dial_transfer(bridge_channel->chan, destination);
-	if (!peer) {
-		ast_bridge_merge_inhibit(bridge, -1);
-		ao2_ref(bridge, -1);
-		ast_stream_and_wait(bridge_channel->chan, fail_sound, AST_DIGIT_NONE);
+	props->transfer_target = dial_transfer(bridge_channel->chan, destination);
+	if (!props->transfer_target) {
+		ast_log(LOG_ERROR, "Unable to request outbound channel for attended transfer target\n");
+		ast_stream_and_wait(props->transferer, props->failsound, AST_DIGIT_NONE);
 		ast_bridge_channel_write_unhold(bridge_channel);
+		attended_transfer_properties_shutdown(props, 0);
 		return 0;
 	}
 
 	/* Create a bridge to use to talk to the person we are calling */
-	attended_bridge = ast_bridge_basic_new();
-	if (!attended_bridge) {
-		ast_bridge_features_cleanup(&caller_features);
-		ast_hangup(peer);
-		ast_bridge_merge_inhibit(bridge, -1);
-		ao2_ref(bridge, -1);
-		ast_stream_and_wait(bridge_channel->chan, fail_sound, AST_DIGIT_NONE);
+	props->target_bridge = ast_bridge_basic_new();
+	if (!props->target_bridge) {
+		ast_log(LOG_ERROR, "Unable to create bridge for attended transfer target\n");
+		ast_stream_and_wait(props->transferer, props->failsound, AST_DIGIT_NONE);
 		ast_bridge_channel_write_unhold(bridge_channel);
+		attended_transfer_properties_shutdown(props, 1);
 		return 0;
 	}
-	ast_bridge_merge_inhibit(attended_bridge, +1);
-
-	props = attended_transfer_properties_alloc(bridge, attended_bridge,
-			bridge_channel->chan, peer, atxferdropcall, atxfernoanswertimeout, atxfercallbackretries);
-	
-	if (!props) {
-		ast_bridge_destroy(attended_bridge);
-		ast_bridge_features_cleanup(&caller_features);
-		ast_hangup(peer);
-		ast_bridge_merge_inhibit(bridge, -1);
-		ao2_ref(bridge, -1);
-		ast_stream_and_wait(bridge_channel->chan, fail_sound, AST_DIGIT_NONE);
+	ast_bridge_merge_inhibit(props->target_bridge, +1);
+
+	if (attach_framehook(props)) {
+		ast_log(LOG_ERROR, "Unable to attach framehook to transfer target\n");
+		ast_stream_and_wait(props->transferer, props->failsound, AST_DIGIT_NONE);
 		ast_bridge_channel_write_unhold(bridge_channel);
+		attended_transfer_properties_shutdown(props, 1);
 		return 0;
 	}
 
-	attach_framehook(peer, props);
-
-	ast_bridge_basic_change_personality_atxfer(attended_bridge, props);
+	ast_bridge_basic_change_personality_atxfer(props->target_bridge, props);
 	ast_bridge_basic_change_personality_atxfer(bridge, props);
 
-	if (ast_call(peer, destination, 0)) {
-		ast_bridge_destroy(attended_bridge);
-		ast_bridge_features_cleanup(&caller_features);
-		ast_hangup(peer);
-		ast_bridge_merge_inhibit(bridge, -1);
-		ao2_ref(bridge, -1);
-		ast_stream_and_wait(bridge_channel->chan, fail_sound, AST_DIGIT_NONE);
-		ao2_cleanup(props);
+	if (ast_call(props->transfer_target, destination, 0)) {
+		ast_log(LOG_ERROR, "Unable to place outbound call to transfer target\n");
+		ast_stream_and_wait(bridge_channel->chan, props->failsound, AST_DIGIT_NONE);
 		ast_bridge_channel_write_unhold(bridge_channel);
+		attended_transfer_properties_shutdown(props, 1);
 		return 0;
 	}
 
 	/* This is how this is going down, we are imparting the channel we called above into this bridge first */
-	if (ast_bridge_impart(attended_bridge, peer, NULL, NULL, 1)) {
-		ast_bridge_destroy(attended_bridge);
-		ast_bridge_features_cleanup(&caller_features);
-		ast_hangup(peer);
-		ast_bridge_merge_inhibit(bridge, -1);
-		ao2_ref(bridge, -1);
-		ast_stream_and_wait(bridge_channel->chan, fail_sound, AST_DIGIT_NONE);
-		ao2_cleanup(props);
+	if (ast_bridge_impart(props->target_bridge, props->transfer_target, NULL, NULL, 1)) {
+		ast_log(LOG_ERROR, "Unable to place transfer target into bridge\n");
+		ast_stream_and_wait(bridge_channel->chan, props->failsound, AST_DIGIT_NONE);
 		ast_bridge_channel_write_unhold(bridge_channel);
+		attended_transfer_properties_shutdown(props, 1);
 		return 0;
 	}
 
-	ast_pthread_create_detached(&thread, NULL, attended_transfer_monitor_thread, props);
+	if (ast_pthread_create_detached(&thread, NULL, attended_transfer_monitor_thread, props)) {
+		ast_log(LOG_ERROR, "Unable to create monitoring thread for attended transfer\n");
+		ast_stream_and_wait(bridge_channel->chan, props->failsound, AST_DIGIT_NONE);
+		ast_bridge_channel_write_unhold(bridge_channel);
+		attended_transfer_properties_shutdown(props, 1);
+		return 0;
+	}
 
 	/* Once the monitoring thread has been created, it is responsible for destroying all
 	 * of the necessary components.
 	 */
 	return 0;
-	
-	/* XXX Leaving the rest of the original commented out as a reference for the time being */
-#if 0
-/*
- * BUGBUG there is a small window where the channel does not point to the bridge_channel.
- *
- * This window is expected to go away when atxfer is redesigned
- * to fully support existing functionality.  There will be one
- * and only one ast_bridge_channel structure per channel.
- */
-	/* Point the channel back to the original bridge and bridge_channel. */
-	ast_bridge_channel_lock(bridge_channel);
-	ast_channel_lock(bridge_channel->chan);
-	ast_channel_internal_bridge_channel_set(bridge_channel->chan, bridge_channel);
-	ast_channel_internal_bridge_set(bridge_channel->chan, bridge_channel->bridge);
-	ast_channel_unlock(bridge_channel->chan);
-	ast_bridge_channel_unlock(bridge_channel);
-
-	/* Wait for peer thread to exit bridge and die. */
-	if (!ast_autoservice_start(bridge_channel->chan)) {
-		ast_bridge_depart(peer);
-		ast_autoservice_stop(bridge_channel->chan);
-	} else {
-		ast_bridge_depart(peer);
-	}
-
-	/* Now that all channels are out of it we can destroy the bridge and the feature structures */
-	ast_bridge_destroy(attended_bridge);
-	ast_bridge_features_cleanup(&caller_features);
-
-	/* Is there a courtesy sound to play to the peer? */
-	ast_channel_lock(bridge_channel->chan);
-	complete_sound = pbx_builtin_getvar_helper(bridge_channel->chan,
-		"ATTENDED_TRANSFER_COMPLETE_SOUND");
-	if (!ast_strlen_zero(complete_sound)) {
-		complete_sound = ast_strdupa(complete_sound);
-	} else {
-		complete_sound = NULL;
-	}
-	ast_channel_unlock(bridge_channel->chan);
-	if (complete_sound) {
-		pbx_builtin_setvar_helper(peer, "BRIDGE_PLAY_SOUND", complete_sound);
-	}
-
-	xfer_failed = -1;
-	switch (transfer_code) {
-	case ATXFER_INCOMPLETE:
-		/* Peer hungup */
-		break;
-	case ATXFER_COMPLETE:
-		/* The peer takes our place in the bridge. */
-		ast_bridge_channel_write_unhold(bridge_channel);
-		ast_bridge_change_state(bridge_channel, AST_BRIDGE_CHANNEL_STATE_HANGUP);
-		xfer_failed = ast_bridge_impart(bridge_channel->bridge, peer, bridge_channel->chan, NULL, 1);
-		break;
-	case ATXFER_THREEWAY:
-		/*
-		 * Transferer wants to convert to a threeway call.
-		 *
-		 * Just impart the peer onto the bridge and have us return to it
-		 * as normal.
-		 */
-		ast_bridge_channel_write_unhold(bridge_channel);
-		xfer_failed = ast_bridge_impart(bridge_channel->bridge, peer, NULL, NULL, 1);
-		break;
-	case ATXFER_ABORT:
-		/* Transferer decided not to transfer the call after all. */
-		break;
-	}
-	ast_bridge_merge_inhibit(bridge, -1);
-	ao2_ref(bridge, -1);
-	if (xfer_failed) {
-		ast_hangup(peer);
-		if (!ast_check_hangup_locked(bridge_channel->chan)) {
-			ast_stream_and_wait(bridge_channel->chan, fail_sound, AST_DIGIT_NONE);
-		}
-		ast_bridge_channel_write_unhold(bridge_channel);
-	}
-
-	return 0;
-#endif
 }
 
 struct ast_bridge_methods ast_bridge_basic_v_table;




More information about the asterisk-commits mailing list