[asterisk-dev] [Code Review] 4166: testsuite: tests/bridge/bridge_action leaves a channel open

[Corey Farrell]

> On Nov. 13, 2014, 11:30 a.m., Mark Michelson wrote:
> > With the fix being made to the leaked bridge in Asterisk, is this change still required? Does hanging up self.channels[1] not result in self.channels[3] and the bridge being destroyed as expected?
> 
> Corey Farrell wrote:
>     Still required, I'm guessing that when the first channel hangs up it leaves the second in a single user bridge.  I'm not sure if this is a bug in Asterisk or just the way it works.  The XMLDOC doesn't specify.
> 
> Corey Farrell wrote:
>     Actually on further inspection it appears the bridge is destroyed, and the caller is sent back to the previous Dialplan location.  core show channels without hangup of self.channels[3]:
>     Channel              Location             State   Application(Data)             
>     Local/waiting_area at d (None)               Up      Echo()                        
>     Local/waiting_area at d (None)               Up      Echo()                        
>     2 active channels
>     2 active calls
>     14 calls processed
>     
>     If I then run 'channel request hangup Local/waiting_area at default-00000003;1' (or hangup ;2), the test finishes and passes leak free.
>     
>     This is strange to me, why all the other channel pairs were hung up but not this one.
> 
> Mark Michelson wrote:
>     There are a few of unique properties to the final bridge that, imo, shouldn't affect operations but in practice might:
>     
>     1) The final bridge between 1 and 3 is the only one where the remaining channel in the bridge came from another bridge rather than directly from the dialplan. In fact, the remaining channel had been in two previous bridges, so its movement through the bridges may have something to do with it.
>     2) The final bridge between 1 and 3 is the only one where a channel is hung up. In the other cases, a channel is stolen from the bridge, resulting in bridge dissolution.
>     
>     After looking at the code again in action_bridge in features.c, the channels that are bridged together have ast_bridge_set_after_go_on() set on them. Looking at the docs for ast_bridge_set_after_go_on(), it says "If parseable_goto, then use the given context/exten/priority as the relative position for the parseable goto. Else goto the given context/exten/priorit+1". We don't provide a parseable goto, so after becoming unbridged, the channels should move to the next priority in the dialplan after the one they're currently in. Since the channels being bridged are currently at the Echo() priority, they should be returned to the dialplan at the next priority, which is Hangup(). This happens with all but channel 3. Somehow, I'm guessing that some code path is causing the priority number to goto after the bridge to get screwed up, so it ends up back in Echo() instead of moving on to Hangup().
>     
>     I think this is a problem in Asterisk, not the test.

Joshua just pointed out ASTERISK-24020, looks like this is already a reported issue against Asterisk.  I'm discarding this review since we've determined the testsuite it not at fault.


- Corey


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


On Nov. 11, 2014, 3:37 p.m., Corey Farrell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4166/
> -----------------------------------------------------------
> 
> (Updated Nov. 11, 2014, 3:37 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: testsuite
> 
> 
> Description
> -------
> 
> self.channels[3] is not hung up, causing the Asterisk graceful shutdown to timeout.  This causes the test to fail under REF_DEBUG mode and prevents coverage from seeing the code executed by this test.
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/tests/bridge/bridge_action/bridge_action.py 5920 
> 
> Diff: https://reviewboard.asterisk.org/r/4166/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Corey Farrell
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20141115/e04c5645/attachment.html>