<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/1101/">https://reviewboard.asterisk.org/r/1101/</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 26th, 2011, 5:36 p.m., <b>David Vossel</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/1101/diff/2/?file=15662#file15662line2697" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/channels/chan_sip.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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 void unlink_peer_from_tables(struct sip_peer *peer)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">2695</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="p">{</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">2697</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="p">{</span></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">2698</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cp">#ifdef REF_DEBUG</span></pre></td>
</tr>
<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">2699</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">__ao2_ref_debug</span><span class="p">(</span><span class="n">peer</span><span class="p">,</span> <span class="o">-</span><span class="mi">1</span><span class="p">,</span> <span class="n">tag</span><span class="p">,</span> <span class="n">file</span><span class="p">,</span> <span class="n">line</span><span class="p">,</span> <span class="n">func</span><span class="p">);</span></pre></td>
</tr>
<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">2700</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cp">#else</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">2696</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">ao2_t_ref</span><span class="p">(</span><span class="n">peer</span><span class="p">,</span> <span class="o">-</span><span class="mi">1</span><span class="p">,</span> <span class="n">tag</span><span class="p">);</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">2701</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">ao2_t_ref</span><span class="p">(</span><span class="n">peer</span><span class="p">,</span> <span class="o">-</span><span class="mi">1</span><span class="p">,</span> <span class="n">tag</span><span class="p">);</span></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">2702</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cp">#endif</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">2697</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="nb">NULL</span><span class="p">;</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">2703</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="nb">NULL</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;">Isn't this what is already done in the astobj2.h header file?</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;">This might depend on the version of asterisk you are patching. I started with 1.4, and then migrated the patch up to 1.6.x family, then 1.8 and the trunk.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 26th, 2011, 5:36 p.m., <b>David Vossel</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/1101/diff/2/?file=15662#file15662line2762" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/channels/chan_sip.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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 void *dialog_unlink_rtpcheck(struct sip_pvt *dialog)</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">2762</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">dialog</span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
<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">2763</font></th>
<td bgcolor="#c5ffc4" 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_ERROR</span><span class="p">,</span> <span class="s">"Attempt unlink a null dialog</span><span class="se">\n</span><span class="s">"</span><span class="p">);</span></pre></td>
</tr>
<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">2764</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="nb">NULL</span><span class="p">;</span></pre></td>
</tr>
<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">2765</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <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;">If this is happening, this is really bad.</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;">Yes it would be bad, but not putting code in here to protect if that were to happen would be catastrophic. Other places in the code verify a pointer is valid before using it, why not here?</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 26th, 2011, 5:36 p.m., <b>David Vossel</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/1101/diff/2/?file=15662#file15662line24748" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/channels/chan_sip.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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 sip_send_mwi_to_peer(struct sip_peer *peer, const struct ast_event *event, int cache_only)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">24689</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">dialog_unlink_all</span><span class="p">(</span><span class="n">p</span><span class="p">,</span> <span class="n">TRUE</span><span class="p">,</span> <span class="n">TRUE</span><span class="p">);</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">24708</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">dialog_unlink_all</span><span class="p">(</span><span class="n">p</span><span class="p">,</span> <span class="n">TRUE</span><span class="p">,</span> <span class="n">TRUE</span><span class="p">);</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">24690</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">dialog_unref</span><span class="p">(</span><span class="n">p</span><span class="p">,</span> <span class="s">"<span class="hl">unref</span> dialog<span class="hl"> p just created via sip_alloc</span>"</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">24709</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">dialog_unref</span><span class="p">(</span><span class="n">p</span><span class="p">,</span> <span class="s">"<span class="hl">destroy</span> dialog"</span><span class="p">);</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">24691</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="cm">/* sip_destroy(p); */</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">24692</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="mi">0</span><span class="p">;</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">24710</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="mi">0</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;">I don't see any reason to reduce the debug information here. This happens several other places as well.</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;">The information is not reduced. It is made more helpful. The string only appears with REF_DEBUG enabled, and that will automatically cause the FILE, FUNCTION, LINE to appear in the refcounter output file. Having the "destroy dialog" messages all the same makes for easier debugging because you can match up "destroy dialog" messages with their corresponding "allocation" messages.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 26th, 2011, 5:36 p.m., <b>David Vossel</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/1101/diff/2/?file=15662#file15662line24968" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/channels/chan_sip.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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 void stop_session_timer(struct sip_pvt *p)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">24908</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">p</span><span class="o">-></span><span class="n">stimer</span><span class="p">)</span> <span class="p">{</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">24926</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">p</span><span class="o">-></span><span class="n">stimer</span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">24909</font></th>
<td bgcolor="#ffc5ce" 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">"Null stimer in stop_session_timer - %s</span><span class="se">\n</span><span class="s">"</span><span class="p">,</span> <span class="n">p</span><span class="o">-></span><span class="n">callid</span><span class="p">);</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">24910</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span><span class="p">;</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">24927</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</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;">I don't see why we need to remove this log msg.</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;">It's a useless message. It fills the log files for no reason. Like deleting a non-existent file. The end result is the file is gone.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 26th, 2011, 5:36 p.m., <b>David Vossel</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/1101/diff/2/?file=15662#file15662line25082" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/channels/chan_sip.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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 proc_session_timer(const void *vp)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">25009</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">stop_session_timer</span><span class="p">(</span><span class="n">p</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">25038</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">p</span><span class="o">-></span><span class="n">stimer</span><span class="o">-></span><span class="n">st_active</span> <span class="o">=</span> <span class="n">FALSE</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;">This doesn't have to change, the unref in stop_session_timer doesn't do anything if st_schedid is -1 thanks to the AST_SCHED_DEL_UNREF macro.</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;">Yes. It does need to change. There is a logic difference between what happens here in proc_session_timer(), and what stop_session_timer() does, or should not do for us while we are in here.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 26th, 2011, 5:36 p.m., <b>David Vossel</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/1101/diff/2/?file=15662#file15662line25283" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/channels/chan_sip.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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 sip_poke_noanswer(const void *data)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">25210</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">dialog_unlink_all</span><span class="p">(</span><span class="n">peer</span><span class="o">-></span><span class="n">call</span><span class="p">,</span> <span class="n">TRUE</span><span class="p">,</span> <span class="n">TRUE</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">25239</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">dialog_unref</span><span class="p">(</span><span class="n">peer</span><span class="o">-></span><span class="n">call</span><span class="p">,</span> <span class="s">"unref dialog peer->call"</span><span class="p">);</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">25211</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">peer</span><span class="o">-></span><span class="n">call</span> <span class="o">=</span> <span class="n">dialog_un<span class="hl">ref</span></span><span class="p">(</span><span class="n">peer</span><span class="o">-></span><span class="n">call</span><span class="p">,</span> <span class="s"><span class="hl">"unref dialog peer->call"</span></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">25240</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">peer</span><span class="o">-></span><span class="n">call</span> <span class="o">=</span> <span class="n">dialog_un<span class="hl">link_all</span></span><span class="p">(</span><span class="n">peer</span><span class="o">-></span><span class="n">call</span><span class="p">,</span> <span class="n"><span class="hl">TRUE</span></span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="n"><span class="hl">TRUE</span></span><span class="p">);</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">25212</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="cm">/* peer->call = sip_destroy(peer->call);*/</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></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;">Perhaps a comment explaining the order of this since the reasoning for this reversal is a little tricky.</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;">The comment for this change is in step 22 under the chan_sip.c file changes. I thought it was clear. Let me know if not</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 26th, 2011, 5:36 p.m., <b>David Vossel</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/1101/diff/2/?file=15662#file15662line25333" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/channels/chan_sip.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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 sip_poke_peer(struct sip_peer *peer, int force)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">25260</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">dialog_unlink_all</span><span class="p">(</span><span class="n">peer</span><span class="o">-></span><span class="n">call</span><span class="p">,</span> <span class="n">TRUE</span><span class="p">,</span> <span class="n">TRUE</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">25288</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">dialog_unref</span><span class="p">(</span><span class="n">peer</span><span class="o">-></span><span class="n">call</span><span class="p">,</span> <span class="s">"unref dialog peer->call"</span><span class="p">);</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">25261</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">peer</span><span class="o">-></span><span class="n">call</span> <span class="o">=</span> <span class="n">dialog_un<span class="hl">ref</span></span><span class="p">(</span><span class="n">peer</span><span class="o">-></span><span class="n">call</span><span class="p">,</span> <span class="s"><span class="hl">"unref dialog peer->call"</span></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">25289</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">peer</span><span class="o">-></span><span class="n">call</span> <span class="o">=</span> <span class="n">dialog_un<span class="hl">link_all</span></span><span class="p">(</span><span class="n">peer</span><span class="o">-></span><span class="n">call</span><span class="p">,</span> <span class="n"><span class="hl">TRUE</span></span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="n"><span class="hl">TRUE</span></span><span class="p">);</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">25262</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="cm">/* peer->call = sip_destroy(peer->call); */</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></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;">A comment here would be nice as well.</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;">The notes for step 22 in the "readme" above apply here for the same reason.
Without the change:
dialog_unlink_all() will nullify peer->call
peer->call is then passed to dialog_unref() in the next line
Since it is null, dialog_unref() won't do anything
</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 26th, 2011, 5:36 p.m., <b>David Vossel</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/1101/diff/2/?file=15662#file15662line25694" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/channels/chan_sip.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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 struct ast_channel *sip_request_call(const char *type, struct ast_format_cap *cap, const struct ast_channel *requestor, void *data, int *cause)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">25620</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">tmpc</span><span class="p">)</span> <span class="p">{</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">25645</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">tmpc</span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">25621</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">dialog_unlink_all</span><span class="p">(</span><span class="n">p</span><span class="p">,</span> <span class="n">TRUE</span><span class="p">,</span> <span class="n">TRUE</span><span class="p">);</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">25646</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">dialog_unlink_all</span><span class="p">(</span><span class="n">p</span><span class="p">,</span> <span class="n">TRUE</span><span class="p">,</span> <span class="n">TRUE</span><span class="p">);</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">25622</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="cm">/* sip_destroy(p); */</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">25647</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">dialog_unref</span><span class="p">(</span><span class="n">p</span><span class="p">,</span> <span class="s">"destroy dialog"</span><span class="p">);</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">25623</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">25648</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">25624</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">dialog_unref</span><span class="p">(</span><span class="n">p</span><span class="p">,</span> <span class="s">"toss pvt ptr at end of sip_request_call"</span><span class="p">);</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></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;">I don't understand this change. It looks correct the way it was before.</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;">The logic prior would perform an unref when one was not needed unless the condition on !tmpc is met.
The original comment "toss pvt ptr at end of sip_request_call" is incorrect in what it thinks it is doing. At this point in the code, it would cause a complete de-reference of the dialog, and that is not wanted unless !tmpc happens.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 26th, 2011, 5:36 p.m., <b>David Vossel</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/1101/diff/2/?file=15662#file15662line29434" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/channels/chan_sip.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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 unload_module(void)</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">29384</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">p</span> <span class="o">=</span> <span class="n">dialog_unref</span><span class="p">(</span><span class="n">p</span><span class="p">,</span> <span class="s">"destroy dialog"</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;">How are we guaranteed this second unref will destroy the dialog?</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;">This is the unref that goes with the original sip_alloc </pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 26th, 2011, 5:36 p.m., <b>David Vossel</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/1101/diff/2/?file=15664#file15664line161" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/include/asterisk/sched.h</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">struct ast_cb_names {</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">161</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">int</span> <span class="n">numassocs</span><span class="p">;</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">161</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">int</span> <span class="n">numassocs</span><span class="p">;</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">162</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">char</span> <span class="o">*</span><span class="n">list</span><span class="p">[</span><span class="mi"><span class="hl">1</span>0</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">162</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">char</span> <span class="o">*</span><span class="n">list</span><span class="p">[</span><span class="mi"><span class="hl">2</span>0</span><span class="p">];</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">163</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">ast_sched_cb</span> <span class="n">cblist</span><span class="p">[</span><span class="mi"><span class="hl">1</span>0</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">163</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">ast_sched_cb</span> <span class="n">cblist</span><span class="p">[</span><span class="mi"><span class="hl">2</span>0</span><span class="p">];</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">164</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="p">};</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">164</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><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;">Could this cause a ABI issue?
I'm not sure if we can make this change in a release branch since it affects the size of the structure. By doing this, a chan_sip module built with this header and these changes would crash if it was loaded with an asterisk core using the older version.</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;">The structure needs to be bigger in order to hold all the function pointers for the "sip show sched" command. A better way to do this would have been to create a linked list of the pointers and names, but I do not plan on altering that capability now. The original code used a simple array. This just extends it some.
The struture is ONLY USED in chan_sip.c and solely for the use of "sip show sched"
PS: Why would you want to combine modules from different versions of asterisk?</pre>
<br />
<p>- rgagnon</p>
<br />
<p>On April 26th, 2011, 11:31 a.m., rgagnon 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, Terry Wilson, wdoekes, rgagnon, and loloski.</div>
<div>By rgagnon.</div>
<p style="color: grey;"><i>Updated 2011-04-26 11:31:33</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;">2011-02-09 by Rob Gagnon <rgagnon> Patch documentation
======================================================
This readme applies to patch files named below under issue 17255:
https://issues.asterisk.org/view.php?id=17255
branch-1.6.1-r307181-sip-dos-mem-leak-fix.diff
branch-1.6.2-r307181-sip-dos-mem-leak-fix.diff
tag-1.6.2.16.1-r307181-sip-dos-mem-leak-fix.diff
branch-1.8-r307181-sip-dos-mem-leak-fix.diff
trunk-r307181-sip-dos-mem-leak-fix.diff
As the patch created is a fair size, I am using this file to explain the
changes made to different files (in lieu of a large post to the issue).
Patches are created for branch 1.6.1, 1.6.2, and 1.8, as well as the trunk,
and tag 1.6.2.16.1.
Special thanks to "wdoekes", and "loloski" for testing, and finding the conditions under
which entries for sip.conf "register => ...." entries were causing problems.
Patches 1.6.1 and 1.6.2 change the following files:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
chan_sip.c
rtp.h
sched.h
rtp.c
sched.c
Patches 1.8 and trunk change the following files:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
chan_sip.c
sip.h
sched.h
sched.c
The changes to chan_sip.c by far are the fix to the majority of the problem
for this issue. I will explain the other files first.
rtp.c (branch 1.6.1 and 1.6.2 only)
~~~~~
Remove the forward declarations for red_write() and ast_rtcp_write() as they
will now be defined within rtp.h.
Renamed red_write() to ast_rtp_red_write() as it will basically be public now.
rtp.h (branch 1.6.1 and 1.6.2 only)
~~~~~
Adds prototypes for ast_rtcp_write() and ast_rtp_red_write() functions so
they may be reference in sip_show_sched() CLI function.
sched.c (all patches)
~~~~~~~
Add a space to align the output of CLI command "sip show sched"
sched.h (all patches)
~~~~~~~
Alter the static size of cb_list array in order to fit all the callback names
from sip_show_sched() CLI function.
sip.h (branch 1.8 and trunk only)
~~~~~
Add macros for ref_peer() and unref_peer() in a manner similar to existing
macros dialog_ref() and dialog_unref() that are defined in dialog.h.
chan_sip.c (all patches)
~~~~~~~~~~
Many alterations in this file. They are all meant to ensure proper balance of
dialog object (struct sip_pvt) reference increases and decreases throughout the
file. Due to the unbalancing of the references before this patch, the
following would occur:
- Memory leaks
- due to dialogs not being destroyed/free'd
- Denial of Service
- RTP, RTCP, and UDPTL ports are not freed if the related dialog object is
not destroyed
- Asterisk core dumps
- In some cases, a dialog reference decrease will cause it to be destroyed
when such an action is not expected, nor wanted. This leads to null
pointers being used, and then KABOOM.
Some changes made may not be in all files. Some are in all patches. Some are
just in the 1.6.x patches, and others are just in the 1.8 and trunk patches.
The notes below should indicate any differences.
1) Add forward declarations for functions that will be referenced in sip_show_sched()
2) ref_peer() and unref_peer():
(1.6.x only):
- Added macro definitions (based on the REF_DEBUG #define) around functions
in a manner similar to how dialog_ref() and dialog_unref() operate. This
allows for better reference debugging as actual chan_sip line numbers will
appear in the /tmp/refs file when invoked.
(1.8 and trunk only):
- Changed to ref_peer_debug() and unref_peer_debug() similar to how
dialog_ref_debug() and dialog_unref_debug() operate. This allows for
better reference debugging as actual chan_sip line numbers will appear in
the /tmp/refs file when invoked.
3) dialog_unlink_all():
- Add null check to ensure we don't attempt to unlink a null dialog
- Change some text in dialog_unref() and registry_unref() calls to properly
explain what they are unreferencing, and make it easier to match to the
reference creation from transmit_register(). See item 27 below for
information on fix to transmit_register().
- Add call to stop_session_timer() so an ended call is not held in memory
for the duration of the session interval
4) __sip_autodestruct():
- Add final dialog_unref() needed to balance the ao2_alloc() for the dialog
There was actually a required unref here that was commented out, but it is
actually needed. You will see why later, down in item 10 for the
find_call() function change explanation.
5) sip_cancel_destroy():
- Function always returns 0. Optimized the code to use AST_SCHED_DEL_UNREF()
6) __sip_destroy():
- Moved redundant session timer free'ing to top of function only.
- Also, the first check of p->stimer at the top of the function was causing
a required timer stop, and de-referencing to not happen.
7) interpret_t38_parameters():
- Change AST_SCHED_DEL() to AST_SCHED_DEL_UNREF() for t38 abort timer as it
is referenced in the dialog.
8) Various places:
- Change any ao2_t_ref() function calls that are for a dialog object to use
dialog_ref(), or dialog_unref() as needed.
- Change any ao2_t_ref() function calls that are for a peer object to use
ref_peer(), or unref_peer() as needed.
- Change ao2_iterator_next() to ao2_t_iterator_next() in order to trouble-
shoot output of the refcounter utility better.
9) sip_alloc():
- Remove early binding of T38 UDPTL port, as it will the code is redundant,
and will be executed in handle_request_invite() anyhow.
10) find_call():
This function is the biggest source of all dialog reference unbalancing.
The function is intended to return a locked, and referenced dialog (if it
does not return NULL). However, if it had to create a dialog object in
order to return it, it would not add a reference, only the lock. With this
function not returning a consistent number of references with its dialog,
all other unreferencing is doomed to failure. The change in this area
fixes this by adding a reference before returning the pointer to the
dialog.
11) reqprep():
- Remove add_header() call for "Require: timer"
- This is mentioned in issue #18704, but I had to include it here, as the
clients I was testing with were getting all messed up when using sessions.
12) sip_show_peers():
- Add an unref_peer() call in order to properly release the ao2_iterator
reference to the peer.
- Remove extraneous unref_peer() calls where no reference was added.
13) dialog_needdestroy():
- Similar to __sip_autodestruct(), added a dialog de-reference where a
comment mistakenly tells you not to. See item 10 about find_call()
14) sip_show_sched():
- Add names, and pointers to almost all functions that can be referenced
by the chan_sip scheduler so the CLI output will minimize the count shown
for "<unknown>".
15) handle_request_invite():
- Before setting the T38 abort timer, ensure one is not already running. If
it is, stop that one, and start a new one. Previously, t38id would be
overwritten without any regard to the reference it had to the dialog.
16) handle_request_cancel():
Another large memory leak here. Any time handle_request_cancel() was
called, any packets belonging to the dialog would be free'd, but two things
were not happening: a) possible packet data was not free'd, and b) the
packet was not being unreferenced from the dialog. The change here ensures
proper memory free'ing, as well as de-referencing.
17) restart_session_timer():
- ast_debug() log write call moved to be ABOVE the AST_SCHED_DEL_UNREF()
call.
- With is BELOW, the output will always show the timer id as -1.
18) stop_session_timer():
- Removed log write for WARNING if calling function will a NULL stimer as
this function can be called to ensure a session timer is stopped, and
there's really no need for the warning wasting space in the log.
- AST_SCHED_DEL_UNREF() moved inside a check to ensure there is a timer
running before it announces to the logs that it stopped the timer.
- ast_debug() log write call moved to be ABOVE the AST_SCHED_DEL_UNREF()
call.
- With is BELOW, the output will always show the timer id as -1.
20) start_session_timer():
- In some cases a session timer is already running when start_session_timer()
is called again (for instance, an extra "200" message reply is sent). The
change here ensures any existing session timer is stopped before a new one
is started. Per RFC4028, this is the right way to do it--IE: the timer
must start from the most recent "200" message.
- The ast_debug() message is also moved to be within the portion of the
if-statement so it does not output a successful timer start should the
process fail to start a timer. The dialog callid is also added to the
output.
21) proc_session_timer():
- A call to stop_session_timer() is removed, and replaced with a portion of
the code from stop_session_timer(). This is to avoid a double
de-referencing of the timer object is the timer is NOT to be rescheduled.
[stop_session_timer() would have de-referenced, as well as the needed
dialog_unref() also in this function]
- This change prevents core dumps!
22) sip_poke_noanswer():
- dialog_unref() and dialog_unlink_all() are reversed here. This is due to
the doubly-linked nature of the peer->call and the dialog.
- Calling dialog_unlink_all() first will leave peer->call set to NULL,
causing the needed dialog_unref() to not do anything, when it needed to
23) sip_poke_peer():
- Same reversal of dialog_unref() and dialog_unlink_all() as #22 above.
- Added a peer_ref() when xmitres == XMIT_ERROR because it will directly
call sip_poke_noanswer() without the scheduler. sip_poke_noanswer() is
guaranteed to remove a reference as it is normally a scheduled callback.
This peer_ref() call keeps the references balanced.
- Removed a dialog_unref() at the end of the function as it would cause
the dialog to be free'd, and it is still needed in memory until either
the poke expires, or the response is received.
24) sip_request_call():
- Add needed dialog_unref() following dialog_unlink_all().
25) load_module():
(1.8 and trunk only):
- Change ao2_container_alloc() for sip_monitor_instances to
ao2_t_container_alloc() for tracking in the refcounter utility.
26) unload_module():
(all versions):
- Add iterator to unreference and free peers in the peers table. Memory leak
just before chan_sip exits, but it was indicated as a problem by the
refcounter utility.
(1.8 and trunk only):
- Add ao2_t_ref() to de-reference the sip_monitor_instances table
27) transmit_register():
- Change some text in different dialog_ref() calls in order to properly
match references and unreferences that occur during dialog_unlink_all().
- Add a dialog_ref() when sip_alloc() is called in the function in order
to balance with the dialog_unref() that is called just before the
function exits.
</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;">Besides myself, 2 other people on this issue have been testing using different methods: wdoekes and loloski.
Loloski (Ronald) reports sipp tests are coming out clean, and his company is applying the patch to their production servers.
wdoekes was the first to test and find a crash location in an area where I had previously not looked. That code was modified as well, and he is now reporting running without any issues.
In my case, we were running 1.6.2.16.1 release and losing all ability to provide service within 15 minutes due to memory leaking objects holding UDP ports in use, causing a denial of service. We normally handle between 8000 and 15000 call requests per hour, so it didn't take long at all for the server that was testing 1.6.2.16.1 to die. With this patch applied, we have been running for a few days, and not a single UDP port has leaked. The CLI "sip show sched" and "sip show objects" do not show increasing timers and object references. As well, a check of the "netstat -anp" command no longer shows all the UDP ports tied up.
When running T38 enabled, 3 UDP ports are tied to each channel. The output of "core show channels verbose" in order to get a live channel count directly compares to the number of UDP ports per channel shown by checking the "netstat" output. Prior to the patch, there was no linear correlation like there should be.</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=16774">16774</a>,
<a href="https://issues.asterisk.org/view.php?id=17255">17255</a>,
<a href="https://issues.asterisk.org/view.php?id=18014">18014</a>,
<a href="https://issues.asterisk.org/view.php?id=18027">18027</a>,
<a href="https://issues.asterisk.org/view.php?id=18121">18121</a>,
<a href="https://issues.asterisk.org/view.php?id=18704">18704</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/channels/chan_sip.c <span style="color: grey">(308370)</span></li>
<li>/trunk/channels/sip/include/sip.h <span style="color: grey">(308370)</span></li>
<li>/trunk/include/asterisk/sched.h <span style="color: grey">(308370)</span></li>
<li>/trunk/main/sched.c <span style="color: grey">(308370)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1101/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>