----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3382/#review11356 -----------------------------------------------------------
branches/12/main/astobj2.c <https://reviewboard.asterisk.org/r/3382/#comment20997> Another thread can remove the last reference immediately after this thread finds that node->common.obj is not destroyed, allowing return of an object that is being (or has already been) destroyed. Any time another thread does ao2_ref(obj, -1); on the object we're trying to retreive we are subject to a race. A big problem is that ao2_ref requires that the caller owns a reference to the object, and no other thread will release that reference. I don't think it's possible to safely change this requirement. - Corey Farrell On March 24, 2014, 1:17 p.m., George Joseph wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3382/ > ----------------------------------------------------------- > > (Updated March 24, 2014, 1:17 p.m.) > > > Review request for Asterisk Developers, Joshua Colp and rmudgett. > > > Repository: Asterisk > > > Description > ------- > > This is a Request For Comments patch that adds weak reference capability to > astobj2 containers. > > Current (Strong-ref) behavior: A container (list, hash or rbtree) increments > an object's ref count when linked and decrements an object's ref count when > unlinked. Therefore the object can never be destroyed while it is an entry > in a container unless someone accidentally decrements the object's ref count > too many times. In this case, the object is invalid but the container > doesn't know it. > > Weak-ref behavior: A container (list, hash or rbtree) allocated with the > option AO2_CONTAINER_ALLOC_OPT_WEAK_REF neither increments an object's ref > count when linked nor decrements an object's ref count when unlinked. If the > object's ref count can therefore validly reach 0 in which case the object is > automatically and cleanly removed from any weak-ref container it may be in. > > Use case: The possible need for weak-ref containers came up during the > development of the Sorcery registry. The first call to sorcery_open from a > module should create a new sorcery instance and subsequent calls from that > same module should use that instance. The last call to close should free > that instance. With a strong-ref container however, the container itself > always has a a reference to the instance so it doesn't get destroyed without > special code to check the ref count on each call to close. > > Implementation: astobj2.priv_data now has a linked list that contains the > weak-ref container nodes that reference the object. When an object is added > to a weak-ref container, the container node is added to the list instead of > the node incrementing the object's ref count. Similarly, when an object is > removed from a weak-ref container the node is removed from the linked list > instead of the object's ref count being decremented. If an object's ref > count reaches 0 the object's linked list is traversed and the corresponding > nodes are cleared effectively removing the object from the container. NOTE: > An object's ref count is still incremented as the result of a retrieval > (find, iterate, etc) from a weak-ref container. > > Backwards compatibility: Unless the AO2_CONTAINER_ALLOC_OPT_WEAK_REF flag > was set on container allocation, all container operations remain exactly as > they are today and nothing prevents an object from being a member of both > strong and weak ref containers at the same time. > > Performance implications: Due to code reorganization, the performance of > strong-ref containers is actually microscopically BETTER than the unmodified > code and the performance of weak-ref containers is even better than that. In > other words, the performance of the default behavior was not penalized by the > addition of the new feature. > > Code reorganization. I moved all of the structure definitions in astobj2.c > to astobj2_private.h. This makes astobj2.c much easier to read and debug. I > was also able to push down some implementation specific code to the hash and > rbtree functions and pull up some duplicated code from the hash and rbtree > functions. > > Patch notes: The patch file itself might be a little hard to read because of > the reorganization so applying the patch is your best bet for detailed > review. The patch will apply cleanly to both branches/12 and trunk. Also, > the patch disables AO2_DEBUG and AST_DEVMODE in astobj2 so that performance > can be tested while still keeping the test framework. The final patch will > remove the disables. > > > Diffs > ----- > > branches/12/tests/test_astobj2_thrash.c 411013 > branches/12/tests/test_astobj2.c 411013 > branches/12/main/astobj2.c 411013 > branches/12/include/asterisk/astobj2.h 411013 > > Diff: https://reviewboard.asterisk.org/r/3382/diff/ > > > Testing > ------- > > All of the existing unit tests in test_astobj2 pass including the thrash test. > > I added 7 additional unit tests specifically for the weak-ref implementation > including a performance comparison test that compares both strong and weak > ref implementations. A thrash test was also added for weak-ref. > > > 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
