[Asterisk-code-review] res_pjsip_session: Fix off-nominal session refresh. (asterisk[13])

[Joshua Colp]

Joshua Colp has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/13736 )


Change subject: res_pjsip_session: Fix off-nominal session refresh.
......................................................................

res_pjsip_session: Fix off-nominal session refresh.

Given a scenario where session refreshes occur close to
each other while another is finishing it was possible for
the session refreshes to occur out of order.

For the out of order session refreshes the first session
refresh would be queued due to a transaction in progress.
This transaction would then finish. When finished a
separate task to process the delayed requests queue
would be queued for handling. A second refresh would
be requested internally before this delayed request
queued task was processed. As no transaction was in
progress this session refresh would be immediately
handled before the queued session refresh.

The code will now check if any delayed requests exist
before allowing a session refresh to immediately occur.
If any exist then the session refresh is queued.

ASTERISK-28730

Change-Id: Ic4a2423d54e750acef900d215333569ff97f6108
---
M res/res_pjsip_session.c
1 file changed, 34 insertions(+), 6 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/36/13736/1

diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index b61b0fd..756bf82 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -61,6 +61,12 @@
 		enum ast_sip_session_response_priority response_priority);
 static void handle_outgoing_request(struct ast_sip_session *session, pjsip_tx_data *tdata);
 static void handle_outgoing_response(struct ast_sip_session *session, pjsip_tx_data *tdata);
+static int sip_session_refresh(struct ast_sip_session *session,
+		ast_sip_session_request_creation_cb on_request_creation,
+		ast_sip_session_sdp_creation_cb on_sdp_creation,
+		ast_sip_session_response_cb on_response,
+		enum ast_sip_session_refresh_method method, int generate_new_sdp,
+		int queued);
 
 /*! \brief NAT hook for modifying outgoing messages with SDP */
 static struct ast_sip_nat_hook *nat_hook;
@@ -556,14 +562,14 @@
 
 	switch (delay->method) {
 	case DELAYED_METHOD_INVITE:
-		ast_sip_session_refresh(session, delay->on_request_creation,
+		sip_session_refresh(session, delay->on_request_creation,
 			delay->on_sdp_creation, delay->on_response,
-			AST_SIP_SESSION_REFRESH_METHOD_INVITE, delay->generate_new_sdp);
+			AST_SIP_SESSION_REFRESH_METHOD_INVITE, delay->generate_new_sdp, 1);
 		return 0;
 	case DELAYED_METHOD_UPDATE:
-		ast_sip_session_refresh(session, delay->on_request_creation,
+		sip_session_refresh(session, delay->on_request_creation,
 			delay->on_sdp_creation, delay->on_response,
-			AST_SIP_SESSION_REFRESH_METHOD_UPDATE, delay->generate_new_sdp);
+			AST_SIP_SESSION_REFRESH_METHOD_UPDATE, delay->generate_new_sdp, 1);
 		return 0;
 	case DELAYED_METHOD_BYE:
 		ast_sip_session_terminate(session, 0);
@@ -855,11 +861,11 @@
     }
 }
 
-int ast_sip_session_refresh(struct ast_sip_session *session,
+static int sip_session_refresh(struct ast_sip_session *session,
 		ast_sip_session_request_creation_cb on_request_creation,
 		ast_sip_session_sdp_creation_cb on_sdp_creation,
 		ast_sip_session_response_cb on_response,
-		enum ast_sip_session_refresh_method method, int generate_new_sdp)
+		enum ast_sip_session_refresh_method method, int generate_new_sdp, int queued)
 {
 	pjsip_inv_session *inv_session = session->inv_session;
 	pjmedia_sdp_session *new_sdp = NULL;
@@ -882,6 +888,18 @@
 				? DELAYED_METHOD_INVITE : DELAYED_METHOD_UPDATE);
 	}
 
+	/* If this is a new session refresh attempt and there are outstanding delayed
+	 * requests ensure they get processed first to ensure proper ordering
+	 */
+	if (!queued && !AST_LIST_EMPTY(&session->delayed_requests)) {
+		ast_debug(3, "Delay sending request to %s because of outstanding requests...\n",
+			ast_sorcery_object_get_id(session->endpoint));
+		return delay_request(session, on_request_creation, on_sdp_creation, on_response,
+			generate_new_sdp,
+			method == AST_SIP_SESSION_REFRESH_METHOD_INVITE
+				? DELAYED_METHOD_INVITE : DELAYED_METHOD_UPDATE);
+	}
+
 	if (method == AST_SIP_SESSION_REFRESH_METHOD_INVITE) {
 		if (inv_session->invite_tsx) {
 			/* We can't send a reinvite yet, so delay it */
@@ -946,6 +964,16 @@
 	return 0;
 }
 
+int ast_sip_session_refresh(struct ast_sip_session *session,
+		ast_sip_session_request_creation_cb on_request_creation,
+		ast_sip_session_sdp_creation_cb on_sdp_creation,
+		ast_sip_session_response_cb on_response,
+		enum ast_sip_session_refresh_method method, int generate_new_sdp)
+{
+	return sip_session_refresh(session, on_request_creation, on_sdp_creation, on_response, method,
+		generate_new_sdp, 0);
+}
+
 int ast_sip_session_regenerate_answer(struct ast_sip_session *session,
 		ast_sip_session_sdp_creation_cb on_sdp_creation)
 {

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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Change-Id: Ic4a2423d54e750acef900d215333569ff97f6108
Gerrit-Change-Number: 13736
Gerrit-PatchSet: 1
Gerrit-Owner: Joshua Colp <jcolp at sangoma.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200210/b6125039/attachment.html>