[asterisk-dev] [Code Review] fixes deadlock in chan_sip caused by use of MASTER_CHANNEL() dialplan function

[Mark Michelson]

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


This is a case where the approach will work, but man does it feel wrong. I have a few problems with this:

1. It seems wasteful to have a thread dedicated only to setting a channel variable that is not widely used.
2. Under heavy enough load, it MAY be possible to not be able to read the correct value of the variable from the dialplan because the taskprocessor hasn't called its callback yet.

A different approach here would be to save the value of the Q.850 reason header in the sip_pvt (perhaps add a new string field). Then once the channel is unlocked in handle_request_do, you can set the channel variable at that time. What do you think?


/trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/606/#comment3907>

    Use AST_MODULE_LOAD_DECLINE here.


- Mark


On 2010-04-05 17:38:51, David Vossel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/606/
> -----------------------------------------------------------
> 
> (Updated 2010-04-05 17:38:51)
> 
> 
> Review request for Asterisk Developers and Tilghman Lesher.
> 
> 
> Summary
> -------
> 
> The use of the MASTER_CHANNEL() dialplan function in chan_sip currently causes a deadlock as it invalidates channel container locking order. This patch uses a taskprocessor in chan_sip to push the task of using pbx setvar on MASTER_CHANNEL() to a separate thread.
> 
> 
> This addresses bug 16767.
>     https://issues.asterisk.org/view.php?id=16767
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 256218 
> 
> Diff: https://reviewboard.asterisk.org/r/606/diff
> 
> 
> Testing
> -------
> 
> made sip calls, verified items were being pushed and processed by the taskprocessor.
> 
> 
> Thanks,
> 
> David
> 
>