[Asterisk-code-review] res pjsip session: Change some asserts to warning/debug mes... (asterisk[15.0])

[George Joseph]

George Joseph has uploaded this change for review. ( https://gerrit.asterisk.org/6549


Change subject: res_pjsip_session:  Change some asserts to warning/debug messages
......................................................................

res_pjsip_session:  Change some asserts to warning/debug messages

There was an issue reported where an SDP received on a 183 Session
Progress message caused a crash because the pending streams had
already been processed when the OK was received.  In that case the
pending topology was legitimately NULL.  There was an assert for an
incorrect number of streams in the topology but not one for
topology being NULL.  In any case, if you're not in dev-mode the
asserts don't do anything and since the scenario is legit, the
asserts weren't appropriate anyway.

* Changed several asserts to warning or debug messages and return
codes as appropriate.

ASTERISK-27264
Reported by: Daniel Heckl

Change-Id: I58daaa9d2938fa980857ab3ec41925ab5ff9c848
---
M res/res_pjsip_session.c
1 file changed, 31 insertions(+), 12 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/49/6549/1

diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index b6fa8bb..bd4b9cf 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -277,7 +277,11 @@
 {
 	int index;
 
-	ast_assert(session->pending_media_state->topology != NULL);
+	if (!session->pending_media_state->topology) {
+		ast_log(LOG_WARNING, "Pending topology was NULL for channel '%s'\n",
+			session->channel ? ast_channel_name(session->channel) : "unknown");
+		return 0;
+	}
 
 	if (ast_stream_get_state(stream) == AST_STREAM_STATE_REMOVED) {
 		return 0;
@@ -766,6 +770,32 @@
 	int i;
 	struct ast_stream_topology *topology;
 
+	/* This situation can legitimately happen when an SDP is received in a
+	 * 183 Session Progress message.  In that case, everything's been done
+	 * by the time this function is called and there are no more pending
+	 * streams.
+	 */
+	if (!session->pending_media_state->topology) {
+		ast_debug(1, "Pending topology was NULL for channel '%s'\n",
+			session->channel ? ast_channel_name(session->channel) : "unknown");
+		return 0;
+	}
+
+	/* If we're handling negotiated streams, then we should already have set
+	 * up session media instances (and Asterisk streams) that correspond to
+	 * the local SDP, and there should be the same number of session medias
+	 * and streams as there are local SDP streams
+	 */
+	if (ast_stream_topology_get_count(session->pending_media_state->topology) != local->media_count
+		|| AST_VECTOR_SIZE(&session->pending_media_state->sessions) != local->media_count) {
+		ast_log(LOG_WARNING, "Pending topology or pending sessions count mismatch for channel '%s': "
+			"%d != %u or %lu != %u\n",
+			session->channel ? ast_channel_name(session->channel) : "unknown",
+			ast_stream_topology_get_count(session->pending_media_state->topology), local->media_count,
+			AST_VECTOR_SIZE(&session->pending_media_state->sessions),  local->media_count);
+		return -1;
+	}
+
 	for (i = 0; i < local->media_count; ++i) {
 		struct ast_sip_session_media *session_media;
 		struct ast_stream *stream;
@@ -773,14 +803,6 @@
 		if (!remote->media[i]) {
 			continue;
 		}
-
-		/* If we're handling negotiated streams, then we should already have set
-		 * up session media instances (and Asterisk streams) that correspond to
-		 * the local SDP, and there should be the same number of session medias
-		 * and streams as there are local SDP streams
-		 */
-		ast_assert(i < AST_VECTOR_SIZE(&session->pending_media_state->sessions));
-		ast_assert(i < ast_stream_topology_get_count(session->pending_media_state->topology));
 
 		session_media = AST_VECTOR_GET(&session->pending_media_state->sessions, i);
 		stream = ast_stream_topology_get_stream(session->pending_media_state->topology, i);
@@ -815,9 +837,6 @@
 		if (!remote->media[i]) {
 			continue;
 		}
-
-		ast_assert(i < AST_VECTOR_SIZE(&session->pending_media_state->sessions));
-		ast_assert(i < ast_stream_topology_get_count(session->pending_media_state->topology));
 
 		session_media = AST_VECTOR_GET(&session->pending_media_state->sessions, i);
 		stream = ast_stream_topology_get_stream(session->pending_media_state->topology, i);

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

Gerrit-Project: asterisk
Gerrit-Branch: 15.0
Gerrit-MessageType: newchange
Gerrit-Change-Id: I58daaa9d2938fa980857ab3ec41925ab5ff9c848
Gerrit-Change-Number: 6549
Gerrit-PatchSet: 1
Gerrit-Owner: George Joseph <gjoseph at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20170920/341c2e8a/attachment.html>