[asterisk-dev] [Code Review]: I broke parking reload in 1.6.2.19. Here's my effort to fix it.

[jrose]

> On Aug. 1, 2011, 4:09 p.m., rmudgett wrote:
> > /branches/1.6.2/main/features.c, lines 4254-4258
> > <https://reviewboard.asterisk.org/r/1337/diff/1/?file=17692#file17692line4254>
> >
> >     p reference leak if p == default_parkinglot
> >     
> >     Move unref to after if test.

Heh, Fixed it in the last commit.  Actually, it shouldn't make too much of negative impact this way since default_parkinglot should be impossible to deference down to zero anyway (I think), and all this would do is continually add one reference to it every reload... unless the user reloaded it some 2^31 times or some such business and caused an overflow.  But it's improper and would have reflected negatively on my cred, so thanks for the catch.


- jrose


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1337/#review3956
-----------------------------------------------------------


On Aug. 1, 2011, 3:49 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1337/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2011, 3:49 p.m.)
> 
> 
> Review request for Asterisk Developers, Jason Parker, David Vossel, and rmudgett.
> 
> 
> Summary
> -------
> 
> I caused an intermittent crash when I backported a bug fix to clear parking lots before reloading (so that they wouldn't be left over).  In 1.8, immediately after the default parking lot would be rebuilt from scratch.  In 1.6.2 however, it would check for the presence of the default parking lot which would at this point have become a dangling pointer.  Back in 1.6.2 deleting the default parking lot was totally unnecessary (and to be blunt, I'm still not sure it's necessary in 1.8 and up, but it doesn't break anything there, so no problem).
> 
> Anyway, this patch just makes it so that while iterating through the parking lots that the default parking lot is never destroyed.  This patch only applies to 1.6.2 and will require an update to 1.6.2.19  Also should go in svn 1.6.2
> 
> Thanks to rmudgett and jparker for helping to spot the problem.
> 
> 
> This addresses bug ASTERISK-18103.
>     https://issues.asterisk.org/jira/browse/ASTERISK-18103
> 
> 
> Diffs
> -----
> 
>   /branches/1.6.2/main/features.c 330488 
> 
> Diff: https://reviewboard.asterisk.org/r/1337/diff
> 
> 
> Testing
> -------
> 
> tested reload roughly 80 times in quick succession.  No explosion.
> 
> Tested changing parkinglot extensions for the default parking lot as well as parking some calls.  Works as advertised.
> 
> Tested adding and removing parking lots.  Works as advertised.
> 
> Made sure reload didn't cause memory leaks by using memory show allocations features.c before and after consecutive reloads.  No unexpected allocations in any of the above cases.
> 
> 
> Thanks,
> 
> jrose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110801/43774eb6/attachment.htm>