[Asterisk-code-review] res_pjsip_session: Fix issue with COLP and 491 (asterisk[18])
George Joseph
asteriskteam at digium.com
Fri Sep 11 11:40:20 CDT 2020
George Joseph has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/14897 )
Change subject: res_pjsip_session: Fix issue with COLP and 491
......................................................................
res_pjsip_session: Fix issue with COLP and 491
The recent 491 changes introduced a check to determine if the active
and pending topologies were equal and to suppress the re-invite if they
were. When a re-invite is sent for a COLP-only change, the pending
topology is NULL so that check doesn't happen and the re-invite is
correctly sent. Of course, sending the re-invite sets the pending
topology. If a 491 is received, when we resend the re-invite, the
pending topology is set and since we didn't request a change to the
topology in the first place, pending and active topologies are equal so
the topologies-equal check causes the re-invite to be erroneously
suppressed.
This change checks if the topologies are equal before we run the media
state resolver (which recreates the pending topology) so that when we
do the final topologies-equal check we know if they were originally
equal and it's OK if they are equal now. In this case, we continue and
send the re-invite.
ASTERISK-29014
Change-Id: Iffd7dd0500301156a566119ebde528d1a9573314
---
M res/res_pjsip_session.c
1 file changed, 17 insertions(+), 16 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/97/14897/1
diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index b93fa4e..a99b625 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -1596,11 +1596,12 @@
struct ast_sip_session_delayed_request *delay = delayed_request_alloc(method,
on_request, on_sdp_creation, on_response, generate_new_sdp, pending_media_state,
active_media_state);
+ SCOPE_ENTER(3, "%s\n", ast_sip_session_get_name(session));
if (!delay) {
ast_sip_session_media_state_free(pending_media_state);
ast_sip_session_media_state_free(active_media_state);
- return -1;
+ SCOPE_EXIT_LOG_RTN_VALUE(-1, LOG_ERROR, "Unable to allocate delay request\n");
}
if (method == DELAYED_METHOD_BYE || queue_head) {
@@ -1609,14 +1610,14 @@
} else {
AST_LIST_INSERT_TAIL(&session->delayed_requests, delay, next);
}
- return 0;
+ SCOPE_EXIT_RTN_VALUE(0);
}
static pjmedia_sdp_session *generate_session_refresh_sdp(struct ast_sip_session *session)
{
pjsip_inv_session *inv_session = session->inv_session;
const pjmedia_sdp_session *previous_sdp = NULL;
- SCOPE_ENTER(1, "%s\n", ast_sip_session_get_name(session));
+ SCOPE_ENTER(3, "%s\n", ast_sip_session_get_name(session));
if (inv_session->neg) {
if (pjmedia_sdp_neg_was_answer_remote(inv_session->neg)) {
@@ -2258,6 +2259,7 @@
if (pending_media_state) {
int index;
int type_streams[AST_MEDIA_TYPE_END] = {0};
+ int original_states_equal = 0;
ast_trace(-1, "%s: Pending media state exists\n", ast_sip_session_get_name(session));
@@ -2278,8 +2280,10 @@
if (active_media_state) {
struct ast_sip_session_media_state *new_pending_state;
+ original_states_equal = ast_stream_topology_equal(active_media_state->topology, pending_media_state->topology);
- ast_trace(-1, "%s: Active media state exists\n", ast_sip_session_get_name(session));
+ ast_trace(-1, "%s: Active media state exists and is%s equal to pending\n", ast_sip_session_get_name(session),
+ original_states_equal ? "" : " not");
ast_trace(-1, "%s: DP: %s\n", ast_sip_session_get_name(session), ast_str_tmp(256, ast_stream_topology_to_str(pending_media_state->topology, &STR_TMP)));
ast_trace(-1, "%s: DA: %s\n", ast_sip_session_get_name(session), ast_str_tmp(256, ast_stream_topology_to_str(active_media_state->topology, &STR_TMP)));
ast_trace(-1, "%s: CP: %s\n", ast_sip_session_get_name(session), ast_str_tmp(256, ast_stream_topology_to_str(session->pending_media_state->topology, &STR_TMP)));
@@ -2438,7 +2442,7 @@
}
/* If the resulting media state matches the existing active state don't bother doing a session refresh */
- if (ast_stream_topology_equal(session->active_media_state->topology, pending_media_state->topology)) {
+ if (!original_states_equal && ast_stream_topology_equal(session->active_media_state->topology, pending_media_state->topology)) {
ast_trace(-1, "%s: CA: %s\n", ast_sip_session_get_name(session), ast_str_tmp(256, ast_stream_topology_to_str(session->active_media_state->topology, &STR_TMP)));
ast_trace(-1, "%s: NP: %s\n", ast_sip_session_get_name(session), ast_str_tmp(256, ast_stream_topology_to_str(pending_media_state->topology, &STR_TMP)));
ast_sip_session_media_state_free(pending_media_state);
@@ -4185,34 +4189,29 @@
struct ast_sip_session_media_state *pending_media_state;
struct ast_sip_session_media_state *active_media_state;
const char *session_name = ast_sip_session_get_name(session);
-
- ast_debug(3, "%s re-INVITE collision.\n", session_name);
+ SCOPE_ENTER(3, "%s\n", session_name);
pending_media_state = ast_sip_session_media_state_clone(session->pending_media_state);
if (!pending_media_state) {
- ast_log(LOG_ERROR, "%s: Failed to clone pending media state\n", session_name);
- return;
+ SCOPE_EXIT_LOG_RTN(LOG_ERROR, "%s: Failed to clone pending media state\n", session_name);
}
active_media_state = ast_sip_session_media_state_clone(session->active_media_state);
if (!active_media_state) {
ast_sip_session_media_state_free(pending_media_state);
- ast_log(LOG_ERROR, "%s: Failed to clone active media state\n", session_name);
- return;
+ SCOPE_EXIT_LOG_RTN(LOG_ERROR, "%s: Failed to clone active media state\n", session_name);
}
if (delay_request(session, NULL, NULL, on_response, 1, DELAYED_METHOD_INVITE, pending_media_state,
active_media_state, 1)) {
ast_sip_session_media_state_free(pending_media_state);
ast_sip_session_media_state_free(active_media_state);
- ast_log(LOG_ERROR, "%s: Failed to add delayed request\n", session_name);
- return;
+ SCOPE_EXIT_LOG_RTN(LOG_ERROR, "%s: Failed to add delayed request\n", session_name);
}
if (pj_timer_entry_running(&session->rescheduled_reinvite)) {
/* Timer already running. Something weird is going on. */
- ast_log(LOG_ERROR, "%s: re-INVITE collision while timer running!!!\n", session_name);
- return;
+ SCOPE_EXIT_LOG_RTN(LOG_ERROR, "%s: re-INVITE collision while timer running!!!\n", session_name);
}
tv.sec = 0;
@@ -4227,8 +4226,10 @@
if (pjsip_endpt_schedule_timer(ast_sip_get_pjsip_endpoint(),
&session->rescheduled_reinvite, &tv) != PJ_SUCCESS) {
ao2_ref(session, -1);
- ast_log(LOG_ERROR, "%s: Couldn't schedule timer\n", session_name);
+ SCOPE_EXIT_LOG_RTN(LOG_ERROR, "%s: Couldn't schedule timer\n", session_name);
}
+
+ SCOPE_EXIT_RTN();
}
static void __print_debug_details(const char *function, pjsip_inv_session *inv, pjsip_transaction *tsx, pjsip_event *e)
--
To view, visit https://gerrit.asterisk.org/c/asterisk/+/14897
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 18
Gerrit-Change-Id: Iffd7dd0500301156a566119ebde528d1a9573314
Gerrit-Change-Number: 14897
Gerrit-PatchSet: 1
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200911/36090958/attachment-0001.html>
More information about the asterisk-code-review
mailing list