[Asterisk-code-review] res pjsip session.c: Process initial INVITE sooner. (key exi... (asterisk[master])

[Jenkins2]

Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/5630 )

Change subject: res_pjsip_session.c: Process initial INVITE sooner. (key exists)
......................................................................


res_pjsip_session.c: Process initial INVITE sooner. (key exists)

Retransmissions of an initial INVITE could be queued in the serializer
before we have processed the first INVITE message.  If the first INVITE
message doesn't get completely processed before the retransmissions are
seen then we could try to setup the same call from the retransmissions.  A
symptom of this is seeing a (key exists) message associated with an
INVITE.  An earlier change attempted to address this kind of problem by
calculating a distributor serializer to use for unassociated messages.
Part of that change also made incoming calls keep using that distributor
serializer.  (ASTERISK-26088) However, some leftover code was still
deferring the INVITE processing to the session's serializer even though we
were already in that serializer.  This not only is unnecessary but would
cause the same call resetup problem.

* Removed the code to defer processing the initial INVITE to the session's
serializer because we are already running in that serializer.

ASTERISK-26998 #close

Change-Id: I1e822d82dcc650e508bc2d40d545d5de4f3421f6
---
M res/res_pjsip_session.c
1 file changed, 20 insertions(+), 51 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, but someone else must approve
  Jenkins2: Approved for Submit
  Joshua Colp: Looks good to me, approved



diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index 2613a74..add7168 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -2096,39 +2096,8 @@
 	pjsip_rx_data *rdata;
 };
 
-static void new_invite_destroy(void *obj)
+static int new_invite(struct new_invite *invite)
 {
-	struct new_invite *invite = obj;
-
-	ao2_cleanup(invite->session);
-
-	if (invite->rdata) {
-		pjsip_rx_data_free_cloned(invite->rdata);
-	}
-}
-
-static struct new_invite *new_invite_alloc(struct ast_sip_session *session, pjsip_rx_data *rdata)
-{
-	struct new_invite *invite = ao2_alloc(sizeof(*invite), new_invite_destroy);
-
-	if (!invite) {
-		return NULL;
-	}
-
-	ao2_ref(session, +1);
-	invite->session = session;
-
-	if (pjsip_rx_data_clone(rdata, 0, &invite->rdata) != PJ_SUCCESS) {
-		ao2_ref(invite, -1);
-		return NULL;
-	}
-
-	return invite;
-}
-
-static int new_invite(void *data)
-{
-	RAII_VAR(struct new_invite *, invite, data, ao2_cleanup);
 	pjsip_tx_data *tdata = NULL;
 	pjsip_timer_setting timer;
 	pjsip_rdata_sdp_info *sdp_info;
@@ -2250,7 +2219,7 @@
 	pjsip_tx_data *tdata = NULL;
 	pjsip_inv_session *inv_session = NULL;
 	struct ast_sip_session *session;
-	struct new_invite *invite;
+	struct new_invite invite;
 
 	ast_assert(endpoint != NULL);
 
@@ -2287,18 +2256,17 @@
 		return;
 	}
 
-	invite = new_invite_alloc(session, rdata);
-	if (!invite || ast_sip_push_task(session->serializer, new_invite, invite)) {
-		if (pjsip_inv_initial_answer(inv_session, rdata, 500, NULL, NULL, &tdata) == PJ_SUCCESS) {
-			pjsip_inv_terminate(inv_session, 500, PJ_FALSE);
-		} else {
-			pjsip_inv_send_msg(inv_session, tdata);
-		}
-#ifdef HAVE_PJSIP_INV_SESSION_REF
-		pjsip_inv_dec_ref(inv_session);
-#endif
-		ao2_cleanup(invite);
-	}
+	/*
+	 * The current thread is supposed be the session serializer to prevent
+	 * any initial INVITE retransmissions from trying to setup the same
+	 * call again.
+	 */
+	ast_assert(ast_taskprocessor_is_task(session->serializer));
+
+	invite.session = session;
+	invite.rdata = rdata;
+	new_invite(&invite);
+
 	ao2_ref(session, -1);
 }
 
@@ -2339,13 +2307,14 @@
  * 2) An in-dialog request that the inv_session layer does not
  *    handle is received (such as an in-dialog INFO)
  *
- * In all cases, there is very little we actually do in this function
+ * Except for INVITEs, there is very little we actually do in this function
  * 1) For requests we don't handle, we return PJ_FALSE
- * 2) For new INVITEs, throw the work into the SIP threadpool to be done
- *    there to free up the thread(s) handling incoming requests
- * 3) For in-dialog requests we handle, we defer handling them until the
- *    on_inv_state_change() callback instead (where we will end up putting
- *    them into the threadpool).
+ * 2) For new INVITEs, handle them now to prevent retransmissions from
+ *    trying to setup the same call again.
+ * 3) For in-dialog requests we handle, we process them in the
+ *    .on_state_changed = session_inv_on_state_changed or
+ *    .on_tsx_state_changed = session_inv_on_tsx_state_changed
+ *    callbacks instead.
  */
 static pj_bool_t session_on_rx_request(pjsip_rx_data *rdata)
 {

-- 
To view, visit https://gerrit.asterisk.org/5630
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I1e822d82dcc650e508bc2d40d545d5de4f3421f6
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>