<blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">(3 comments)</p><p style="white-space: pre-wrap; word-wrap: break-word;">Please cherry pick this to 16, 17, 17.0, and master branches.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">It looks like in 16, 17 and master branch has the rtp instance bump already implemented. So, I think, those versions are safe, no need to apply this patch.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/13044">View Change</a></p><p>3 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/#/c/13044/1/channels/pjsip/cli_commands.c">File channels/pjsip/cli_commands.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/13044/1/channels/pjsip/cli_commands.c@369">Patch Set #1, Line 369:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">    if (!media || !media->rtp) {<br>               ast_str_append(&context->output_buffer, 0, " %s not valid\n", snapshot->name);<br>            ast_channel_unlock(channel);<br>          ao2_cleanup(channel);<br>         return 0;<br>     }<br><br>   rtp = ao2_bump(media->rtp);<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I think there still exists the potential for rtp to be NULL here. ao2_bump is NULL tolerant. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/13044/1/channels/pjsip/cli_commands.c@383">Patch Set #1, Line 383:</a> <code style="font-family:monospace,monospace">    stats_res = ast_rtp_instance_get_stats(rtp, &stats, AST_RTP_INSTANCE_STAT_ALL);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Why does this need to be done with the channel locked?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">My understanding is, As we bumped the rtp reference and lock rtp inside ast_rtp_instance_get_stats, this would be enough to hold rtp session until finish the status read. I have little confusion on, is there any chance to cleanup the channel just before the rtp session get locked? If it cleanup then does it destroy the rtp object either? Please correct me if I am thinking wrong.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/13044/1/channels/pjsip/cli_commands.c@418">Patch Set #1, Line 418:</a> <code style="font-family:monospace,monospace">      ao2_cleanup(rtp);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Very minor, but you could clean up the ref right after the call to get_stats if you want as it appea […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/13044">change 13044</a>. To unsubscribe, or for help writing mail filters, 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/c/asterisk/+/13044"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13 </div>
<div style="display:none"> Gerrit-Change-Id: Ia918bfa3c8119060633e39b14852e8d983e0417e </div>
<div style="display:none"> Gerrit-Change-Number: 13044 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Salah Ahmed <txrubel@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Salah Ahmed <txrubel@gmail.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 16 Oct 2019 06:56:36 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Comment-In-Reply-To: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>