<p>George Joseph has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/6549">View Change</a></p><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, 31 insertions(+), 12 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/49/6549/1</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 b6fa8bb..bd4b9cf 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,32 @@<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, "Pending topology or pending sessions count mismatch for channel '%s': "<br>+ "%d != %u or %lu != %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>+ AST_VECTOR_SIZE(&session->pending_media_state->sessions), 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 +803,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 +837,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/6549">change 6549</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/6549"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 15.0 </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: I58daaa9d2938fa980857ab3ec41925ab5ff9c848 </div>
<div style="display:none"> Gerrit-Change-Number: 6549 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: George Joseph <gjoseph@digium.com> </div>