[asterisk-dev] [Code Review] Pickup can leave orphan channels in race condition

[schmidts]

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

Ship it!


we have talked about this problem today on irc and i think this is a good and easy way to prevent the race condition we have found.

good work!

- schmidts


On July 29, 2011, 11:12 a.m., irroot wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1323/
> -----------------------------------------------------------
> 
> (Updated July 29, 2011, 11:12 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> seems like app_queue [and others likely] can leave a orphaned channel with no bridge and no thread running on it.
> 
> this is "ungood" here is a small patch that starts auto service waits for bridging and turns it off allowing the 
> hangup frame to get through and shut down the channel ...  
> 
> 
> Diffs
> -----
> 
>   /team/irroot/distrotech-customers-1.8/main/channel.c 330200 
> 
> Diff: https://reviewboard.asterisk.org/r/1323/diff
> 
> 
> Testing
> -------
> 
> [Jul 29 09:23:57] NOTICE[9698] app_directed_pickup.c: pickup SIP/0207-000002d3 attempt by SIP/0226-000002d5
> [Jul 29 09:23:58] WARNING[9698] chan_sip.c: No SIP tech_pvt! Fixup of SIP/0226-000002d5 failed.
> [Jul 29 09:23:58] WARNING[9698] channel.c: Fixup failed on channel SIP/0226-000002d5<MASQ>, strange things may happen.
> [Jul 29 09:25:10] NOTICE[10465] chan_sip.c: Disconnecting call 'SIP/0226-000002d5' for lack of RTP activity in 61 seconds
> 
> it seems possible this is a race condition between ast_hangup and ast_do_masquerade
> 
> do_masq holds the channels container [not allowing the channels to be unlinked] it also holds the original channel
> ast_hangup hold origchan but will unlock it to run do_masq 
> 
> do_masq will release the origchannel to avoid a deadlock with the clonechannel when this happens do_masq will run twice and orig chan will be removed by hangup
> 
> clonechan will be striped of all dignity 
> 
> 
> Thanks,
> 
> irroot
> 
>

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