[asterisk-dev] [Code Review] app_directed_pickup Implement the ability to remove a channel group on bridged channel

[irroot]

> On 2011-04-28 03:26:45, Alec Davis wrote:
> > I sort of see where your coming from releasing the target before the ast_channel_masquerade, but I don't think it's necessary, as I understand it's ast_do_masquerade that requires all locks to be released before its executed.

i want to release this lock as soon as possible as the masqurading will be done in another thread i dont need it here so ill drop it and let things happen.


> On 2011-04-28 03:26:45, Alec Davis wrote:
> > /trunk/apps/app_directed_pickup.c, line 145
> > <https://reviewboard.asterisk.org/r/1118/diff/6/?file=16265#file16265line145>
> >
> >     do we have a ref leak here, particulary when the code path could fail above is ast_answer fails?

Ah indeed ... 


> On 2011-04-28 03:26:45, Alec Davis wrote:
> > /trunk/main/channel.c, lines 6443-6447
> > <https://reviewboard.asterisk.org/r/1118/diff/6/?file=16266#file16266line6443>
> >
> >     The idea of locking the channels container was to stall all other masquerades.

using the chan_pickup_group ao2_callback method the channels container is locked when more than one channel is contesting the pickup ... but the kicker is the order will be wrong in that case channels first then channel we are locking channel then channels this avoidance will still work as follows ...

it will release the channels container until the channel is obtained at this point in the code there could be many contesting it at this point the first one to grab them in the right order will win and all others will be rejected ... 

unfortunately i cant find the debug trace that is related to this ....

using the chan_pickup_group callback introduced this deadlock path and would hate it to persist in the clean up you doing.


- irroot


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


On 2011-04-27 08:39:15, irroot wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1118/
> -----------------------------------------------------------
> 
> (Updated 2011-04-27 08:39:15)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> as a companion to 0018825 to aid with the move from call-limits to channel group variables. the problem with pickup and queue is that they manage channels outside the dialplan and need "helpers" to deal with channel groups.
> 
> in the case of pickup when a call is placed to a extension with say a "<EXTEN>@called" channel group when it is picked up this needs to be cleared to allow further calls when call waiting is disabled.
> 
> exten => s,n,GotoIf($[$[${ GROUP_COUNT(${ARG1}@called)} >= 1] | $[${GROUP_COUNT(${ARG1}@caller)} >= 1] | $[${GROUP_COUNT(${ARG1}@pickup)} >= 1] | $[${GROUP_COUNT(${ARG1}@qagent)} >= 1]]?s-BUSY,1)
> 
> where
> caller is set when a call is placed.
> called is set when a call is received
> pickup is set when before calling pickup in the dialplan
> qagent is set with patch to queue
> 
> Hope this is found to be useful and finds a home in trunk. 
> 
> not sure what time to put in the "wait for bridge" perhaps there is a better method ?? 
> 
> 
> This addresses bug 18830.
>     https://issues.asterisk.org/view.php?id=18830
> 
> 
> Diffs
> -----
> 
>   /trunk/apps/app_directed_pickup.c 315724 
>   /trunk/main/channel.c 315724 
> 
> Diff: https://reviewboard.asterisk.org/r/1118/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> irroot
> 
>

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