> On March 31, 2015, 5:52 p.m., rmudgett wrote: > > /trunk/main/astobj2.c, line 798 > > <https://reviewboard.asterisk.org/r/4108/diff/5/?file=71780#file71780line798> > > > > I think you need to have a weakproxy destructor callback that performs > > a sanity check on the proxy object to ensure that it is not still linked > > with a real object. > > > > Or even better, make the ao2_ref code do this check when it is about to > > destroy a weakproxy object. This check can also destroy anything in the > > notify list for the case if the real object was not set but subscribers > > were.
Once linked to a real object the weakproxy cannot be destroyed until after no references to the real object exist. Remember that until the real object is destroyed it holds a reference to the weakproxy. As for subscriptions I will have to look into this more, but my intention for subscriptions is a way to request notification as soon as the weakproxy points to NULL. So in the case of adding a subscription when no real object is linked, I think we should run the callback immediately and not add it to the subscription list. > On March 31, 2015, 5:52 p.m., rmudgett wrote: > > /trunk/main/astobj2.c, lines 450-451 > > <https://reviewboard.asterisk.org/r/4108/diff/5/?file=71780#file71780line450> > > > > Isn't the shell game you are playing with the real object's ref count > > going to mess up the DEBUG_REF output? > > > > How about this code: > > Code assumes that weakproxy ref wasn't bumped when set above. > > if weakproxy > > if current_value == 1 > > unlink real and weak > > unlock weakproxy > > if current_value == 1 > > run weakproxy callbacks (locking between each callback or locking > > to run all) > > unref weakproxy > > unref real Lets use realobj to represent the variable in the callers scope (non-ao2 code), user_data is the variable in internal_ao2_ref. This shell game results in REF_DEBUG logs showing the 2nd to last reference being released by ao2_ref(user_data, -1) a couple lines below your finding, just prior to the user releasing the last reference. REF_DEBUG is produced after internal_ao2_ref completes, so it's impossible for the unref's in this block to be logged after ao2_ref(realobj, -1). So without this REF_DEBUG would say that the object was destroyed due to ao2_ref(user_data, -1) before the 2nd to last reference was released. This REF_DEBUG output was verified using the provided test. - Corey ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4108/#review14995 ----------------------------------------------------------- On March 4, 2015, 4:43 p.m., Corey Farrell wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/4108/ > ----------------------------------------------------------- > > (Updated March 4, 2015, 4:43 p.m.) > > > Review request for Asterisk Developers, George Joseph and rmudgett. > > > Repository: Asterisk > > > Description > ------- > > This implements "weak" references. The weakproxy object is a real ao2 with > normal reference counting of its own. When a weakproxy is pointed to a > normal object they hold references to each other. The normal object is > automatically freed when a single reference remains (the weakproxy). The > weakproxy also supports subscriptions that will notify callbacks when the > normal object is about to be destroyed. > > > Diffs > ----- > > /trunk/tests/test_astobj2_weaken.c PRE-CREATION > /trunk/main/astobj2.c 432445 > /trunk/include/asterisk/astobj2.h 432445 > > Diff: https://reviewboard.asterisk.org/r/4108/diff/ > > > Testing > ------- > > Ran the included test with REF_DEBUG enabled under valgrind. No reference > leaks or improper memory access. Though this does not test for races, I > don't know of an automated way to do that. > > > Thanks, > > Corey Farrell > >
-- _____________________________________________________________________ -- 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
