[asterisk-dev] [Code Review] 3209: Crash in ast_format_cmp on shutdown

[Mark Michelson]

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


I'm not a fan of this patch for two reasons:
1) We're preventing a crash by introducing a memory leak. 
2) The condition of undestroyed channels doesn't seem like enough to guarantee that the interfaces container is not currently being referenced

A more graceful fix is to replace the interfaces ao2_container in format.c with an ao2 global object. This way, each time a piece of code attempts to get the container, they will need to get a reference using ao2_global_obj_ref()  (and check the function for a NULL return). This protects access to the container and guarantees that the container will not be freed as long as someone may be trying to use it.

- Mark Michelson


On Feb. 11, 2014, 7:37 p.m., Scott Griepentrog wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3209/
> -----------------------------------------------------------
> 
> (Updated Feb. 11, 2014, 7:37 p.m.)
> 
> 
> Review request for Asterisk Developers, Corey Farrell and n8ideas.
> 
> 
> Bugs: ASTERISK-23103
>     https://issues.asterisk.org/jira/browse/ASTERISK-23103
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This fixes a possible crash on shutdown by preventing ao2_unref of interfaces while channels are active.
> 
> Patch by cfarrell
> 
> 
> Diffs
> -----
> 
>   /branches/11/main/format.c 407957 
> 
> Diff: https://reviewboard.asterisk.org/r/3209/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Scott Griepentrog
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140212/f2b74392/attachment.html>