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

[Mark Michelson]

> On July 6, 2012, 9:09 a.m., jrose wrote:
> > /branches/1.8/main/bridging.c, line 530
> > <https://reviewboard.asterisk.org/r/2012/diff/1/?file=29663#file29663line530>
> >
> >     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.

I did a grep through the source and none of the calls to pthread_join() in Asterisk check the return value. I looked at the man page, and all error conditions returned are such that there isn't really an alternate action we could take. At most, if an error occurred, we could print a message, but that's about it.


- Mark


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


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/0369ef4d/attachment-0001.htm>