----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3964/#review13217 -----------------------------------------------------------
/branches/12/main/cdr.c <https://reviewboard.asterisk.org/r/3964/#comment23706> * Nothing cares about this return value. * The RAII_VAR and SCOPED_AO2LOCK can easily be eliminated by changing the "return 0" statements in the for loop to breaks. /branches/12/main/cdr.c <https://reviewboard.asterisk.org/r/3964/#comment23705> This change has no effect. The break only exited the switch statement and then control goes to the next iteration in the loop. Changing it to a continue just does the same thing. /branches/12/main/cdr.c <https://reviewboard.asterisk.org/r/3964/#comment23707> This is a tail recursion that could be eliminated by pulling the above for loop into its own routine. Then the cdr lock also won't be recursively obtained. - rmudgett On Sept. 1, 2014, 11:51 a.m., Matt Jordan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3964/ > ----------------------------------------------------------- > > (Updated Sept. 1, 2014, 11:51 a.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 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 > >
-- _____________________________________________________________________ -- 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
