[asterisk-dev] [Code Review] Data Store Destruction moved to ast_hangup

[jrose]

> On 2011-03-14 15:15:53, Russell Bryant wrote:
> > /trunk/main/channel.c, lines 2837-2842
> > <https://reviewboard.asterisk.org/r/1136/diff/1/?file=15846#file15846line2837>
> >
> >     Add braces {} to this while loop since you're touching this code anyway.
> 
> jrose wrote:
>     Alrighty, now it looks like:
>     
>     while ((datastore = AST_LIST_REMOVE_HEAD(&chan->datastores, entry))) {
>     	/* Free the data store */
>     	ast_datastore_free(datastore);
>     }
>     
>     I'll wait for more feedback before I post a new diff.
> 
> jrose wrote:
>     Oh wait, you said ship it.  Shipping it.

Ok, David says this sounds like a memory leak above... so perhaps this fix is a little hasty.  I'm going to go ahead and commit it, but I'd like to discuss this with the two of you tomorrow.


- jrose


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


On 2011-03-14 12:44:27, jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1136/
> -----------------------------------------------------------
> 
> (Updated 2011-03-14 12:44:27)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Changes channel.c so that the call hangup function (instead of channel being closed) is responsible for destroying application data stores and signalling to the application thread that it can continue it's destruction event.
> 
> This was necessary because applications like chanspy could prevent channels from closing under certain conditions, causing the threads to become stuck and never complete.
> 
> 
> This addresses bug https://issues.asterisk.org/view.php?id=18742.
>     https://issues.asterisk.org/view.php?id=https://issues.asterisk.org/view.php?id=18742
> 
> 
> Diffs
> -----
> 
>   /trunk/main/channel.c 310588 
> 
> Diff: https://reviewboard.asterisk.org/r/1136/diff
> 
> 
> Testing
> -------
> 
> Rudimentary testing was done using dialplans similar to what was suggested in the noted bug report.  In addition, initial response to the patch suggests it works.
> 
> 
> Thanks,
> 
> jrose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110314/31ae5c73/attachment-0001.htm>