----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3964/#review13222 -----------------------------------------------------------
Ship it! Ship It! - rmudgett On Sept. 3, 2014, 3:22 p.m., Matt Jordan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3964/ > ----------------------------------------------------------- > > (Updated Sept. 3, 2014, 3:22 p.m.) > > > 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 422556 > > 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 > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
