[Asterisk-code-review] bridge: suppress initial connected line update on attended t... (asterisk[13])

[Richard Mudgett]

Richard Mudgett has posted comments on this change. ( https://gerrit.asterisk.org/9558 )

Change subject: bridge: suppress initial connected line update on attended transfer
......................................................................


Patch Set 2:

(1 comment)

https://gerrit.asterisk.org/#/c/9558/2/main/bridge_basic.c
File main/bridge_basic.c:

https://gerrit.asterisk.org/#/c/9558/2/main/bridge_basic.c@3433
PS2, Line 3433: 	/* We increase the refcount of the transfer target because ast_bridge_impart() will
              : 	 * steal the reference we already have. We need to keep a reference, so the only
              : 	 * choice is to give it a bump
              : 	 */
              : 	ast_channel_ref(props->transfer_target);
              : 	if (ast_bridge_impart(props->target_bridge, props->transfer_target, NULL, NULL,
              : 		AST_BRIDGE_IMPART_CHAN_INDEPENDENT)) {
              : 		ast_log(LOG_ERROR, "Channel %s: Unable to place transfer target into bridge.\n",
              : 			ast_channel_name(bridge_channel->chan));
              : 		stream_failsound(props->transferer);
              : 		ast_bridge_channel_write_unhold(bridge_channel);
              : 		ast_hangup(props->transfer_target);
              : 		props->transfer_target = NULL;
              : 		attended_transfer_properties_shutdown(props);
              : 		return 0;
              : 	}
> > You need to redo this patch to *only* suppress the *initial* […]
It suppresses *more* than just the initial connected line update.  There are two transitions into the TRANSFER_CALLING_TARGET state.  The initial entry into the attended transfer state machine where we can suppress the connected line update and a transition from the TRANSFER_HESITANT state where we must not suppress the connected line update.

We must not suppress it when transitioning from the TRANSFER_HESITANT state to the TRANSFER_CALLING_TARGET state because there is a possible window of opportunity where we could loose the connected line update from Charlie to Alice if Charlie answers during the transition.

All this discussion is really pointing to the fragility of your dialplan.  Just the suppression of the *initial* connected line update is an accommodation for that fragile dialplan.  If nothing else happens then when Alice eventually completes the attended transfer of Bob to Charlie there will be a connected line update exchange between Bob and Charlie.



-- 
To view, visit https://gerrit.asterisk.org/9558
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: comment
Gerrit-Change-Id: I346652661949b6611c23e431ede0dbea1be3017a
Gerrit-Change-Number: 9558
Gerrit-PatchSet: 2
Gerrit-Owner: Alexei Gradinari <alex2grad at gmail.com>
Gerrit-Reviewer: Alexei Gradinari <alex2grad at gmail.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Thu, 02 Aug 2018 21:41:59 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180802/3dfffbec/attachment-0001.html>