[asterisk-dev] [Code Review] Pickup segfault when multiple pickups of multiple localchan calls
rmudgett
reviewboard at asterisk.org
Fri Aug 26 11:54:36 CDT 2011
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1353/#review4143
-----------------------------------------------------------
The parent pointer approach bothers me because it can become stale easily.
After Party A's call is picked up by Party B, the call has a parent pointer that is no longer needed.
If Party B then attended-transfers the call to Party C and Party D does a call pickup, will the parent pointer be pointing to Party A or Party B of the transfer?
If Party A transfers Party B to Party C what does Party B's parent pointer point to? A nonexistent Party A?
Am I right in thinking that this is a race between the pickup masquerade and ast_hangup? Is the pickup masquerade being setup into the channel after ast_hangup is called by the dial app? The masquerade would then happen to a dead channel.
What happens if Party A hangs up while Party B is attempting to pickup the call? This would be a similar race situation that would be much harder to recognize in the field.
trunk/main/features.c
<https://reviewboard.asterisk.org/r/1353/#comment8128>
When getting the locks for two channels, you must do deadlock avoidance. chan and chan->parent in this case
You probably should not use the value put in parent until you have the necessary locks to avoid the possibility that parent has become inconsistent with chan->parent.
I also think that you must get the chan->parent lock first thing in this function because pickup conditions can change on you while doing deadlock avoidance.
trunk/main/features.c
<https://reviewboard.asterisk.org/r/1353/#comment8129>
You must mark the target before you can mark the parent because you must do deadlock avoidance between the target and target->parent. You cannot release the target lock before marking the target or you reintroduce the earlier pickup race.
trunk/main/features.c
<https://reviewboard.asterisk.org/r/1353/#comment8131>
Remove the parent datastore before re-acquiring the target lock to avoid needing deadlock avoidance.
- rmudgett
On Aug. 26, 2011, 7:02 a.m., Alec Davis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1353/
> -----------------------------------------------------------
>
> (Updated Aug. 26, 2011, 7:02 a.m.)
>
>
> Review request for Asterisk Developers and rmudgett.
>
>
> Summary
> -------
>
> If ast_can_pickup() can pickup the target channel, it also needs to check that the originating parent channel isn't being picked up from one of the other spawned the calls.
>
> Example dialplan below causes a segfault as ast-hangup removes the same channel as is being picked up.
>
> How to crash it!
> dial 801 from 1 phone.
> from 2 phones simultaneously dial 800.
> result = segfault!
>
> or segfault with 2 dahdi channels:
> dial 803 from 1 phone.
> from 2 phones simultaneously dial *8
> result = segfault!
>
> or NULL objects and orpaned channel
> dial 801 or 802 from 1 phone.
> from 2 phones simultaneously dial *8
> result = an orphaned channel!
>
> exten => 800,1,NoOp(Local pickup: Pickup through Localchan call)
> exten => 800,n,Dial(Local/824 at en-pickup&Local/823 at en-pickup)
>
> exten => 801,1,NoOp(Local pickup debug: Ring Phones)
> exten => 801,n,Dial(Local/823 at en-phone&Local/824 at en-phone)
>
> exten => 802,1,Dial(SIP/gxp-823&SIP/gxp-824)
>
> exten => 803,1,Dial(DAHDI/33&DAHDI/35)
>
> [en-pickup]
> exten => _[0-9*#]!, 1, PickupChan(Local/${EXTEN}@en-phone)
>
> [en-phone]
> exten => _[0-9*#]!, 1, Dial(SIP/gxp-${EXTEN},20,rwt)
>
>
> This addresses bugs ASTERISK-18222 and ASTERISK-18273.
> https://issues.asterisk.org/jira/browse/ASTERISK-18222
> https://issues.asterisk.org/jira/browse/ASTERISK-18273
>
>
> Diffs
> -----
>
> trunk/channels/chan_local.c 333199
> trunk/include/asterisk/channel.h 333199
> trunk/main/channel.c 333199
> trunk/main/features.c 333199
>
> Diff: https://reviewboard.asterisk.org/r/1353/diff
>
>
> Testing
> -------
>
> Confirmed no orphaned channels, and no NULL object messages with DAHDI, SIP and LOCALCHAN calls
>
> Example below where parent is updated sucessfully to head caller
>
> -- Executing [801 at phones:1] NoOp("SIP/snom8929-0000000b", "Debug Localchan Pickup, Ring Phones") in new stack
> -- Executing [801 at phones:2] Dial("SIP/snom8929-0000000b", "Local/823 at en_phone&Local/824 at en_phone") in new stack
> [2011-08-26 22:09:26.246139] NOTICE[21577]: channel.c:5712 ast_call: ALEC parent=SIP/snom8929-0000000b
> -- Called Local/823 at en_phone
> -- Executing [823 at en_phone:1] Dial("Local/823 at en_phone-9edc;2", "SIP/gxp-823,20,rwt") in new stack
> == Using UDPTL CoS mark 5
> == Using SIP RTP CoS mark 5
> [2011-08-26 22:09:26.252722] NOTICE[21577]: channel.c:5712 ast_call: ALEC parent=SIP/snom8929-0000000b
> -- Called Local/824 at en_phone
> -- Executing [824 at en_phone:1] Dial("Local/824 at en_phone-41b8;2", "SIP/gxp-824,20,rwt") in new stack
> == Using UDPTL CoS mark 5
> == Extension Changed 823[phones] new state Ringing for Notify User gxp-823
> == Extension Changed 823[phones] new state Ringing for Notify User gxp-824
> == Extension Changed 823[phones] new state Ringing for Notify User snom8929
> == Using SIP RTP CoS mark 5
> [2011-08-26 22:09:26.265837] NOTICE[21578]: channel.c:5712 ast_call: ALEC parent=SIP/snom8929-0000000b
> -- Called SIP/gxp-823
> -- Local/823 at en_phone-9edc;1 is ringing
> -- Local/823 at en_phone-9edc;1 connected line has changed. Saving it until answer for SIP/snom8929-0000000b
> -- Local/823 at en_phone-9edc;1 connected line has changed. Saving it until answer for SIP/snom8929-0000000b
> == Extension Changed 824[phones] new state Ringing for Notify User gxp-823
> == Extension Changed 824[phones] new state Ringing for Notify User gxp-824
> == Extension Changed 824[phones] new state Ringing for Notify User snom8929
> [2011-08-26 22:09:26.277966] NOTICE[21579]: channel.c:5712 ast_call: ALEC parent=SIP/snom8929-0000000b
> -- Called SIP/gxp-824
> -- Local/824 at en_phone-41b8;1 is ringing
> -- Local/824 at en_phone-41b8;1 connected line has changed. Saving it until answer for SIP/snom8929-0000000b
> -- Local/824 at en_phone-41b8;1 connected line has changed. Saving it until answer for SIP/snom8929-0000000b
> -- SIP/gxp-823-0000000c is ringing
>
>
> -- Executing [800 at phones:1] NoOp("SIP/gxp-824-00000013", "Debug Localchan pickup") in new stack
> -- Executing [800 at phones:2] Dial("SIP/gxp-824-00000013", "Local/824 at en_pickup&Local/823 at en_pickup") in new stack
> == Using SIP RTP CoS mark 5
> [2011-08-26 22:11:19.238111] NOTICE[21592]: channel.c:5712 ast_call: ALEC parent=SIP/gxp-824-00000013
> -- Called Local/824 at en_pickup
> == Extension Changed 823[phones] new state InUse&Ringing for Notify User gxp-823
> == Extension Changed 823[phones] new state InUse&Ringing for Notify User gxp-824
> == Extension Changed 823[phones] new state InUse&Ringing for Notify User snom8929
> -- Executing [824 at en_pickup:1] PickupChan("Local/824 at en_pickup-c63b;2", "Local/824 at en_phone") in new stack
> [2011-08-26 22:11:19.248294] NOTICE[21592]: channel.c:5712 ast_call: ALEC parent=SIP/gxp-824-00000013
> -- Called Local/823 at en_pickup
> -- Local/824 at en_pickup-c63b;1 connected line has changed. Saving it until answer for SIP/gxp-824-00000013
> -- Executing [823 at en_pickup:1] PickupChan("Local/823 at en_pickup-7a9f;2", "Local/823 at en_phone") in new stack
> -- Local/824 at en_pickup-c63b;1 answered SIP/gxp-824-00000013
> [2011-08-26 22:11:19.250604] NOTICE[21595]: app_directed_pickup.c:375 pickupchan_exec: No target channel found for Local/823 at en_phone.
> -- Executing [800 at phones:1] NoOp("SIP/gxp-823-00000014", "Debug Localchan pickup") in new stack
> -- Executing [800 at phones:2] Dial("SIP/gxp-823-00000014", "Local/824 at en_pickup&Local/823 at en_pickup") in new stack
> -- Auto fallthrough, channel 'Local/823 at en_pickup-7a9f;2' status is 'UNKNOWN'
> [2011-08-26 22:11:19.267860] NOTICE[21594]: channel.c:5712 ast_call: ALEC parent=SIP/gxp-823-00000014
> -- Called Local/824 at en_pickup
> -- Executing [824 at en_pickup:1] PickupChan("Local/824 at en_pickup-c8ca;2", "Local/824 at en_phone") in new stack
> [2011-08-26 22:11:19.272006] NOTICE[21594]: channel.c:5712 ast_call: ALEC parent=SIP/gxp-823-00000014
> -- Called Local/823 at en_pickup
> [2011-08-26 22:11:19.274014] NOTICE[21596]: app_directed_pickup.c:375 pickupchan_exec: No target channel found for Local/824 at en_phone.
> -- Auto fallthrough, channel 'Local/824 at en_pickup-c8ca;2' status is 'UNKNOWN'
> -- Executing [823 at en_pickup:1] PickupChan("Local/823 at en_pickup-dc84;2", "Local/823 at en_phone") in new stack
> [2011-08-26 22:11:19.276514] NOTICE[21597]: app_directed_pickup.c:375 pickupchan_exec: No target channel found for Local/823 at en_phone.
> -- Auto fallthrough, channel 'Local/823 at en_pickup-dc84;2' status is 'UNKNOWN'
> == Everyone is busy/congested at this time (2:0/0/2)
> -- Auto fallthrough, channel 'SIP/gxp-823-00000014' status is 'CHANUNAVAIL'
>
> asterix*CLI> core show channels
> Channel Location State Application(Data)
> SIP/gxp-824-00000013 800 at phones:2 Up Dial(Local/824 at en_pickup&Local
> SIP/snom8929-0000001 (None) Up AppDial((Outgoing Line))
> 2 active channels
> 1 active call
>
>
> Thanks,
>
> Alec
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110826/d4a4159f/attachment-0001.htm>
More information about the asterisk-dev
mailing list