summaryrefslogtreecommitdiff
path: root/main/features.c
diff options
context:
space:
mode:
authorMatthew Jordan <mjordan@digium.com>2013-08-12 15:59:19 +0000
committerMatthew Jordan <mjordan@digium.com>2013-08-12 15:59:19 +0000
commit63b5bf26ec31ca667ba2338aab649aefde21433c (patch)
tree4e29de502b11f738dc17ac2ffd8b1b18294f4a4e /main/features.c
parent5b013bc659a1cc92c5985e3e5b015feafa332d7f (diff)
Fix two race conditions and ref counting issue when joining a bridge
These problems were all caught by a test in the Asterisk Test Suite that originated some Local channels and attempted to move the ;2 half of the Local channel into a bridge using the Bridge AMI action. (1) When originating a channel, the Newchannel event is emitted quickly; however, the ;2 channel will not have a pbx thread assigned to it until after the outbound 'dialing' for the ;1 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 by first 'yanking' the channel. At the same time, a race condition can occur after the yank (but before entering the bridge) when ;1 answers and starts a PBX on the ;2. The end result currently is an assertion failure in the Bridging API, as a channel with a PBX is imparted into the Bridge. 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 (and impossible). 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) When the first condition occurs, we have to take one of two actions: either hangup the yanked channel as it did not enter the bridge, or deref it because we don't own it. We can determine if we own it or not by testing for the presence of the PBX thread. If we hung it up directly, we'd crash. (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. Review: https://reviewboard.asterisk.org/r/2741/ git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@396543 65c4cc65-6c06-0410-ace0-fbb531ad65f3
Diffstat (limited to 'main/features.c')
-rw-r--r--main/features.c2
1 files changed, 2 insertions, 0 deletions
diff --git a/main/features.c b/main/features.c
index 9e5dfddb3..6585aeccd 100644
--- a/main/features.c
+++ b/main/features.c
@@ -1142,6 +1142,8 @@ static int action_bridge(struct mansession *s, const struct message *m)
return 0;
}
+ ast_debug(1, "Performing Bridge action on %s and %s\n", channela, channelb);
+
/* Start with chana */
chana = ast_channel_get_by_name_prefix(channela, strlen(channela));
if (!chana) {