> On Feb. 6, 2014, 2:21 p.m., rmudgett wrote: > > branches/12/main/sorcery.c, lines 1630-1633 > > <https://reviewboard.asterisk.org/r/3184/diff/3/?file=53617#file53617line1630> > > > > There are two red flags here: > > 1) You are using sorcery after you have released your ref to it. > > > > 2) Any time you are looking at the number of references left, there is > > a design problem. In this case, only the owner of the sorcery object > > should close/destroy it which will remove it from the instances container. > > Only the owner of the sorcery object should open/create it which will add > > it to the instances container. Everyone else will search the instances > > container for the object to get a reference. > > George Joseph wrote: > There's no concept of weak references so when the sorcery instance is > linked to the registry container, the refcount gets bumped without the actual > consumer knowing. When the last consumer releases the instance, the > instance's refcount is still 1 so the destructor will never run and and > remove the entry. Instead of checking the refcount I could simply decrement > it automatically after inserting the instance to the container. This way the > last consumer to release it will trigger the cleanup. > > Now, if you're saying that only 1 consumer should ever call > ast_sorcery_open then I need to rethink. The original idea was that multiple > consumers could simply call ast_sorcery_open and let it figure out if it > needed to create a new one or return the existing one. > >
You cannot decrement the ref that is held by the container without causing problems for the container when it decrements the the ref on an unlink. You would get a negative ref count. I was thinking only one creator/owner should ever call the open/destroy. Bridges and channels have the same kind of problem since they are also put into a global container. They have trigger events to know when they need to be destroyed so they can unlink from the global container. - rmudgett ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3184/#review10799 ----------------------------------------------------------- On Feb. 6, 2014, 2:07 p.m., George Joseph wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3184/ > ----------------------------------------------------------- > > (Updated Feb. 6, 2014, 2:07 p.m.) > > > Review request for Asterisk Developers. > > > Bugs: ASTERISK-22537 > https://issues.asterisk.org/jira/browse/ASTERISK-22537 > > > Repository: Asterisk > > > Description > ------- > > Create sorcery instance registry as a precursor to creating a generic > dialplan function that can retrieve parameters from a sorcery-based config > file. > > ast_sorcery_init now creates a hashtab as a registry. > ast_sorcery_open now checks the hashtab for an existing sorcery instance > matching the caller's module name. If it finds one, it bumps the refcount > and returns it. If not, it creates a new sorcery instance, adds it to the > hashtab, then returns it. > ast_sorcery_retrieve_by_module_name is a new function that does a hashtab > lookup by module name. It can be called by the future dialplan function. > > A side effect of this patch is that a module can only have 1 sorcery instance > (because it's the key for the hashtab). res_pjsip/config_system needed a > small change to share the main res_pjsip sorcery instance. > > > Diffs > ----- > > branches/12/tests/test_sorcery.c 407566 > branches/12/res/res_pjsip/include/res_pjsip_private.h 407566 > branches/12/res/res_pjsip/config_system.c 407566 > branches/12/res/res_pjsip.c 407566 > branches/12/main/sorcery.c 407566 > branches/12/include/asterisk/sorcery.h 407566 > > Diff: https://reviewboard.asterisk.org/r/3184/diff/ > > > Testing > ------- > > Made sure that users of sorcery (mostly res_pjsip) continued to load their > configs correctly. > Made sure there were no ill effects on res_pjsip from config_system sharing > the same sorcery instance as the rest of the pjsip infrastructure. > Made sure that config_system was properly marked as 'not reloadable' and that > it was maintaining it's original values when res_pjsip was reloaded. > > > ernie*CLI> test execute category /main/sorcery/ > Running all available tests matching category /main/sorcery/ > > START /main/sorcery/ - configuration_file_wizard_retrieve_multiple_all > END /main/sorcery/ - configuration_file_wizard_retrieve_multiple_all Time: > <1ms Result: PASS > [snip] > START /main/sorcery/ - open > END /main/sorcery/ - open Time: <1ms Result: PASS > START /main/sorcery/ - wizard_registration > END /main/sorcery/ - wizard_registration Time: <1ms Result: PASS > > 43 Test(s) Executed 43 Passed 0 Failed > > > 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
