[asterisk-dev] [Code Review] Fix Bridging thread leak

[jrose]

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



/branches/1.8/main/bridging.c
<https://reviewboard.asterisk.org/r/2012/#comment12574>

    pthread_join has unchecked return. Maybe you should display a warning or an error when it fails. I'm not sure how useful that would be. Inconsistencies from unchecked returns were something I had to work on for an issue a while back though.


- jrose


On July 5, 2012, 5:11 p.m., Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2012/
> -----------------------------------------------------------
> 
> (Updated July 5, 2012, 5:11 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> It appears that under normal circumstances, bridging threads are never joined using pthread_join(). The reporter supplied a patch that makes the bridge thread detached rather than joinable. I don't feel this is sufficient because the smart_bridge_operation() joins the thread partway through its method. This implies that it intends for the bridge thread to be completely finished and cleaned up prior to continuing.
> 
> In this patch, I have fixed the leak by adding a pthread_join() call to the code in ast_bridge_destroy(). I also have modified bridge_check_dissolve() not to set the stop flag. If bridge_check_dissolve() sets the stop flag for the bridge, then the bridge_thread will set bridge->thread to AST_PTHREADT_NULL. This will make it impossible for ast_bridge_destroy() to be able to tell if there is a thread that needs to be joined.
> 
> 
> This addresses bug ASTERISK-19834.
>     https://issues.asterisk.org/jira/browse/ASTERISK-19834
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/main/bridging.c 369625 
> 
> Diff: https://reviewboard.asterisk.org/r/2012/diff
> 
> 
> Testing
> -------
> 
> Ran a SIPp scenario that had calls constantly joining and leaving a conference bridge until a total of 2000 participants had joined and left.
> 
> Unfortunately, it's difficult to see if the added pthread_join() calls are doing anything because on my system, neither gdb nor /proc will display zombie threads. So for me anyway, gdb and /proc appear the same before and after applying the patch. More importantly, this patch does not appear to cause any crashes or other problems.
> 
> 
> Thanks,
> 
> Mark
> 
>

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