<p>Richard Mudgett <strong>posted comments</strong> on this change.</p><p><a href="https://gerrit.asterisk.org/8374">View Change</a></p><p>Patch set 7:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4;">Code-Review -1</span></p><p style="white-space: pre-wrap; word-wrap: break-word;">While creating the patch for the v13 branch it occurred to me that sending the NOTIFY in the dialog happened under the wrong serializer.  This is bad as it can potentially cause memory corruption.  That NOTIFY must be sent using the channel session's serializer.</p><p>(3 comments)</p><ul style="list-style: none; padding-left: 20px;"><li><p><a href="https://gerrit.asterisk.org/#/c/8374/7/res/res_pjsip_notify.c">File res/res_pjsip_notify.c:</a></p><ul style="list-style: none; padding-left: 20px;"><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/8374/7/res/res_pjsip_notify.c@303">Patch Set #7, Line 303:</a> <code style="font-family:monospace,monospace">    struct ast_channel *channel;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This should be changed to a struct ast_sip_session *session pointer.</p></li><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/8374/7/res/res_pjsip_notify.c@739">Patch Set #7, Line 739:</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;"> dlg = ast_sip_session_get_dialog_for_channel(data->channel);<br>       /* keep the channel locked so the dialog can't go away */<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">When run under the channel session's serializer the dialog won't go away after you check it's validity.  Also I don't think keeping the channel lock prevents the dialog from going away anyway.</p></li><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/8374/7/res/res_pjsip_notify.c@863">Patch Set #7, Line 863:</a> <code style="font-family:monospace,monospace">     if (ast_sip_push_task(NULL, notify_channel, data)) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This should be pushing the notify_channel task onto the channel session's serializer.  This ensures that NOTIFY messages to the same channel go out in the same order if they are rapidly sent.  Also it will avoid potential threading issues.</p><p style="white-space: pre-wrap; word-wrap: break-word;">An example that can be used is chan_pjsip.c:chan_pjsip_sendtext().  That function sends MESSAGE requests in dialog.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/8374">change 8374</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/8374"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 15 </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: If7f3151a6d633e414d5dc319d5efc1443c43dd29 </div>
<div style="display:none"> Gerrit-Change-Number: 8374 </div>
<div style="display:none"> Gerrit-PatchSet: 7 </div>
<div style="display:none"> Gerrit-Owner: Nathan Bruning <nathan@iperity.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: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Sean Bright <sean.bright@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: lvl <digium@lvlconsultancy.nl> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 14 Mar 2018 17:16:52 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>