----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3108/#review10525 -----------------------------------------------------------
Ship it! Ship It! - Mark Michelson On Jan. 7, 2014, 6:33 p.m., Matt Jordan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3108/ > ----------------------------------------------------------- > > (Updated Jan. 7, 2014, 6:33 p.m.) > > > Review request for Asterisk Developers. > > > Bugs: AST-1258 > https://issues.asterisk.org/jira/browse/AST-1258 > > > Repository: Asterisk > > > Description > ------- > > Say we have three users who are joining a ConfBridge: two wait marked (WM) > users and a marked (M) user. Say the WM users join the conference and wait > for awhile being entertained. The M user then joins, and the ConfBridge > starts. Internally, this is represented by state transitions that also keep > track of the number of active and waiting users. In this case, the ConfBridge > state goes through the following: > > 1. WM1 joins EMPTY -> INACTIVE (active: 0; waiting: 1) > 2. WM2 joins INACTIVE (active: 0; waiting: 2) > 3. M1 joins INACTIVE -> MULTI_MARKED (active: 3; waiting: 0) > > Now say someone - perhaps an overzealous sys admin - jumps onto Asterisk and > starts booting users left and right. They even have a script! They issue the > CLI kick user command so fast that we immediately kick M1 first, followed by > WM1 and WM2. Ideally, this would do the following: > > 4. M1 leaves MULTI_MARKED -> INACTIVE (active: 0; waiting: 2) > 5. WM2 leaves INACTIVE (waiting: 1) > 6. WM1 leaves INACTIVE (waiting: 0) > > When we transition states, the ConfBridge *must* be locked. This prevents the > user counts from getting jumbled and the state transitions from being > perturbed. However, MULTI_MARKED has a race condition: in order to play a > prompt to the bridge notifying the WM users that the leader has left the > conference, it unlocks the conference. This allows the WM user(s) to leave > prior to the M1 user from completely leaving/transitioning the conference. > More often than not, this causes a crash as ConfBridge enters into some very > invalid states. > > This patch fixes this by doing the following: > > (1) It prevents a user from getting kicked more than once. This isn't > strictly necessary, but is a good idea - we already keep track of whether or > not a user has been kicked, and kicking them multiple times is risky. > (2) It removes the race condition by deferring the "leader has left" playback > until after the conference state has transitioned. If someone sneaks in and > leaves at that time; no harm, no foul. > > > Diffs > ----- > > /branches/11/apps/confbridge/conf_state_multi_marked.c 404856 > /branches/11/apps/app_confbridge.c 404856 > > Diff: https://reviewboard.asterisk.org/r/3108/diff/ > > > Testing > ------- > > An automated test was written that reproduced the scenario. Without the > patch: Explosions. With the patch: Asterisk allowed the users to be kicked > without terminating itself. > > Note that the tests will be posted under a separate review. > > > 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
