No subject

e iterator is the same.  The AO2_ITERATOR_SNAPSHOT lets us take a few advan=
tages with the actual iterating - since the iterator owns the copied contai=
ner and the objects inside are ref'd, we no longer need to lock the con=
tainer when we traverse it.  The AO2_ITERATOR_DONTLOCK flag is added by def=
ault when this flag is used.

Tested with 1.8.  Used valgrind to check for any introduced memory leaks, u=
sed the ref counting debug option to check for ref counting errors.  Checke=
d with multiple calls to "sip show peers", as well as reloads of =
chan_sip.</pre>
  </td>
 </tr>
</table>


<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;">The SIP show peers command stores pointers to the current pe=
ers in the peers container in a fixed size array.  The size of this array i=
s determined a bit before iterating through the peers container.  The conta=
iner is not locked during the array initialization, nor while the iterator =
is initialized.  There are at least two problems with this:
1) The array is allocated at the beginning of the method.  There are a numb=
er of exit points from the function that occur before the array is dealloca=
ted, resulting in a memory leak.
2) The array size is supposed to be the current number of peers in the cont=
ainer.  However, the peers container is not locked when the size is determi=
ned, and the version of the iterator is not set until ao2_iterator_next is =
called.  Thus, any items added to the peers container prior to the first tr=
aversal of the container will be &quot;counted&quot; during the traversal, =
and the array can be overrun.

This would typically occur if some other thread performed a sip reload at t=
he same time as the sip show peers command, and additional peers were added=
 in between when the array was sized and when the peers were traversed.

This patch takes a bit more of a pessimistic view of the thread-safety invo=
lved with showing the peers.  We lock the container during the period that =
we build the array for sorting and when we initialize the ao2_iterator.  Th=
e ao2_iterator initialization has been modified as well to set the version =
to the container version, as opposed to waiting for the first traversal of =
the container.  This should be safe:
1) If an item is added to the container prior to/while traversing it, the v=
ersion number of that item will be greater then the version of iterator, an=
d will be ignored
2) If an item is removed from the container prior to/while traversing it, t=
he number of items added to the array will be less then the array size, whi=
ch is acceptable

Note that you can still have a situation where a reload occurs during a sip=
 show peers which changes what the command will output while it is building=
 the peer array; however, that&#39;s no different then the current behavior=
.  The only way to prevent that would be to lock the peers container during=
 the entire traversal, which would potentially impact performance on normal=
 SIP operations.</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;">Basic testing involving running sip show peers and reloading=
.  More testing will need to be done by the issue reporter, who was perform=
ing &quot;sip reloads&quot; alongside a script that ran &quot;sip show peer=
s&quot; once a second.</pre>
  </td>
 </tr>
</table>




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

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

 <li>/branches/1.8/include/asterisk/astobj2.h <span style=3D"color: grey">(=
355276)</span></li>

 <li>/branches/1.8/main/astobj2.c <span style=3D"color: grey">(355276)</spa=
n></li>

</ul>

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




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




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


--===============8489260289899266696==--