[asterisk-dev] asterisk-dev Digest, Vol 109, Issue 78
Jeff Johnson
jjohnson at neturallyspeaking.com
Mon Aug 12 11:30:36 CDT 2013
sure wed is fine
Jeff Johnson
Director of Operations
410 S Ware Blvd | Suite 411 | Tampa FL 33619
Direct: (813) 774-3570 | Fax (813-569-2366
Email: jjohnson at neturallyspeaking.com | www.neturallyspeaking.com
Customer Service 866-448-0038
________________________________________
From: asterisk-dev-bounces at lists.digium.com [asterisk-dev-bounces at lists.digium.com] on behalf of asterisk-dev-request at lists.digium.com [asterisk-dev-request at lists.digium.com]
Sent: Monday, August 12, 2013 12:16 PM
To: asterisk-dev at lists.digium.com
Subject: asterisk-dev Digest, Vol 109, Issue 78
Send asterisk-dev mailing list submissions to
asterisk-dev at lists.digium.com
To subscribe or unsubscribe via the World Wide Web, visit
http://lists.digium.com/mailman/listinfo/asterisk-dev
or, via email, send a message with subject or body 'help' to
asterisk-dev-request at lists.digium.com
You can reach the person managing the list at
asterisk-dev-owner at lists.digium.com
When replying, please edit your Subject line so it is more specific
than "Re: Contents of asterisk-dev digest..."
Today's Topics:
1. [Code Review] 2756: Fix deadlocks in chan_sip in REFER and
BYE handling (opticron)
2. Re: [Code Review] 2741: Handle two race conditions while
joining a bridge and fix a ref counting error (svnbot)
3. Re: [Code Review] 2756: Fix deadlocks in chan_sip in REFER
and BYE handling (Mark Michelson)
----------------------------------------------------------------------
Message: 1
Date: Mon, 12 Aug 2013 15:51:38 -0000
From: "opticron" <reviewboard at asterisk.org>
Subject: [asterisk-dev] [Code Review] 2756: Fix deadlocks in chan_sip
in REFER and BYE handling
To: "Mark Michelson" <mmichelson at digium.com>
Cc: opticron <reviewboard at asterisk.org>, Asterisk Developers
<asterisk-dev at lists.digium.com>
Message-ID: <20130812155138.14672.3689 at hotblack.digium.com>
Content-Type: text/plain; charset="utf-8"
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2756/
-----------------------------------------------------------
Review request for Asterisk Developers and Mark Michelson.
Bugs: ASTERISK-22215
https://issues.asterisk.org/jira/browse/ASTERISK-22215
Repository: Asterisk
Description
-------
This resolves several deadlocks in chan_sip relating to usage of ast_channel_bridge_peer and improves accessibility of lock debugging function calls.
These issues were found while fixing the sip attended transfer integration tests for the referenced bug.
Diffs
-----
trunk/channels/chan_sip.c 396504
trunk/include/asterisk/lock.h 396504
trunk/main/utils.c 396504
Diff: https://reviewboard.asterisk.org/r/2756/diff/
Testing
-------
With these changes, chan_sip no longer stalls part of the way through the rewritten sip attended transfer tests on the REFER or at the end on the final BYE.
Thanks,
opticron
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130812/368b6a80/attachment-0001.htm>
------------------------------
Message: 2
Date: Mon, 12 Aug 2013 15:59:23 -0000
From: "svnbot" <reviewboard at asterisk.org>
Subject: Re: [asterisk-dev] [Code Review] 2741: Handle two race
conditions while joining a bridge and fix a ref counting error
To: rmudgett at digium.com
Cc: svnbot <reviewboard at asterisk.org>, Asterisk Developers
<asterisk-dev at lists.digium.com>
Message-ID: <20130812155923.14792.83753 at hotblack.digium.com>
Content-Type: text/plain; charset="utf-8"
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2741/
-----------------------------------------------------------
(Updated Aug. 12, 2013, 10:59 a.m.)
Status
------
This change has been marked as submitted.
Review request for Asterisk Developers and rmudgett.
Changes
-------
Committed in revision 396543
Repository: Asterisk
Description
-------
This patch fixes three problems found using the Bridge AMI action.
The scenario for reproducing the problem was always the same:
* Originate three Local channels into Echo()
* Execute Bridge on Local-0;2 and Local-1;2
* Immediately execute Bridge on Local-1;2 and Local-2;2
This reproduced the following problems:
(1) When originating a channel, the Newchannel event is emitted quickly; however, the channel will not have a pbx thread assigned to it until after the outbound 'dialing' is complete. Thus, there is a period of time where the outside world "knows" of the channel's existence and can influence it but Asterisk has not yet started the dialplan execution thread. If a Bridge AMI action is taken on the channel, the channel appears to be a Dialed channel with no PBX thread; hence, the channel will be imparted into the Bridge. At the same time, another thread that caused the origination will attempt to start a PBX on it. The end result currently is an assertion failure in the Bridging API.
There's no way to prevent AMI from attempting to Bridge a channel immediately after creation; likewise, holding the channel lock through the entire Dial operation is unwise. Instead of treating the presence of a PBX thread as an error, we simply bail out of the adding the channel to the bridge through ast_bridge_impart. The Bridge action will then fail - but we avoid a situation where the channel is both executing a PBX thread and simultaneously being given a separate thread in the bridging system (which would be a "bad thing"). Since imparting a channel with a PBX *can* occur and is not a programming error, the asserts have been removed.
(2) Adding a channel to a bridge consists of two checks: If the channel is in a bridge already, we move the channel using the Bridging API. If not, we steal the channel from its current location and impart it. To know the difference, we check the ast_bridge pointer on the channel. However, if the channel was previously imparted into a Bridge, the ast_bridge pointer is not set on the channel until the impart channel thread has spun up. During that period of time, the channel is unlocked and will appear for all intents and purposes as not being "in" the Bridging API - even though it is just a matter of time until the Bridge takes it on. This patch avoids this race condition by first checking if the channel being imparted is a Zombie - in which case, we bail - and by setting the ast_bridge pointer immediately upon entering the Bridging API. If later actions fail, the ast_bridge pointer is cleared.
(3) bridge_find_channel does not increase the reference count of the ast_bridge_channel object. The RAII_VAR usage in ast_bridge_add_channel thus created a ticking time bomb in whatever bridge the channel moved into, as the destructor for the ast_bridge_channel object would be called.
Diffs
-----
/trunk/main/bridge.c 396498
/trunk/main/bridge_channel.c 396498
/trunk/main/features.c 396498
Diff: https://reviewboard.asterisk.org/r/2741/diff/
Testing
-------
The nascent AMI Bridge action test stops crashing constantly.
Thanks,
Matt Jordan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130812/db3a2bd1/attachment-0001.htm>
------------------------------
Message: 3
Date: Mon, 12 Aug 2013 16:13:55 -0000
From: "Mark Michelson" <reviewboard at asterisk.org>
Subject: Re: [asterisk-dev] [Code Review] 2756: Fix deadlocks in
chan_sip in REFER and BYE handling
To: "Mark Michelson" <mmichelson at digium.com>
Cc: Mark Michelson <reviewboard at asterisk.org>, Asterisk Developers
<asterisk-dev at lists.digium.com>
Message-ID: <20130812161355.15210.47624 at hotblack.digium.com>
Content-Type: text/plain; charset="utf-8"
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2756/#review9392
-----------------------------------------------------------
trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/2756/#comment18453>
Grab a reference to transferer->owner before unlocking transferer. This way, you can guarantee not to pass an invalid pointer into ast_channel_bridge_peer().
trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/2756/#comment18455>
Like with your previous case, get a reference to p->owner before unlocking p.
Since this is the exact same sequence as the previous case where the bridged peer is acquired, I think it should exist as its own function.
Since this case also then uses the same deadlock avoidance technique in order to lock the bridged peer, I think the function you create should also try to return with all components (sip_pvt, sip_pvt's owner, bridged peer) locked.
- Mark Michelson
On Aug. 12, 2013, 3:51 p.m., opticron wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2756/
> -----------------------------------------------------------
>
> (Updated Aug. 12, 2013, 3:51 p.m.)
>
>
> Review request for Asterisk Developers and Mark Michelson.
>
>
> Bugs: ASTERISK-22215
> https://issues.asterisk.org/jira/browse/ASTERISK-22215
>
>
> Repository: Asterisk
>
>
> Description
> -------
>
> This resolves several deadlocks in chan_sip relating to usage of ast_channel_bridge_peer and improves accessibility of lock debugging function calls.
>
> These issues were found while fixing the sip attended transfer integration tests for the referenced bug.
>
>
> Diffs
> -----
>
> trunk/channels/chan_sip.c 396504
> trunk/include/asterisk/lock.h 396504
> trunk/main/utils.c 396504
>
> Diff: https://reviewboard.asterisk.org/r/2756/diff/
>
>
> Testing
> -------
>
> With these changes, chan_sip no longer stalls part of the way through the rewritten sip attended transfer tests on the REFER or at the end on the final BYE.
>
>
> Thanks,
>
> opticron
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130812/ad68eb02/attachment.htm>
------------------------------
_______________________________________________
--Bandwidth and Colocation Provided by http://www.api-digital.com--
AstriCon 2010 - October 26-28 Washington, DC
Put in your talk proposal: http://www.bit.ly/speak-astricon2010
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
End of asterisk-dev Digest, Vol 109, Issue 78
*********************************************
More information about the asterisk-dev
mailing list