----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3368/#review11302 -----------------------------------------------------------
Ship it! Thanks for untangling this one, I think this shows that changing the scheduler API in trunk is worth it. /tags/1.8.25.0/channels/chan_sip.c <https://reviewboard.asterisk.org/r/3368/#comment20942> Minor nit, no space immediately after ( or before ). /tags/1.8.25.0/channels/chan_sip.c <https://reviewboard.asterisk.org/r/3368/#comment20943> I would prefer we use a goto here instead of duplicating this code, but since this works around a crash I don't want to hold it up over a few duplicated lines. https://wiki.asterisk.org/wiki/display/AST/Coding+Guidelines#CodingGuidelines-Labels/gotoareacceptable - Corey Farrell On March 17, 2014, 10:21 a.m., one47 wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3368/ > ----------------------------------------------------------- > > (Updated March 17, 2014, 10:21 a.m.) > > > Review request for Asterisk Developers. > > > Bugs: ASTERISK-22079 > https://issues.asterisk.org/jira/browse/ASTERISK-22079 > > > Repository: Asterisk > > > Description > ------- > > As requested, here is a workaround (not a fix) for the SIP SEGV caused by a > scheduler race condition. > > If a provisional keepalive is simultaneously rescheduled/cancelled, and > executed by 2 parallel threads, then a scheduled event can be leaked. > > The correct fix will probably involve a re-factoring of the scheduler so that > a scheduled-job-reference is held by the owner, rather than just a > scheduled-job-id as at present. That is outside the scope of this fix, which > simply re-checks that the scheduler-id is unchanged after the lock has been > obtained when running the scheduled job. > > There is probably scope for doing this is several other scheduled function > calls. > > > Diffs > ----- > > /tags/1.8.25.0/channels/chan_sip.c 408955 > > Diff: https://reviewboard.asterisk.org/r/3368/diff/ > > > Testing > ------- > > Run on live server for several weeks. > > Tested on load-test environment with following dialplan, which previously > caused a crash in < 30 mins at 1 call per second. > > exten => 900,1,NoOp(Crash Generator) > same => n,Ringing > same => n,Wait(60) > same => n,Progress > same => n,Hangup > > > Thanks, > > one47 > >
-- _____________________________________________________________________ -- 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
