> On July 15, 2014, 2 p.m., Corey Farrell wrote: > > The ability to add an object to multiple weak reference containers is > > concerning to me. I think this adds unnecessary complexity/risk to > > astobj2. If you could tell me a use case where this amount of freedom is > > required I might be convinced otherwise. Otherwise I'd feel much better > > with a more limited ability to use weak reference containers: > > > > 1. Only allow the caller of ao2_alloc to add the object to a weak > > container, and only one container. > > 2. Insertion to the weak container should only happen before the object is > > reachable outside the local scope. > > 3. The weak container would be a private detail to the unit that owns it. > > All other code would use a method to retrieve references from that > > container. > > > > These two rules would greatly reduce the possibility of races, which > > reduces the amount of work needed to avoid races. In particular rule #2 > > would allow ao2_ref to only perform locking during ao2_ref(obj, -1). I had > > started work on this in r3463 but discarded it when you started splitting > > astobj2.c.
We're going to disgree. :) I think this is a pretty severe restriction and it would be more complex to enforce the restriction than to safely allow it. How about this... If you can break it and I can't reasonably fix it, I'll back off. I know you're busy so just write a pseudo code test case, or just give me an idea for one and I'll create a unit test for it. - George ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3716/#review12583 ----------------------------------------------------------- On July 7, 2014, 10:43 p.m., George Joseph wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3716/ > ----------------------------------------------------------- > > (Updated July 7, 2014, 10:43 p.m.) > > > Review request for Asterisk Developers, Corey Farrell and rmudgett. > > > Repository: Asterisk > > > Description > ------- > > Weak Reference Containers are hash containers that don't maintain references > to the objects they contain. > Basically, ao2_link() doesn't increase the ref count and ao2_unlink() doesn't > decrease it. > > Sounds simple but because the container doesn't have a reference to the > object, it's entirely possible that the objects could be destroyed while > still in the container. Therefore much of the work in this patch is making > sure that if the object is destroyed, it's properly cleaned out of any weak > reference containers that it may have been in. > > This patch is almost 6000 lines but half of it is units tests...functional, > performance, and stress. > > Richard: I apologize in advance...I undid some of the refactor undos you had > me do earlier. :) > > > Diffs > ----- > > branches/12/utils/Makefile 418136 > branches/12/tests/test_astobj2_weak.c PRE-CREATION > branches/12/tests/test_astobj2_torture.c PRE-CREATION > branches/12/tests/test_astobj2_thrash.c 418136 > branches/12/tests/test_astobj2.c 418136 > branches/12/main/astobj2_weak.c PRE-CREATION > branches/12/main/astobj2_rbtree.c 418136 > branches/12/main/astobj2_private.h 418136 > branches/12/main/astobj2_hash_private.h PRE-CREATION > branches/12/main/astobj2_hash.c 418136 > branches/12/main/astobj2_container_private.h 418136 > branches/12/main/astobj2_container.c 418136 > branches/12/main/astobj2.c 418136 > branches/12/include/asterisk/astobj2.h 418136 > > Diff: https://reviewboard.asterisk.org/r/3716/diff/ > > > Testing > ------- > > A whole slew of unit tests were added and the existing astobj2 tests were > modified to also test weak containers. All passed. > A torture test was added that tests multiple threads accessing multiple > containers for weak, hash and rbtree containers. All passed. > The full Testsuite was run with the same number of tests passing that passed > before the patch. > > > 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
