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/
........

Merged revisions 432668 from http://svn.asterisk.org/svn/asterisk/branches/13


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

3 files changed, 129 insertions, 46 deletions

diff --git a/res/res_pjsip_refer.c b/res/res_pjsip_refer.c
index c1b76ae20..5195177d5 100644
--- a/res/res_pjsip_refer.c
+++ b/res/res_pjsip_refer.c
@@ -452,20 +452,30 @@ static struct refer_attended *refer_attended_alloc(struct ast_sip_session *trans
 	return attended;
 }
 
-/*! \brief Task for attended transfer */
-static int refer_attended(void *data)
+static int defer_termination_cancel(void *data)
 {
-	RAII_VAR(struct refer_attended *, attended, data, ao2_cleanup);
-	int response = 0;
+	struct ast_sip_session *session = data;
 
-	if (!attended->transferer_second->channel) {
-		return -1;
-	}
+	ast_sip_session_defer_termination_cancel(session);
+	ao2_ref(session, -1);
+	return 0;
+}
 
-	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));
+/*!
+ * \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;
 
-	switch (ast_bridge_transfer_attended(attended->transferer_chan, attended->transferer_second->channel)) {
+	response = 503;
+	switch (xfer_code) {
 	case AST_BRIDGE_TRANSFER_INVALID:
 		response = 400;
 		break;
@@ -477,21 +487,55 @@ static int refer_attended(void *data)
 		break;
 	case AST_BRIDGE_TRANSFER_SUCCESS:
 		response = 200;
-		ast_sip_session_defer_termination(attended->transferer);
 		break;
 	}
+	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));
 
-	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);
+		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 && response) {
-		struct refer_progress_notification *notification = refer_progress_notification_alloc(attended->progress, response, PJSIP_EVSUB_STATE_TERMINATED);
+	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 @@ static int refer_incoming_attended_request(struct ast_sip_session *session, pjsi
 			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 @@ static int refer_incoming_attended_request(struct ast_sip_session *session, pjsi
 	} else {
 		const char *context;
 		struct refer_blind refer = { 0, };
+		int response;
 
 		DETERMINE_TRANSFER_CONTEXT(context, session);
 
@@ -715,19 +768,19 @@ static int refer_incoming_attended_request(struct ast_sip_session *session, pjsi
 		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 @@ static int refer_incoming_blind_request(struct ast_sip_session *session, pjsip_r
 	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 @@ static int refer_incoming_blind_request(struct ast_sip_session *session, pjsip_r
 	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 */
diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index 9c01d3ccf..5b75f3c7e 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -1524,6 +1524,7 @@ void ast_sip_session_terminate(struct ast_sip_session *session, int response)
 	pjsip_tx_data *packet = NULL;
 
 	if (session->defer_terminate) {
+		session->terminate_while_deferred = 1;
 		return;
 	}
 
@@ -1559,17 +1560,16 @@ void ast_sip_session_terminate(struct ast_sip_session *session, int response)
 
 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;
-	}
+	struct ast_sip_session *session = data;
 
-	if (pjsip_inv_end_session(session->inv_session, 603, NULL, &packet) == PJ_SUCCESS) {
-		ast_sip_session_send_request(session, packet);
+	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 @@ static void session_termination_cb(pj_timer_heap_t *timer_heap, struct pj_timer_
 	}
 }
 
-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 @@ void ast_sip_session_defer_termination(struct ast_sip_session *session)
 	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);
 	}
 }
diff --git a/res/res_pjsip_session.exports.in b/res/res_pjsip_session.exports.in
index 5a21b1d2f..a39485e66 100644
--- a/res/res_pjsip_session.exports.in
+++ b/res/res_pjsip_session.exports.in
@@ -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;