<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
    <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="https://reviewboard.asterisk.org/r/1610/">https://reviewboard.asterisk.org/r/1610/</a>
     </td>
    </tr>
   </table>
   <br />


<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
 <tr>
  <td>

<div>Review request for Asterisk Developers, David Vossel and opticron.</div>
<div>By mjordan.</div>





<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">ASTERISK-18663 originally manifested as a deadlock when setting &#39;allowsubscribe=yes&#39;, &#39;callercounter = yes&#39; and setting the subscribecontext in chan_sip.  When the deadlock was resolved by r345063, a crash would occur in chan_sip.  This would manifest when an MWI notification was to be sent to a peer, but the peer had been deleted due to being dereferenced to a ref count of 0.  The root cause of this ended up being the MWI event subscription being resubscribed to in several places, and orphaning the previous event subscription.  When an MWI event would occur, all of the event subscriptions (including the orphaned subscriptions) would be notified.  This didn&#39;t cause any issues until a peer was removed, either by pruning realtime SIP peers, unloading chan_sip, etc.  When the peer cleaned itself up, it only removes the subscription that it&#39;s aware of - the orphaned subscriptions would continue to exist and, if a new MWI event occurred, would crash Asterisk by referencing the deleted peer.

This patch does several things:
1. It resolves the issue in subscribing to the MWI event callback by first unsubscribing the old event subscription
2. It more aggressively holds the authpeer in handle_request_subscribe and removes some unneeded peer ref&#39;ing / deref&#39;ing.  This was done more for clarity, as the previous location of deref&#39;ing the authpeer ignored that the relatedpeer, set to the authpeer, was still used later in the method
3. It fixes a potential bug wherein an authentication result could be positive, but all failures are assumed to be negative</pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Testing was done extensively using 1.8 and 1.8.8.0-rc4.  This included using two SIP phones with BLF and MWI subscriptions, with multiple mailboxes defined for various extensions, and module unloading / reloading chan_sip at various times (both before SUBSCRIBE messages were received and after multiple SUBSCRIBE messages had been recevied).  The patch was also confirmed to resolve the issue by the issue reporter.</pre>
  </td>
 </tr>
</table>



<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>


 <a href="https://issues.asterisk.org/jira/browse/ASTERISK-18663">ASTERISK-18663</a>


</div>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>/branches/1.8/channels/chan_sip.c <span style="color: grey">(347057)</span></li>

 <li>/branches/1.8/channels/sip/include/sip.h <span style="color: grey">(347057)</span></li>

</ul>

<p><a href="https://reviewboard.asterisk.org/r/1610/diff/" style="margin-left: 3em;">View Diff</a></p>




  </td>
 </tr>
</table>




  </div>
 </body>
</html>