[asterisk-dev] [Code Review] Prevent 'Bad Magic Number' caused when a channel is optimized out by masquerade

[Russell Bryant]

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

Ship it!


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
<https://reviewboard.asterisk.org/r/928/#comment5940>

    Remove this before commit



trunk/main/channel.c
<https://reviewboard.asterisk.org/r/928/#comment5941>

    I think this comment can be removed


- Russell


On 2010-09-18 05:45:40, Alec Davis wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/928/
> -----------------------------------------------------------
> 
> (Updated 2010-09-18 05:45:40)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> 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.
> 
> 
> This addresses bugs 16057, 17037, 17363, and 17801.
>     https://issues.asterisk.org/view.php?id=16057
>     https://issues.asterisk.org/view.php?id=17037
>     https://issues.asterisk.org/view.php?id=17363
>     https://issues.asterisk.org/view.php?id=17801
> 
> 
> Diffs
> -----
> 
>   trunk/main/channel.c 287464 
> 
> Diff: https://reviewboard.asterisk.org/r/928/diff
> 
> 
> Testing
> -------
> 
> 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 at phones:2       Up      Dial(Local/10009 at phones)
>     Local/10000 at phones-2 s at echo-test:4        Up      Echo()
>     Local/10000 at 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 at phones-2 s at echo-test:3        Up      Playback(echo-test)
>    Local/10000 at phones-2 (None)               Up      AppDial((Outgoing Line))
>    2 active channels
>    1 active call
>    21 calls processed
>    asterix*CLI>
> 
> 
> Thanks,
> 
> Alec
> 
>