Skip to content
Snippets Groups Projects
  • Matthew Jordan's avatar
    63b5bf26
    Fix two race conditions and ref counting issue when joining a bridge · 63b5bf26
    Matthew Jordan authored
    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
    63b5bf26
    History
    Fix two race conditions and ref counting issue when joining a bridge
    Matthew Jordan authored
    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