[asterisk-dev] [Code Review] Pickup segfault when multiple pickups of multiple localchan calls

[Alec Davis]

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

(Updated Aug. 25, 2011, 7:02 a.m.)


Review request for Asterisk Developers and rmudgett.


Changes
-------

Added a similar techique of adding a 'pickup datastore' to the originating dialling channel of the multiple spawned calls, as is done for the target pickup channel.

Then when the 2nd pickup (a split second later) is attempted on one of the spawned calls, ast_can_pickup checks the dialling channel of the target extension and if it finds the dialling channel has a 'pickup datastore' then it's already being picked up elsewhere, and fails the pickup gracefully.

The above approach prevents orphaned channels, and messages that "strange things may happen", I believe is better than fixing the dial application, as I understand it - queues ringing multiple phones have the similar issues when concurrent pickup attempts happen.

Also fixes the simpler example below (AST-18273), where NULL Ojects messages were thrown, and orpaned channels were left, after a simulanteous pickup.
exten => 801,1,NoOp(pickup debug: Ring Phones) 
exten => 801,n,Dial(SIP/phone1&SIP/phone2)


Summary
-------

The weird issue is why isn't the pickup datastore added to a local channel not working as it should.

This an attempt to fix Asterisk SVN-trunk-r330650 but perhaps the wrong approach.
 
 

If ast_can_pickup() can pickup the channel, it needs to change the chan->_state to AST_STATE_PICKUP.

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.
  segfault!

exten => 801,1,NoOp(Local pickup debug: Ring Phones) 
exten => 801,n,Dial(Local/823 at en-phone&Local/824 at en-phone)

exten => 800,1,NoOp(Local pickup: Pickup through Localchan call)
exten => 800,n,Dial(Local/824 at en-pickup&Local/823 at en-pickup)

[en-pickup]
exten => _[0-9*#]!, 1, PickupChan(Local/${EXTEN}@en-phone)

[en-phone]
exten => _[0-9*#]!, 1, Dial(SIP/gxp-${EXTEN},20,rwt)

will remove red blobs, and perhaps ast_pickup_active() test.


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 (updated)
-----

  trunk/channels/chan_local.c 333199 
  trunk/channels/chan_sip.c 333199 
  trunk/include/asterisk/channel.h 333199 
  trunk/main/features.c 333199 

Diff: https://reviewboard.asterisk.org/r/1353/diff


Testing
-------

Party A ring in which triggers off 2 internal localchan calls, as per 801 in the example dialplan.
Party B and Party C, attempt to pickup the same calls, using 800 in the example dialplan.

There is another senario thats not right, before even applying this patch.
Party A ring in, which again triggers 2 internal localchan calls as per 801 in dialplan.
Party B and Party C, use *8 to pickup. We then have a NULL OBJ message on screen, and an orpaned channel.


Thanks,

Alec

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