<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/1185/">https://reviewboard.asterisk.org/r/1185/</a>
     </td>
    </tr>
   </table>
   <br />








<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On April 27th, 2011, 6:29 a.m., <b>irroot</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  



<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
 <thead>
  <tr>
   <th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
    <a href="https://reviewboard.asterisk.org/r/1185/diff/4/?file=16229#file16229line5778" style="color: black; font-weight: bold; text-decoration: underline;">trunk/main/features.c</a>
    <span style="font-weight: normal;">

     (Diff revision 4)

    </span>
   </th>
  </tr>
 </thead>

 <tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
  <tr>

   <td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static int find_channel_by_group(void *obj, void *arg, void *data, int flags)</pre></td>

  </tr>
 </tbody>



 
 




 <tbody>

  <tr>
    <th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">5752</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="n">ast_stream_and_wait</span><span class="p">(</span><span class="n">cur</span><span class="p">,</span> <span class="n">pickupsound</span><span class="p">,</span> <span class="s">&quot;&quot;</span><span class="p">);</span></pre></td>
    <th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">5768</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="n">ast_log</span><span class="p">(</span><span class="n">LOG_WARNING</span><span class="p">,</span> <span class="s">&quot;Unable to masquerade &#39;%s&#39; into &#39;%s&#39;</span><span class="se">\n</span><span class="s">&quot;</span><span class="p">,</span> <span class="n">chan</span><span class="o">-&gt;</span><span class="n">name</span><span class="p">,</span> <span class="n">target</span><span class="o">-&gt;</span><span class="n">name</span><span class="p">);</span></pre></td>
  </tr>

 </tbody>

</table>

  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">There will be problems here if target is still locked when masquerade is called see r1118 i banged my head on this using ao2_callback.

this function will return with target unlocked so you will need the other bits from 1118 to handle this.

this patch is most needed thx i give it 2 opposing thumbs up.</pre>
 </blockquote>



 <p>On April 27th, 2011, 2:04 p.m., <b>Alec Davis</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">ast_do_pickup is a verbatim cut and paste of pickup_do() from app_directed_pickup().
In all previous cases the target is locked before the pickup_do().
Thus in all proposed cases the target is locked before the ast_do_pickup().

What I think you&#39;re suggesting is that all of the other pickup methods from app_directed_pickup, &#39;which seem to work&#39;, have a pre existing problem?</pre>
 </blockquote>







</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Yip there is a problem calling ast_channel_masquerade should be done without the channels been locked so before its called unlock target.

there is a deadlock with target and channels container lock from ao2_callback if target is not locked then this deadlock does not occur.

this race condition occurs when the pickup is contended by more than one channel ive added deadlock avoidance not having the channel locked going in should prevent it.

hope you find this useful the problem is using the pickup_by_group method introduces this not a original problem as such mixing them together causes this.
</pre>
<br />




<p>- irroot</p>


<br />
<p>On April 21st, 2011, 6:09 a.m., Alec Davis wrote:</p>






<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.orgrb/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.</div>
<div>By Alec Davis.</div>


<p style="color: grey;"><i>Updated 2011-04-21 06:09:52</i></p>




<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;">debugging mantis 18654, found common code, of which 1 set was wrong, the ast_pickup_call() code in features.c
The actual fault was ast_pickup_call where the target chan was unlocked too early - see bug18654.diff2.txt

Moved app_directed:pickup_do() to features:ast_do_pickup().

Now functions below all now use the new ast_do_pickup()
app_directed_pickup.c:
   pickup_by_channel()
   pickup_by_exten()
   pickup_by_mark()
   pickup_by_part()
features.c:
   ast_pickup_call() </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;">pickup using *8 feature code, with pickup sounds enabled/disabled

exten =&gt; 71,1,Pickup()           ; any ringing extension in same pickupgroup 
exten =&gt; 72,1,Pickup(85@phones)  ; dahdi extension
exten =&gt; 73,1,Pickup(823@phones) ; sip extension
</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/view.php?id=18654">18654</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>trunk/apps/app_directed_pickup.c <span style="color: grey">(314594)</span></li>

 <li>trunk/channels/chan_sip.c <span style="color: grey">(314594)</span></li>

 <li>trunk/include/asterisk/features.h <span style="color: grey">(314594)</span></li>

 <li>trunk/main/features.c <span style="color: grey">(314594)</span></li>

 <li>trunk/res/res_features.exports.in <span style="color: grey">(314594)</span></li>

</ul>

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




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








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