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



 <p>Ship it!</p>



 <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Ship It!</pre>
 <br />







<div>




<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/4174/diff/1/?file=68929#file68929line1180" style="color: black; font-weight: bold; text-decoration: underline;">/branches/12/res/res_pjsip_pubsub.c</a>
    <span style="font-weight: normal;">

     (Diff revision 1)

    </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 pj_bool_t pubsub_on_rx_subscribe_request(pjsip_rx_data *rdata)</pre></td>

  </tr>
 </tbody>



 
 

 <tbody>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
    <th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1180</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb">   </span><span class="tb">  </span><span class="tb">  </span><span class="kt">long</span> <span class="n">dlg_status</span> <span class="o">=</span> <span class="p">(</span><span class="kt">long</span><span class="p">)</span> <span class="n">ast_sip_mod_data_get</span><span class="p">(</span><span class="n">rdata</span><span class="o">-></span><span class="n">endpt_info</span><span class="p">.</span><span class="n">mod_data</span><span class="p">,</span> <span class="n">pubsub_module</span><span class="p">.</span><span class="n">id</span><span class="p">,</span> <span class="n">MOD_DATA_DLG_STATUS</span><span class="p">);</span></pre></td>
  </tr>

 </tbody>

</table>

<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Ew.</pre>
</div>
<br />



<p>- Joshua Colp</p>


<br />
<p>On November 12th, 2014, 9:51 p.m. UTC, Mark Michelson wrote:</p>








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

<div>Review request for Asterisk Developers.</div>
<div>By Mark Michelson.</div>


<p style="color: grey;"><i>Updated Nov. 12, 2014, 9:51 p.m.</i></p>









<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</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;">During testing, an odd situation was encountered. In the test, a phone sent an INVITE to Asterisk. A half second later, after receiving no response, the phone retransmits the INVITE to Asterisk. About this time, Asterisk starts to process both incoming INVITEs at the same time in separate threads.

In thread 1, a dialog is successfully created, a 100 Trying response is sent, and the call is sent into the dialplan.
In thread 2, the dialog cannot be created because thread 1 has already created a transaction in PJSIP with the same details. The collision results in thread 2 sending a 500 response to the phone.

At this point, the phone has received an error final response, so the phone assumes the call is failed. However, Asterisk has a successful dialog going, still, so Asterisk continues on with the call. This results in some "fun" situations. Luckily, the situations haven't proven fatal for Asterisk, but they are very confusing for people involved in the calls.

The solution proposed to fix this problem is to not respond to incoming requests if attempting to create a transaction results in the PJ_EEXISTS error. The logic is that if PJ_EEXISTS is returned, that means that elsewhere, we have already successfully created a transaction for this request and we can safely ignore this one.

After auditing the code, the only places that required changes were the places that created dialogs based on incoming requests. Places that create out-of-dialog stateful responses were not reacting to errors by sending stateless responses.

The actual change implemented here is to modify ast_sip_create_dialog_uas() to take an additional parameter that is the status returned from PJSIP when attempting to create the dialog. This way, we can react accordingly if the dialog cannot be created. The Asterisk 12 changes are presented here. The Asterisk 13 changes are on /r/4175</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;">I ran the testsuite nominal incoming calls tests and the presence subscription tests to be sure that this change did not adversely affect them. They still pass.</pre>
  </td>
 </tr>
</table>


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

 <li>/branches/12/res/res_pjsip_session.c <span style="color: grey">(427735)</span></li>

 <li>/branches/12/res/res_pjsip_pubsub.c <span style="color: grey">(427735)</span></li>

 <li>/branches/12/res/res_pjsip.c <span style="color: grey">(427735)</span></li>

 <li>/branches/12/include/asterisk/res_pjsip.h <span style="color: grey">(427735)</span></li>

</ul>

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







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








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