Review Board 1.7.16


Prevent 'Bad Magic Number' caused when a channel is optimized out by masquerade

Review Request #928 - Created Sept. 17, 2010 and submitted

Alec Davis
trunk
16057, 17037, 17363, 17801
Reviewers
asterisk-dev
Asterisk
there are 2 main fixes here.

1). Prevent a further masquerade being planned if either original/clonechan(masq/masqr) are set.
2). While the actual masquerade happens keep the channels container locked for the full duration. 

There is a comment 'that the channels container can be freed' after both channels are locked and unlink, but that not the case.
Using the following test plan, calling 10020 creates 20 looped Local channel calls, that then get optimized out.

[test]
exten => 10000,1,Answer()
exten => 10000,n,Playback(test-tones-follow)
exten => 10000,n,Milliwatt()

exten => _1XXXX,1,Set(i=${MATH(${EXTEN}-1,int)})
exten => _1XXXX,n,Dial(Local/${i}@test)

After patch: normal expected channels.
    asterix*CLI> core show channels
    Channel              Location             State   Application(Data)
    DAHDI/35-1           10010@phones:2       Up      Dial(Local/10009@phones)
    Local/10000@phones-2 s@echo-test:4        Up      Echo()
    Local/10000@phones-2 (None)               Up      AppDial((Outgoing Line))
    3 active channels
    2 active calls
    97 calls processed
    asterix*CLI>


prior to the patch:
   'Bad Magic Number' would reqularly been seen.

   dead channels left in container
   asterix*CLI> core show channels
   Channel              Location             State   Application(Data)
   Local/10000@phones-2 s@echo-test:3        Up      Playback(echo-test)
   Local/10000@phones-2 (None)               Up      AppDial((Outgoing Line))
   2 active channels
   1 active call
   21 calls processed
   asterix*CLI>
Review request changed
Updated (Sept. 18, 2010, 12:45 a.m.)
After comments from Stefan Schmidt on devlist.

In ast_do_masquerade:

  undid my initial defferal of unlocking the channels container, it's now in it's original place.

  Now, don't unlock the original and clonechan channel before they are linked back into the channels container, as they may disappear.
  There was a comment already there, suggesting that it could!

  When they do dissappear, we have a 'Bad Magic Number' senario setup. 
Ship it!
Posted (Sept. 20, 2010, 11:19 a.m.)
Note that this review is on the first version of the patch, not the second one.  The second one changes things such that it violates locking order.  If the code needs both the channel and container locked at the same time, it _must_ lock the container first.  That's why I like this version more than the first.
trunk/main/channel.c (Diff revision 1)
 
 
 
 
 
 
 
Remove this before commit
trunk/main/channel.c (Diff revision 1)
 
 
I think this comment can be removed

https://reviewboard.asterisk.org/ runs on a server provided by Digium, Inc. and uses bandwidth donated to the open source Asterisk community by API Digital Communications in Huntsville, AL USA.
Please report problems with this site to asteriskteam@digium.com.