Author: rmudgett Date: Tue Mar 10 11:04:32 2015 New Revision: 432668 URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=432668 Log: res_pjsip_refer: Fix occasional unexpected BYE sent after receiving a REFER.
A race condition happened between initiating a transfer and requesting that a dialog termination be delayed. Occasionally, the transferrer channels would exit the bridge and hangup before the dialog termination delay was requested. * Made request dialog termination delay before initiating the transfer action. If the transfer fails then cancel the delayed dialog termination request. ASTERISK-24755 #close Reported by: John Bigelow Review: https://reviewboard.asterisk.org/r/4460/ Modified: branches/13/include/asterisk/res_pjsip_session.h branches/13/res/res_pjsip_refer.c branches/13/res/res_pjsip_session.c branches/13/res/res_pjsip_session.exports.in Modified: branches/13/include/asterisk/res_pjsip_session.h URL: http://svnview.digium.com/svn/asterisk/branches/13/include/asterisk/res_pjsip_session.h?view=diff&rev=432668&r1=432667&r2=432668 ============================================================================== --- branches/13/include/asterisk/res_pjsip_session.h (original) +++ branches/13/include/asterisk/res_pjsip_session.h Tue Mar 10 11:04:32 2015 @@ -139,6 +139,8 @@ struct ast_dsp *dsp; /*! Whether the termination of the session should be deferred */ unsigned int defer_terminate:1; + /*! Termination requested while termination deferred */ + unsigned int terminate_while_deferred:1; /*! Deferred incoming re-invite */ pjsip_rx_data *deferred_reinvite; /*! Current T.38 state */ @@ -447,8 +449,18 @@ * \brief Defer local termination of a session until remote side terminates, or an amount of time passes * * \param session The session to defer termination on - */ -void ast_sip_session_defer_termination(struct ast_sip_session *session); + * + * \retval 0 Success + * \retval -1 Failure + */ +int ast_sip_session_defer_termination(struct ast_sip_session *session); + +/*! + * \brief Cancel a pending deferred termination. + * + * \param session The session to cancel a deferred termination on. + */ +void ast_sip_session_defer_termination_cancel(struct ast_sip_session *session); /*! * \brief Register an SDP handler Modified: branches/13/res/res_pjsip_refer.c URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip_refer.c?view=diff&rev=432668&r1=432667&r2=432668 ============================================================================== --- branches/13/res/res_pjsip_refer.c (original) +++ branches/13/res/res_pjsip_refer.c Tue Mar 10 11:04:32 2015 @@ -452,20 +452,30 @@ return attended; } -/*! \brief Task for attended transfer */ -static int refer_attended(void *data) -{ - RAII_VAR(struct refer_attended *, attended, data, ao2_cleanup); - int response = 0; - - if (!attended->transferer_second->channel) { - return -1; - } - - ast_debug(3, "Performing a REFER attended transfer - Transferer #1: %s Transferer #2: %s\n", - ast_channel_name(attended->transferer_chan), ast_channel_name(attended->transferer_second->channel)); - - switch (ast_bridge_transfer_attended(attended->transferer_chan, attended->transferer_second->channel)) { +static int defer_termination_cancel(void *data) +{ + struct ast_sip_session *session = data; + + ast_sip_session_defer_termination_cancel(session); + ao2_ref(session, -1); + return 0; +} + +/*! + * \internal + * \brief Convert transfer enum to SIP response code. + * \since 13.3.0 + * + * \param xfer_code Core transfer function enum result. + * + * \return SIP response code + */ +static int xfer_response_code2sip(enum ast_transfer_result xfer_code) +{ + int response; + + response = 503; + switch (xfer_code) { case AST_BRIDGE_TRANSFER_INVALID: response = 400; break; @@ -477,21 +487,55 @@ break; case AST_BRIDGE_TRANSFER_SUCCESS: response = 200; - ast_sip_session_defer_termination(attended->transferer); break; } - - ast_debug(3, "Final response for REFER attended transfer - Transferer #1: %s Transferer #2: %s is '%d'\n", - ast_channel_name(attended->transferer_chan), ast_channel_name(attended->transferer_second->channel), response); - - if (attended->progress && response) { - struct refer_progress_notification *notification = refer_progress_notification_alloc(attended->progress, response, PJSIP_EVSUB_STATE_TERMINATED); - + return response; +} + +/*! \brief Task for attended transfer executed by attended->transferer_second serializer */ +static int refer_attended_task(void *data) +{ + struct refer_attended *attended = data; + int response; + + if (attended->transferer_second->channel) { + ast_debug(3, "Performing a REFER attended transfer - Transferer #1: %s Transferer #2: %s\n", + ast_channel_name(attended->transferer_chan), + ast_channel_name(attended->transferer_second->channel)); + + response = xfer_response_code2sip(ast_bridge_transfer_attended( + attended->transferer_chan, + attended->transferer_second->channel)); + + ast_debug(3, "Final response for REFER attended transfer - Transferer #1: %s Transferer #2: %s is '%d'\n", + ast_channel_name(attended->transferer_chan), + ast_channel_name(attended->transferer_second->channel), + response); + } else { + ast_debug(3, "Received REFER request on channel '%s' but other channel has gone.\n", + ast_channel_name(attended->transferer_chan)); + response = 603; + } + + if (attended->progress) { + struct refer_progress_notification *notification; + + notification = refer_progress_notification_alloc(attended->progress, response, + PJSIP_EVSUB_STATE_TERMINATED); if (notification) { refer_progress_notify(notification); } } + if (response != 200) { + if (!ast_sip_push_task(attended->transferer->serializer, + defer_termination_cancel, attended->transferer)) { + /* Gave the ref to the pushed task. */ + attended->transferer = NULL; + } + } + + ao2_ref(attended, -1); return 0; } @@ -687,8 +731,16 @@ return 500; } + if (ast_sip_session_defer_termination(session)) { + ast_log(LOG_ERROR, "Received REFER request on channel '%s' from endpoint '%s' for local dialog but could not defer termination, rejecting\n", + ast_channel_name(session->channel), ast_sorcery_object_get_id(session->endpoint)); + ao2_cleanup(attended); + return 500; + } + /* Push it to the other session, which will have both channels with minimal locking */ - if (ast_sip_push_task(other_session->serializer, refer_attended, attended)) { + if (ast_sip_push_task(other_session->serializer, refer_attended_task, attended)) { + ast_sip_session_defer_termination_cancel(session); ao2_cleanup(attended); return 500; } @@ -700,6 +752,7 @@ } else { const char *context; struct refer_blind refer = { 0, }; + int response; DETERMINE_TRANSFER_CONTEXT(context, session); @@ -715,19 +768,19 @@ refer.replaces = replaces; refer.refer_to = target_uri; - switch (ast_bridge_transfer_blind(1, session->channel, "external_replaces", context, refer_blind_callback, &refer)) { - case AST_BRIDGE_TRANSFER_INVALID: - return 400; - case AST_BRIDGE_TRANSFER_NOT_PERMITTED: - return 403; - case AST_BRIDGE_TRANSFER_FAIL: + if (ast_sip_session_defer_termination(session)) { + ast_log(LOG_ERROR, "Received REFER for remote session on channel '%s' from endpoint '%s' but could not defer termination, rejecting\n", + ast_channel_name(session->channel), + ast_sorcery_object_get_id(session->endpoint)); return 500; - case AST_BRIDGE_TRANSFER_SUCCESS: - ast_sip_session_defer_termination(session); - return 200; - } - - return 503; + } + + response = xfer_response_code2sip(ast_bridge_transfer_blind(1, session->channel, + "external_replaces", context, refer_blind_callback, &refer)); + if (response != 200) { + ast_sip_session_defer_termination_cancel(session); + } + return response; } } @@ -737,6 +790,7 @@ const char *context; char exten[AST_MAX_EXTENSION]; struct refer_blind refer = { 0, }; + int response; /* If no explicit transfer context has been provided use their configured context */ DETERMINE_TRANSFER_CONTEXT(context, session); @@ -754,19 +808,19 @@ refer.rdata = rdata; refer.refer_to = target; - switch (ast_bridge_transfer_blind(1, session->channel, exten, context, refer_blind_callback, &refer)) { - case AST_BRIDGE_TRANSFER_INVALID: - return 400; - case AST_BRIDGE_TRANSFER_NOT_PERMITTED: - return 403; - case AST_BRIDGE_TRANSFER_FAIL: + if (ast_sip_session_defer_termination(session)) { + ast_log(LOG_ERROR, "Channel '%s' from endpoint '%s' attempted blind transfer but could not defer termination, rejecting\n", + ast_channel_name(session->channel), + ast_sorcery_object_get_id(session->endpoint)); return 500; - case AST_BRIDGE_TRANSFER_SUCCESS: - ast_sip_session_defer_termination(session); - return 200; - } - - return 503; + } + + response = xfer_response_code2sip(ast_bridge_transfer_blind(1, session->channel, + exten, context, refer_blind_callback, &refer)); + if (response != 200) { + ast_sip_session_defer_termination_cancel(session); + } + return response; } /*! \brief Structure used to retrieve channel from another session */ Modified: branches/13/res/res_pjsip_session.c URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip_session.c?view=diff&rev=432668&r1=432667&r2=432668 ============================================================================== --- branches/13/res/res_pjsip_session.c (original) +++ branches/13/res/res_pjsip_session.c Tue Mar 10 11:04:32 2015 @@ -1524,6 +1524,7 @@ pjsip_tx_data *packet = NULL; if (session->defer_terminate) { + session->terminate_while_deferred = 1; return; } @@ -1559,17 +1560,16 @@ static int session_termination_task(void *data) { - RAII_VAR(struct ast_sip_session *, session, data, ao2_cleanup); - pjsip_tx_data *packet = NULL; - - if (!session->inv_session) { - return 0; - } - - if (pjsip_inv_end_session(session->inv_session, 603, NULL, &packet) == PJ_SUCCESS) { - ast_sip_session_send_request(session, packet); - } - + struct ast_sip_session *session = data; + + if (session->defer_terminate) { + session->defer_terminate = 0; + if (session->inv_session) { + ast_sip_session_terminate(session, 0); + } + } + + ao2_ref(session, -1); return 0; } @@ -1582,9 +1582,13 @@ } } -void ast_sip_session_defer_termination(struct ast_sip_session *session) +int ast_sip_session_defer_termination(struct ast_sip_session *session) { pj_time_val delay = { .sec = 60, }; + int res; + + /* The session should not have an active deferred termination request. */ + ast_assert(!session->defer_terminate); session->defer_terminate = 1; @@ -1593,7 +1597,31 @@ session->scheduled_termination.user_data = session; session->scheduled_termination.cb = session_termination_cb; - if (pjsip_endpt_schedule_timer(ast_sip_get_pjsip_endpoint(), &session->scheduled_termination, &delay) != PJ_SUCCESS) { + res = (pjsip_endpt_schedule_timer(ast_sip_get_pjsip_endpoint(), + &session->scheduled_termination, &delay) != PJ_SUCCESS) ? -1 : 0; + if (res) { + session->defer_terminate = 0; + ao2_ref(session, -1); + } + return res; +} + +void ast_sip_session_defer_termination_cancel(struct ast_sip_session *session) +{ + if (!session->defer_terminate) { + /* Already canceled or timer fired. */ + return; + } + session->defer_terminate = 0; + + if (session->terminate_while_deferred) { + /* Complete the termination started by the upper layer. */ + ast_sip_session_terminate(session, 0); + } + + /* Stop the termination timer if it is still running. */ + if (pj_timer_heap_cancel(pjsip_endpt_get_timer_heap(ast_sip_get_pjsip_endpoint()), + &session->scheduled_termination)) { ao2_ref(session, -1); } } Modified: branches/13/res/res_pjsip_session.exports.in URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip_session.exports.in?view=diff&rev=432668&r1=432667&r2=432668 ============================================================================== --- branches/13/res/res_pjsip_session.exports.in (original) +++ branches/13/res/res_pjsip_session.exports.in Tue Mar 10 11:04:32 2015 @@ -2,6 +2,7 @@ global: LINKER_SYMBOL_PREFIXast_sip_session_terminate; LINKER_SYMBOL_PREFIXast_sip_session_defer_termination; + LINKER_SYMBOL_PREFIXast_sip_session_defer_termination_cancel; LINKER_SYMBOL_PREFIXast_sip_session_register_sdp_handler; LINKER_SYMBOL_PREFIXast_sip_session_unregister_sdp_handler; LINKER_SYMBOL_PREFIXast_sip_session_register_supplement; -- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- svn-commits mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/svn-commits
