[asterisk-dev] [Code Review] 3964: CDRs: Fix crash caused by infinite generation of new CDR records when a channel enters, leaves, and enters a multi-party bridge

[Matt Jordan]

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

Review request for Asterisk Developers.


Bugs: ASTERISK-24241
    https://issues.asterisk.org/jira/browse/ASTERISK-24241


Repository: Asterisk


Description
-------

This patch fixes an issue where CDRs would get stuck generating an infinite number of CDRs, eventually crashing Asterisk.

When a channel enters into a multi-party bridge, the CDR engine creates mappings of each participant to each other participant, picking the 'A' party as it goes. So, if we have four channels in a multi-party bridge (Alice, Bob, Charlie, Denise), we would have something like:

Alice => Bob
Alice => Charlie
Alice => Denise
Bob => Charlie
Bob => Denise
Charlie => Denise

This works fine when participants enter the bridge a single time.

When a participant leaves a bridge, the CDRs for that channel are transitioned to a finalized state. In the previous example, if Bob leaves the bridge, then the following CDRs are all marked as finalized:

Alice => Bob (F)
Bob => Charlie (F)
Bob => Denise (F)

The bug occurs if Bob rejoins. When the CDR engine creates mappings between the channels, a CDR in the finalized state reports back to the engine "nope, I can't handle this, you need a new CDR". That is correct, but the code that generates the mappings makes two mistakes at that point:
(1) It bails instead of looking for more CDRs, immediately making a new CDR and appending it to the chain. When you make a new CDR for a channel, it does not originally have a Party B - so the engine calls the bridge mapping code "knowing" that Bob will be a perfect Party B.
(2) When a new CDR is appended to a chain, one code path allowed us to continue on down the chain inspecting more CDRs. We do update the 'new' CDR, but we end up transitioning it to the Bridge state and calling the original code on it again, hitting point #1 again (as there is still an original CDR in the finalized state).

Let's say Bob enters the bridge again. The CDR engine starts making pairs of CDRs, starting with Alice. Since Alice and Bob's original CDR is finalized (he left the bridge), the record returns "you need a new one". The CDR engine makes a new CDR for Alice, appends it to her CDRs, and then tells Bob to go see if he can find a CDR for Alice that is suitable. This hits bug #2, which maps Bob to Alice but then keeps looping instead of returning automatically. When we're done updating the new CDR, the original code takes over, hits point #1 again, and generates a new CDR. Hilarity!

We end up with:

Alice => Bob (F)   <--- this keeps getting inspected and telling us to generate new CDRs :-(
Alice => Bob (new)
Alice => Bob (new)
Alice => Bob (new)
 ...

This patch fixes the two glitches mentioned above.
(1) If, when making matches, a CDR returns 'you need a new one', we keep inspecting the chain to see if anyone else can take a match for us
(2) When a match is made, we return if we successfully placed our channel as the Party B on an existing CDR, instead of continuing to match up

And, even after this bug, I still maintain this is easier to diagnose and fix then the original CDR code in features.c :-)  If nothing else, all of this code was in one place, and I didn't have to think "what happens if something masquerades right now".


Diffs
-----

  /branches/12/main/cdr.c 422502 

Diff: https://reviewboard.asterisk.org/r/3964/diff/


Testing
-------

All existing unit tests/testsuite tests pass.

Issue was reproduced using the test on https://reviewboard.asterisk.org/r/3965. The test creates 5 Local channels, puts them in a multi-party bridges, then removes channels 0 and 3 and re-adds them.

Without this patch - explosions. With this patch - we generate the correct number of CDRs even when channels bounce themselves in and out of multi-party bridges.


Thanks,

Matt Jordan

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