Fix two race conditions and ref counting issue when joining a bridge
authorMatthew Jordan <mjordan@digium.com>
Mon, 12 Aug 2013 15:59:19 +0000 (15:59 +0000)
committerMatthew Jordan <mjordan@digium.com>
Mon, 12 Aug 2013 15:59:19 +0000 (15:59 +0000)
commit63b5bf26ec31ca667ba2338aab649aefde21433c
tree4e29de502b11f738dc17ac2ffd8b1b18294f4a4e
parent5b013bc659a1cc92c5985e3e5b015feafa332d7f
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
main/bridge.c
main/bridge_channel.c
main/features.c