> On Sept. 3, 2014, 6:04 p.m., Mark Michelson wrote: > > /branches/13/res/res_pjsip/pjsip_options.c, lines 401-404 > > <https://reviewboard.asterisk.org/r/3954/diff/2/?file=67194#file67194line401> > > > > I don't understand the race condition you're referring to here since > > the scheduler has been altered to deal with an external deletion of the > > currently running task. > > > > Also, switching from a variable scheduler callback means that if the > > qualify frequency of a contact is changed, we will not switch to the new > > qualify frequency.
The frequency changes because the old scheduled event is always deleted before being restarted with the new time. As for the race condition, I was unable to see in the code self deletion and external deletion being safe. > On Sept. 3, 2014, 6:04 p.m., Mark Michelson wrote: > > /branches/13/res/res_pjsip.c, lines 2523-2539 > > <https://reviewboard.asterisk.org/r/3954/diff/2/?file=67193#file67193line2523> > > > > The approach taken here is incorrect. send_request_cb() has a > > pjsip_event passed to it that we currently are assuming is always > > PJSIP_EVENT_TSX_STATE, with the transaction state change being caused by > > event PJSIP_EVENT_RX_MSG. However, for the cases you mention where the > > callback is called during an error, the event is likely > > PJSIP_EVENT_TRANSPORT_ERROR, or it is PJSIP_EVENT_TSX_STATE with the > > transaction state change being caused by PJSIP_EVENT_TRANSPORT_ERROR. We > > should be handling this event in send_request_cb() by logging the error and > > immediately returning. That way, there is no risk that send_request_cb() > > has unreffed req_data, and since the user's callback has not been called, > > there is no risk that user data has been freed either. > > > > By doing this, an error return for pjsip_endpt_send_request() can > > safely unref req_data and return an error. Callers of > > send_out_of_dialog_request() can safely unref/free their data on an error > > return as well. I'll have to investigate some more. - rmudgett ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3954/#review13226 ----------------------------------------------------------- On Sept. 3, 2014, 9:40 a.m., rmudgett wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3954/ > ----------------------------------------------------------- > > (Updated Sept. 3, 2014, 9:40 a.m.) > > > Review request for Asterisk Developers. > > > Bugs: AFS-155 and ASTERISK-24295 > https://issues.asterisk.org/jira/browse/AFS-155 > https://issues.asterisk.org/jira/browse/ASTERISK-24295 > > > Repository: Asterisk > > > Description > ------- > > The crash on the issues is a result of an invalid transport configuration > change when asterisk is restarted. The attempt to send the qualify request > fails and we cleaned up. However, the callback is also called which results > in a double unref of the objects involved. > > * Fixed send_out_of_dialog_request() to not return error or cleanup resources > if pjsip_endpt_send_request() is not successful. > > * Fix periodic endpoint qualify OPTIONS sched deletion race by avoiding it. > The sched entry will no longer self stop and must be externally stopped. > > * Added REF_DEBUG description tags to struct sched_data in pjsip_options.c. > > * Fix some off-nominal ref leaks in schedule_qualify(), > qualify_and_schedule(). > > * Reordered pjsip_options.c module start/stop code to cleanup better on error. > > > Diffs > ----- > > /branches/13/res/res_pjsip/pjsip_options.c 422562 > /branches/13/res/res_pjsip.c 422562 > > Diff: https://reviewboard.asterisk.org/r/3954/diff/ > > > Testing > ------- > > * With the qualify_frequency option enabled, added and removed a "local_net=" > line in the transport section and restarted asterisk via "core restart now". > Before the latest patch version, asterisk would crash. With the new patch, > it keeps on going. > > * Set the qualify_frequency option to different values and reloaded res_pjsip > each time. The OPTIONS poll frequency changed, started, and stopped > according to the new qualify_frequency value. > > > Thanks, > > rmudgett > >
-- _____________________________________________________________________ -- 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
