[asterisk-dev] [Code Review] ast_pickup_call() refactor to create a common core function ast_do_pickup()

[irroot]

> On 2011-04-27 06:29:39, irroot wrote:
> > trunk/main/features.c, line 5778
> > <https://reviewboard.asterisk.org/r/1185/diff/4/?file=16229#file16229line5778>
> >
> >     There will be problems here if target is still locked when masquerade is called see r1118 i banged my head on this using ao2_callback.
> >     
> >     this function will return with target unlocked so you will need the other bits from 1118 to handle this.
> >     
> >     this patch is most needed thx i give it 2 opposing thumbs up.
> 
> Alec Davis wrote:
>     ast_do_pickup is a verbatim cut and paste of pickup_do() from app_directed_pickup().
>     In all previous cases the target is locked before the pickup_do().
>     Thus in all proposed cases the target is locked before the ast_do_pickup().
>     
>     What I think you're suggesting is that all of the other pickup methods from app_directed_pickup, 'which seem to work', have a pre existing problem?

Yip there is a problem calling ast_channel_masquerade should be done without the channels been locked so before its called unlock target.

there is a deadlock with target and channels container lock from ao2_callback if target is not locked then this deadlock does not occur.

this race condition occurs when the pickup is contended by more than one channel ive added deadlock avoidance not having the channel locked going in should prevent it.

hope you find this useful the problem is using the pickup_by_group method introduces this not a original problem as such mixing them together causes this.


- irroot


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


On 2011-04-21 06:09:52, Alec Davis wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1185/
> -----------------------------------------------------------
> 
> (Updated 2011-04-21 06:09:52)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> debugging mantis 18654, found common code, of which 1 set was wrong, the ast_pickup_call() code in features.c
> The actual fault was ast_pickup_call where the target chan was unlocked too early - see bug18654.diff2.txt
> 
> Moved app_directed:pickup_do() to features:ast_do_pickup().
> 
> Now functions below all now use the new ast_do_pickup()
> app_directed_pickup.c:
>    pickup_by_channel()
>    pickup_by_exten()
>    pickup_by_mark()
>    pickup_by_part()
> features.c:
>    ast_pickup_call() 
> 
> 
> This addresses bug 18654.
>     https://issues.asterisk.org/view.php?id=18654
> 
> 
> Diffs
> -----
> 
>   trunk/apps/app_directed_pickup.c 314594 
>   trunk/channels/chan_sip.c 314594 
>   trunk/include/asterisk/features.h 314594 
>   trunk/main/features.c 314594 
>   trunk/res/res_features.exports.in 314594 
> 
> Diff: https://reviewboard.asterisk.org/r/1185/diff
> 
> 
> Testing
> -------
> 
> pickup using *8 feature code, with pickup sounds enabled/disabled
> 
> exten => 71,1,Pickup()           ; any ringing extension in same pickupgroup 
> exten => 72,1,Pickup(85 at phones)  ; dahdi extension
> exten => 73,1,Pickup(823 at phones) ; sip extension
> 
> 
> Thanks,
> 
> Alec
> 
>

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