[Asterisk-code-review] res_pjsip_session: initialize pending's topology to endpoint's (asterisk[17])

[Kevin Harwell]

Kevin Harwell has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/13197 )

Change subject: res_pjsip_session: initialize pending's topology to endpoint's
......................................................................

res_pjsip_session: initialize pending's topology to endpoint's

Found during some testing, there is a race condition between selecting an
appropriate bridge type for a call versus the applying of media on the callee's
session. In some instances a native bridge type would have been chosen, but
due to the callee's media not yet being established at bridge compatibility
check time the simple bridge type is picked instead.

When using chan_pjsip this initiates a topology change event. The topologies
are then compared for the two sessions. However, when the topology was created
for the caller its streams are initialized to "inactive". This topology is then
used as a base when creating the callee's topology, and streams. Soon after
the caller's topology's stream(s) get updated based on the sdp (get set to
sendrecv in the failing scenario).

Now when the topology change event is raised, and the two topologies are
compared, the comparison fails due to a stream state mismatch (sendrecv vs
inactive). And since they differ a reinvite is sent out (to the caller in
this case).

This patch makes it such that when the caller's topology is initially created
it gets created based on its configured endpoint's media topology. When the
endpoint's topology is created its stream's state(s) are initialized to
sendrecv instead of inactive. Subsequently, now when the callee's topology is
created its topology streams are now initialized to sendrecv. Thus when the
topology change event occurs due to the mentioned scenario the stream states
match for the given sessions, and the reinvite is not sent unless due to some
other valid mismatch.

Note, this patch only changes one pending media state's creation point. It's
possible other places *could* be changed, however for now it was deemed best
to only alter what's here.

Change-Id: I6ba3a6a75f64824a1b963044c37acbe951c389c7
---
M res/res_pjsip_session.c
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  Benjamin Keith Ford: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Kevin Harwell: Approved for Submit



diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index a620719..7373c19 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -712,7 +712,7 @@
 
 	/* It is possible for SDP deferral to have already created a pending topology */
 	if (!session->pending_media_state->topology) {
-		session->pending_media_state->topology = ast_stream_topology_alloc();
+		session->pending_media_state->topology = ast_stream_topology_clone(session->endpoint->media.topology);
 		if (!session->pending_media_state->topology) {
 			return -1;
 		}

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

Gerrit-Project: asterisk
Gerrit-Branch: 17
Gerrit-Change-Id: I6ba3a6a75f64824a1b963044c37acbe951c389c7
Gerrit-Change-Number: 13197
Gerrit-PatchSet: 1
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20191114/00153aa5/attachment.html>