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

[rmudgett]

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

Ship it!


Other than the reference leak it looks ok.


/branches/1.6.2/main/features.c
<https://reviewboard.asterisk.org/r/1337/#comment7796>

    p reference leak if p == default_parkinglot
    
    Move unref to after if test.


- rmudgett


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/aae32f1b/attachment-0001.htm>