[asterisk-dev] [Code Review]: fix using external MP3 player with res_timing_dahdi being used as timer causing MOH stream to stop

[mjordan]

> On Dec. 6, 2011, 8:11 p.m., elguero wrote:
> > /trunk/res/res_musiconhold.c, line 1639
> > <https://reviewboard.asterisk.org/r/1578/diff/1/?file=21688#file21688line1639>
> >
> >     I need to make sure I am understanding the suggestion.
> >     
> >     When we move the removal of the mohmembers to the beginning of the destructor, we get a lock first.  We then keep the lock; never call ao2_unlock?
> >     
> >     When I do this, I am getting the following error on the console for each moh class:
> >     
> >     ERROR[11769]: lock.c:136 __ast_pthread_mutex_destroy: astobj2.c line 270 (internal_ao2_ref): Error destroying mutex &obj->priv_data.lock: Device or resource busy
> >     
> >     Am I missing something?

No, you definitely need to unlock.  What I was saying was that getting rid of the ao2_lock / ao2_unlock pair doesn't appear to be safe, as the moh members can be accessed from another thread while the class is in its destructor.

The statement of moving the removal of the members from the moh_class to the beginning of the destructor is to prevent the member from accessing attributes of the moh_class while its being destructed.


- mjordan


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


On Nov. 14, 2011, 11:02 a.m., elguero wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1578/
> -----------------------------------------------------------
> 
> (Updated Nov. 14, 2011, 11:02 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> The attached patch does the following:
> 
> - Changes the load priority so that this module is loaded after the timing interfaces are.
> 
> At times, res_musichold.so would work with an external mp3 player.  Through debugging, I noticed that res_timing_pthread was being used at first.  If I only loaded res_timing_dahdi, then the external mp3 stream would start and then pause causing nothing to be heard on the channel. So, if res_timing_pthread was present at start, upon reload, since res_timing_dahdi takes priority as a timer, the timing changed to this timing interface and would just sit there, hence the need for the following change.
> 
> - Adds the POLLPRI event for ast_poll, otherwise ast_poll just sits there waiting when the timer being used is res_timing_dahdi.so
> 
> - Attempt to cleanup a few items
> 
> 
> This addresses bug ASTERISK-17474.
>     https://issues.asterisk.org/jira/browse/ASTERISK-17474
> 
> 
> Diffs
> -----
> 
>   /trunk/res/res_musiconhold.c 344329 
> 
> Diff: https://reviewboard.asterisk.org/r/1578/diff
> 
> 
> Testing
> -------
> 
> Local box running CentOS 5.7 and dahdi-trunk.
> 
> On JIRA, tested by:
> Thomas Arimont - 1.8.7
> Luke H - 1.8.8-rc3, CentOS 5.5 (32bit), DAHDI 2.5.0.2
> 
> 
> Thanks,
> 
> elguero
> 
>

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