<p>Joshua Colp <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/6550">View Change</a></p><div style="white-space:pre-wrap">Approvals:
  Joshua Colp: Looks good to me, but someone else must approve; Approved for Submit
  Kevin Harwell: Looks good to me, approved

</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_pjsip_session:  Change some asserts to warning/debug messages<br><br>There was an issue reported where an SDP received on a 183 Session<br>Progress message caused a crash because the pending streams had<br>already been processed when the OK was received.  In that case the<br>pending topology was legitimately NULL.  There was an assert for an<br>incorrect number of streams in the topology but not one for<br>topology being NULL.  In any case, if you're not in dev-mode the<br>asserts don't do anything and since the scenario is legit, the<br>asserts weren't appropriate anyway.<br><br>* Changed several asserts to warning or debug messages and return<br>codes as appropriate.<br><br>ASTERISK-27264<br>Reported by: Daniel Heckl<br><br>Change-Id: I58daaa9d2938fa980857ab3ec41925ab5ff9c848<br>---<br>M res/res_pjsip_session.c<br>1 file changed, 29 insertions(+), 12 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c<br>index 64416a0..978c95a 100644<br>--- a/res/res_pjsip_session.c<br>+++ b/res/res_pjsip_session.c<br>@@ -277,7 +277,11 @@<br> {<br>    int index;<br> <br>-        ast_assert(session->pending_media_state->topology != NULL);<br>+    if (!session->pending_media_state->topology) {<br>+         ast_log(LOG_WARNING, "Pending topology was NULL for channel '%s'\n",<br>+                       session->channel ? ast_channel_name(session->channel) : "unknown");<br>+          return 0;<br>+    }<br> <br>  if (ast_stream_get_state(stream) == AST_STREAM_STATE_REMOVED) {<br>               return 0;<br>@@ -766,6 +770,30 @@<br>       int i;<br>        struct ast_stream_topology *topology;<br> <br>+     /* This situation can legitimately happen when an SDP is received in a<br>+        * 183 Session Progress message.  In that case, everything's been done<br>+    * by the time this function is called and there are no more pending<br>+  * streams.<br>+   */<br>+  if (!session->pending_media_state->topology) {<br>+         ast_debug(1, "Pending topology was NULL for channel '%s'\n",<br>+                       session->channel ? ast_channel_name(session->channel) : "unknown");<br>+          return 0;<br>+    }<br>+<br>+ /* If we're handling negotiated streams, then we should already have set<br>+  * up session media instances (and Asterisk streams) that correspond to<br>+       * the local SDP, and there should be the same number of session medias<br>+       * and streams as there are local SDP streams<br>+         */<br>+  if (ast_stream_topology_get_count(session->pending_media_state->topology) != local->media_count<br>+             || AST_VECTOR_SIZE(&session->pending_media_state->sessions) != local->media_count) {<br>+            ast_log(LOG_WARNING, "Local SDP for channel '%s' contains %d media streams while we expected it to contain %u\n",<br>+                  session->channel ? ast_channel_name(session->channel) : "unknown",<br>+                   ast_stream_topology_get_count(session->pending_media_state->topology), local->media_count);<br>+         return -1;<br>+   }<br>+<br>  for (i = 0; i < local->media_count; ++i) {<br>              struct ast_sip_session_media *session_media;<br>          struct ast_stream *stream;<br>@@ -773,14 +801,6 @@<br>              if (!remote->media[i]) {<br>                   continue;<br>             }<br>-<br>-         /* If we're handling negotiated streams, then we should already have set<br>-          * up session media instances (and Asterisk streams) that correspond to<br>-               * the local SDP, and there should be the same number of session medias<br>-               * and streams as there are local SDP streams<br>-                 */<br>-          ast_assert(i < AST_VECTOR_SIZE(&session->pending_media_state->sessions));<br>-               ast_assert(i < ast_stream_topology_get_count(session->pending_media_state->topology));<br> <br>            session_media = AST_VECTOR_GET(&session->pending_media_state->sessions, i);<br>                 stream = ast_stream_topology_get_stream(session->pending_media_state->topology, i);<br>@@ -815,9 +835,6 @@<br>                if (!remote->media[i]) {<br>                   continue;<br>             }<br>-<br>-         ast_assert(i < AST_VECTOR_SIZE(&session->pending_media_state->sessions));<br>-               ast_assert(i < ast_stream_topology_get_count(session->pending_media_state->topology));<br> <br>            session_media = AST_VECTOR_GET(&session->pending_media_state->sessions, i);<br>                 stream = ast_stream_topology_get_stream(session->pending_media_state->topology, i);<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/6550">change 6550</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/6550"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I58daaa9d2938fa980857ab3ec41925ab5ff9c848 </div>
<div style="display:none"> Gerrit-Change-Number: 6550 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>