[asterisk-dev] [Code Review]: A whole mess of ref counting cleanups

[Matt Jordan]

> On Oct. 2, 2012, 10:57 a.m., rmudgett wrote:
> > /trunk/main/message.c, line 1305
> > <https://reviewboard.asterisk.org/r/2137/diff/2/?file=31606#file31606line1305>
> >
> >     What happened to the void parameter list?
> >     
> >     void ast_msg_shutdown() is not the same as
> >     void ast_msg_shutdown(void).
> >     
> >     The prototype does not match.

Yikes.  Fixed.


> On Oct. 2, 2012, 10:57 a.m., rmudgett wrote:
> > /trunk/main/astobj2.c, lines 2020-2025
> > <https://reviewboard.asterisk.org/r/2137/diff/2/?file=31593#file31593line2020>
> >
> >     Why didn't you just change this to ao2_t_alloc() ?

It should be ao2_t_alloc_options.  That's much cleaner; changed.


> On Oct. 2, 2012, 10:57 a.m., rmudgett wrote:
> > /trunk/main/astobj2.c, lines 1502-1506
> > <https://reviewboard.asterisk.org/r/2137/diff/2/?file=31593#file31593line1502>
> >
> >     Why didn't you just change this to ao2_t_ref()?

That would be cleaner; changed.


- Matt


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


On Oct. 1, 2012, 3:31 p.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2137/
> -----------------------------------------------------------
> 
> (Updated Oct. 1, 2012, 3:31 p.m.)
> 
> 
> Review request for Asterisk Developers, Mark Michelson, rmudgett, and jcolp.
> 
> 
> Summary
> -------
> 
> While hunting for a channel ref leak, I found that after running the refcounter utility, there were a whole host of ao2 object ref leaks.  Wondering if that was just my test causing that, I started Asterisk and immediately stopped it.
> 
> Nope, lots of ref leaks still.
> 
> This patch cleans up a fair number of them - so that at least with my admittedly limited configuration (which had a number of modules excluded), there were no longer any ao2 ref leaks on Asterisk shutdown.  Most of these are simply Asterisk not properly cleaning up after itself on shutdown; however, a few could arguably result in slow growing memory leaks, particularly in systems that used frequent reloads.
> 
> Still haven't found the channel ref leak, but this does remove a lot of error messages.
> 
> Note that while this patch is for trunk, a version should be applied to 1.8, 10, and 11 as well.
> 
> For Richard:
> 
> I noticed that some of the internally allocated nodes in astobj2 would not be represented in a REF_DEBUG log, as they never used a 'debug' version of ao2_alloc or ao2_ref.  Unfortunately, a few code paths resulted in those same nodes showing up in the REF_DEBUG log, leading to warnings that nodes were appearing that were never allocated.  This patch manages to get all of the nodes represented in a REF_DEBUG log; however, I imagine that for the red/black tree work, this is going to be compounded.  If you have another approach you'd like to take other than what I have here, let me know.
> 
> 
> Diffs
> -----
> 
>   /trunk/include/asterisk/astobj2.h 374095 
>   /trunk/channels/chan_agent.c 374095 
>   /trunk/main/cel.c 374095 
>   /trunk/main/channel.c 374095 
>   /trunk/main/config_options.c 374095 
>   /trunk/main/data.c 374095 
>   /trunk/main/asterisk.c 374095 
>   /trunk/main/astobj2.c 374095 
>   /trunk/main/ccss.c 374095 
>   /trunk/main/db.c 374095 
>   /trunk/main/event.c 374095 
>   /trunk/main/features.c 374095 
>   /trunk/main/format.c 374095 
>   /trunk/main/format_pref.c 374095 
>   /trunk/main/indications.c 374095 
>   /trunk/main/manager.c 374095 
>   /trunk/main/message.c 374095 
>   /trunk/main/named_acl.c 374095 
>   /trunk/main/pbx.c 374095 
>   /trunk/main/taskprocessor.c 374095 
>   /trunk/main/udptl.c 374095 
>   /trunk/main/xmldoc.c 374095 
>   /trunk/res/res_musiconhold.c 374095 
>   /trunk/res/res_xmpp.c 374095 
> 
> Diff: https://reviewboard.asterisk.org/r/2137/diff
> 
> 
> Testing
> -------
> 
> A whole lot of loading/unloading, changing configuration, and generally peeking through REF_DEBUG logs.
> 
> 
> Thanks,
> 
> Matt
> 
>

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