<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/4355/">https://reviewboard.asterisk.org/r/4355/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 19th, 2015, 6:56 p.m. CST, <b>rmudgett</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;">cleanup_all_regs() should just be the ao2_callback() line and the original guts should be put into a cleanup_registration() ao2_callback function.</pre>
</blockquote>
<p>On January 19th, 2015, 7:48 p.m. CST, <b>Matt Jordan</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;">I'm not sure I understand your comment. cleanup_all_regs is being called by the ao2_callback, and is only called by the ao2_callback. Why would the structure need to be changed?</pre>
</blockquote>
<p>On January 20th, 2015, 9:30 a.m. CST, <b>rmudgett</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;">int cleanup_registration(void *obj, void *arg, flags)
{
guts of original cleanup_all_regs to destroy one registration. i.e., What you have currently converted cleanup_all_regs into.
return CMP_MATCH;
}
void cleanup_all_regs()
{
ao2_t_callback(registry_list, flags, cleanup_registration, NULL, "remove all SIP registry items");
}
This way you don't have to get the ao2_callback correct in two places. :)
</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Got it. That does make the code marginally better - changed :-)</pre>
<br />
<p>- Matt</p>
<br />
<p>On January 19th, 2015, 7:50 p.m. CST, Matt Jordan 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 Matt Jordan.</div>
<p style="color: grey;"><i>Updated Jan. 19, 2015, 7:50 p.m.</i></p>
<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/jira/browse/ASTERISK-24640">ASTERISK-24640</a>,
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-24673">ASTERISK-24673</a>
</div>
<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;">When the SIP registrations were migrated to using ao2 in what was then trunk, the explicit destruction of the registrations on module reload was removed and not replaced with an ao2 equivalent. Debugging done by Stefan Engström, the issue reporter, on ASTERISK-24673 confirmed that the reference in the registry_list container was being leaked.
Since the purpose of cleanup_all_regs is to prep a registration for destruction, this function has been converted to being an ao2_callback function callback, and an ao2_callback with OBJ_MULTIPLE | OBJ_NODATA | OBJ_UNLINK is now used to invoke the function. This cleans up each registration, but also removes it from the registration container registry_list.</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;">Created an outbound registration to a SIP trunk. Confirmed that Asterisk was registered. Commented out the "register" line sip.conf, reloaded, and confirmed that the registration was gone.
Stefan tested the patch independently and also confirmed that it fixed it on his test system.</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/13/channels/chan_sip.c <span style="color: grey">(430794)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/4355/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>