[asterisk-dev] [Code Review] 4048: res_fax: Fix fax handler module reference leak

[Corey Farrell]

> On Oct. 6, 2014, 12:06 p.m., Matt Jordan wrote:
> > This does look like the same issue as ASTERISK-18923.
> > 
> > Johann Steinwendtner provided a patch on that issue that does something similar, except that he doesn't bother bumping the reference count if the fax session isn't reserved. What do you think of that approach versus the one here?

I think at this time it is equivalent.  One thing this does make me question is if it is possible for the fax tech selected by fax_session_new to be different from the one selected by fax_session_reserve (like a better module being loaded between).  If so then my patch would deal with that edge case.  If not then maybe we should skip the faxmodules iteration all together?


- Corey


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


On Oct. 5, 2014, 4:34 p.m., Corey Farrell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4048/
> -----------------------------------------------------------
> 
> (Updated Oct. 5, 2014, 4:34 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24391
>     https://issues.asterisk.org/jira/browse/ASTERISK-24391
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> fax_session_reserve adds a reference to the fax driver's module, then fax_session_new adds another reference, but when the session is freed it only removes a single reference to the module.
> 
> This fixes it by unreferencing the module from fax_session_new when it is based on a reserved session.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/res/res_fax.c 424175 
> 
> Diff: https://reviewboard.asterisk.org/r/4048/diff/
> 
> 
> Testing
> -------
> 
> In testsuite against 11 verified that res_fax_spandsp now unloads in tests that previously leaked references to the module.
> 
> 
> Thanks,
> 
> Corey Farrell
> 
>

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