> On April 9, 2015, 1:49 p.m., Corey Farrell wrote: > > branches/13/res/res_pjsip_phoneprov_provider.c, lines 358-363 > > <https://reviewboard.asterisk.org/r/4608/diff/1/?file=73836#file73836line358> > > > > To record our brief discussion about this change: > > <coreyfarrell> gtjoseph: so one thing I don't see how using an > > ao2_iterator instead of ao2_callback in users_apply_handler makes it more > > straight forward.. > > <coreyfarrell> I don't want to hold up the fix, but that change doesn't > > make much sense to me > > <gtjoseph> there's no worry about the callback return value, > > OBJ_NODATA, etc. Get the container and iterate over it. It also > > eliminates a lot of overhead in the ao2_traversal code. > > <gtjoseph> unless the callback is controlling the traversal with it's > > return code, there's no point.
Actually, I think more people should use an ao2_iterator instead of ao2_callback when they can. That internal_ao2_traverse code is ugly. :) - George ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4608/#review15164 ----------------------------------------------------------- On April 9, 2015, 1:57 p.m., George Joseph wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/4608/ > ----------------------------------------------------------- > > (Updated April 9, 2015, 1:57 p.m.) > > > Review request for Asterisk Developers and Corey Farrell. > > > Bugs: ASTERISK-24935 > https://issues.asterisk.org/jira/browse/ASTERISK-24935 > > > Repository: Asterisk > > > Description > ------- > > res_pjsip_phoneprov_provider was leaking references to phoneprov objects due > to a missing OBJ_NODATA in an ao2_callback in load_users(). Rather than > adding the OBJ_NODATA, I changed load_users to use a more straightforward > ao2_iterator. This plugged the leak but exposed an unload order issue > between res_pjsip_phoneprov_provider, res_phoneprov and res_pjsip. > > res_pjsip_phoneprov_provider unloads first, then res_phoneprov, then > res_pjsip. Since res_pjsip_phoneprov_provider uses res_pjsip's sorcery > instance, when it unloads, it's objects are still in the sorcery instance. > When res_pjsip unloads, it destroys all its objects including > res_pjsip_phoneprov_provider's. The phoneprov destructor then attempts to > unregister the extension from res_phoneprov but because res_phoneprov is > already cleaned up, its users container is gone and we get a FRACK. > > Simple solution, check for the NULL users container before attempting to > remove the entry. Duh. > > > Diffs > ----- > > branches/13/res/res_pjsip_phoneprov_provider.c 434447 > branches/13/res/res_phoneprov.c 434447 > > Diff: https://reviewboard.asterisk.org/r/4608/diff/ > > > Testing > ------- > > Ran tests/res_phoneprov/res_phoneprov_provider. No leaks in > res_pjsip_phoneprov_provider and no FRACKs. > > > Thanks, > > George Joseph > >
-- _____________________________________________________________________ -- 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
