[asterisk-dev] [Code Review] Fix descriptor (and structure reference) leak when call is hungup before answer

[Tilghman Lesher]

> On 2010-03-30 12:11:59, Mark Michelson wrote:
> > /trunk/channels/sip/dialplan_functions.c, line 472
> > <https://reviewboard.asterisk.org/r/591/diff/1/?file=8936#file8936line472>
> >
> >     I contend that this test should be re-tooled as an external test. I have two main arguments for this.
> >     
> >     1. There is a dependency on an external application (SIPp). Internal tests should live entirely within the bounds of Asterisk.
> >     
> >     2. The scope of this test goes well beyond what is typically defined as a "unit test." While it does exercise a specific issue that has been resolved with this patch, the coverage of this test is much too broad to be considered a test of a unit of code.
> >     
> >     The tricky part of this is that the condition on which this test will pass or fail is something that is not necessarily easily detected outside of Asterisk. Specifically, it is not easy to tell from outside of Asterisk which address was used to allocate a sip_pvt and then test whether the memory at that address has been freed. In this particular test, there is a way to test externally whether the proper resources have been freed because you can check if the RTP port is still being used after the call has finished.
> >     
> >     This test has opened the door to discussion of a good method of externally monitoring the memory usage of Asterisk. I, unfortunately, am not exactly a fount of knowledge in this particular area, but I would be willing to help with the necessary research.

Unfortunately, this cannot be retooled as an external test, because access is required to the internals of chan_sip, to areas which cannot be seen from any external interface at this point in time.  Internal access is needed to verify that the test is successful.


- Tilghman


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


On 2010-03-25 23:34:39, Tilghman Lesher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/591/
> -----------------------------------------------------------
> 
> (Updated 2010-03-25 23:34:39)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> In trunk and current 1.6.x releases, if a SIP channel is hungup prior to being answered, then a reference leak results, which causes RTP ports not to be freed.
> 
> The fix itself is a one-liner, but the additional changes were needed in order to build a regression test for this scenario.
> 
> 
> This addresses bug 16774.
>     https://issues.asterisk.org/view.php?id=16774
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 254405 
>   /trunk/channels/sip/dialplan_functions.c 254405 
>   /trunk/channels/sip/include/devices.h PRE-CREATION 
>   /trunk/channels/sip/include/globals.h 254405 
> 
> Diff: https://reviewboard.asterisk.org/r/591/diff
> 
> 
> Testing
> -------
> 
> Unit test written and tested.  There is one error that running the test still causes, but it's cosmetic and should be able to be tracked down fairly easily.
> 
> 
> Thanks,
> 
> Tilghman
> 
>