No subject

r to determine if the peer was already destroyed (we'll get the "b=
ad magic number" error when we try to ref it if it is already destroye=
d).  I'm not sure if this gives us anything new though, as the ao2_lock=
 in sip_send_mwi_to_peer() should offer the same debug information.</pre>
</div>
<br />



<p>- David</p>


<br />
<p>On December 6th, 2011, 10:10 a.m., mjordan wrote:</p>






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

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


<p style=3D"color: grey;"><i>Updated Dec. 6, 2011, 10:10 a.m.</i></p>




<h1 style=3D"color: #575012; font-size: 10pt; margin-top: 1.5em;">Descripti=
on </h1>
<table width=3D"100%" bgcolor=3D"#ffffff" cellspacing=3D"0" cellpadding=3D"=
10" style=3D"border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style=3D"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 sett=
ing &#39;allowsubscribe=3Dyes&#39;, &#39;callercounter =3D yes&#39; and set=
ting the subscribecontext in chan_sip.  When the deadlock was resolved by r=
345063, a crash would occur in chan_sip.  This would manifest when an MWI n=
otification 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 be=
ing 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) woul=
d 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 pe=
er 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 e=
vent 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 ignore=
d that the relatedpeer, set to the authpeer, was still used later in the me=
thod
3. It fixes a potential bug wherein an authentication result could be posit=
ive, but all failures are assumed to be negative</pre>
  </td>
 </tr>
</table>


<h1 style=3D"color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing <=
/h1>
<table width=3D"100%" bgcolor=3D"#ffffff" cellspacing=3D"0" cellpadding=3D"=
10" style=3D"border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style=3D"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.  Thi=
s included using two SIP phones with BLF and MWI subscriptions, with multip=
le mailboxes defined for various extensions, and module unloading / reloadi=
ng chan_sip at various times (both before SUBSCRIBE messages were received =
and after multiple SUBSCRIBE messages had been recevied).  The patch was al=
so confirmed to resolve the issue by the issue reporter.</pre>
  </td>
 </tr>
</table>



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


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


</div>


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

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

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

</ul>

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




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








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


--===============0560041178053269719==--