[asterisk-dev] [Code Review] 4034: chan_pjsip: Fix deadlock when masquerading PJSIP channels.

[Mark Michelson]

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






/branches/13/include/asterisk/taskprocessor.h
<https://reviewboard.asterisk.org/r/4034/#comment23906>

    I don't think this function is necessary. Instead of forcing taskprocessor listeners to set the thread ID, just make ast_taskprocessor_execute() set and unset the thread_id.
    
    I assume this was implemented this way as a means of saving on locking since you can set the thread ID, execute a bunch of tasks, and then unset the thread ID. However, since ast_taskprocessor_execute() already locks the taskprocessor both before and after executing the task, it makes just as much sense to set the thread ID there instead.
    
    The biggest benefit for doing this is that any taskprocessor implementation will automatically have the thread ID set correctly.



/branches/13/main/taskprocessor.c
<https://reviewboard.asterisk.org/r/4034/#comment23895>

    Use pthread_equal() instead of the == operator.


- Mark Michelson


On Sept. 29, 2014, 10:29 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4034/
> -----------------------------------------------------------
> 
> (Updated Sept. 29, 2014, 10:29 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24356
>     https://issues.asterisk.org/jira/browse/ASTERISK-24356
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Performing a directed call pickup resulted in a deadlock when PJSIP channels were involved.
> 
> A masquerade needs to hold onto the channel locks while it swaps channel information between
> the two channels involved in the masquerade.  With PJSIP channels, the fixup routine needed
> to push a fixup task onto the PJSIP channel's serializer.  Unfortunately, if the serializer
> was also processing a task that needed to lock the channel, you get deadlock.
> 
> * Added a new control frame that is used to notify the channels that a
> masquerade is about to start and when it has completed.
> 
> * Added the ability to query supported taskprocessors if the current thread is the
> taskprocessor thread.
> 
> * Added the ability to suspend/unsuspend the PJSIP serializer thread so a masquerade could
> fixup the PJSIP channel without using the serializer.
> 
> 
> Diffs
> -----
> 
>   /branches/13/res/res_pjsip_session.exports.in 424126 
>   /branches/13/res/res_pjsip_session.c 424126 
>   /branches/13/main/threadpool.c 424126 
>   /branches/13/main/taskprocessor.c 424126 
>   /branches/13/main/core_unreal.c 424126 
>   /branches/13/main/channel.c 424126 
>   /branches/13/main/bridge_channel.c 424126 
>   /branches/13/include/asterisk/taskprocessor.h 424126 
>   /branches/13/include/asterisk/res_pjsip_session.h 424126 
>   /branches/13/include/asterisk/frame.h 424126 
>   /branches/13/funcs/func_frame_trace.c 424126 
>   /branches/13/channels/chan_unistim.c 424126 
>   /branches/13/channels/chan_skinny.c 424126 
>   /branches/13/channels/chan_sip.c 424126 
>   /branches/13/channels/chan_pjsip.c 424126 
>   /branches/13/channels/chan_motif.c 424126 
>   /branches/13/channels/chan_misdn.c 424126 
>   /branches/13/channels/chan_iax2.c 424126 
>   /branches/13/addons/chan_ooh323.c 424126 
> 
> Diff: https://reviewboard.asterisk.org/r/4034/diff/
> 
> 
> Testing
> -------
> 
> * Performed several directed call pickups without deadlocking.  Before the patch, the system would usually deadlock.
> 
> * Performed a SIP attended transfer to an appliction.  The transfer still works.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

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