[asterisk-dev] [Code Review] Revert astobj2 change, add unit test for regression

[Russell Bryant]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1254/
-----------------------------------------------------------

(Updated 2011-06-03 13:04:38.354241)


Review request for Asterisk Developers.


Changes
-------

fix reference leaks pointed out by dvossel.


Summary
-------

The astobj2 unit tests have been failing in trunk a few times lately.  I still can't explain the failures, but while reviewing recent changes and trying to think of what could cause a problem, I thought of a case where a recent optimization will break things.  Specifically, this commit:


------------------------------------------------------------------------
r316962 | schmidts | 2011-05-05 02:09:20 -0500 (Thu, 05 May 2011) | 10 lines

Adding the Move to Front Hash functionality

Moving a found object to the front of its bucket to reduce the necessary traversal steps to find an object. This change improves the search time on large system with many data or in link lists.

(closes issue #19233)
Reported by: schmidts

Review: https://reviewboard.asterisk.org/r/1201/
------------------------------------------------------------------------

The complication with this has to do with how astobj2 iterators work.  The iterators are implemented in such a way that you do not have to hold the lock on the container for the entire duration of the container.  The lock is only held while getting the next element.  This works by using some internal version numbers to detect changes to the container while the lock was not held.  As a part of this code, it assumes that the order of elements in a given hash bucket are _always_ ordered by how long they have been in the container.


Diffs (updated)
-----

  /trunk/tests/test_astobj2.c 321746 
  /trunk/main/astobj2.c 321746 

Diff: https://reviewboard.asterisk.org/r/1254/diff


Testing
-------

The included new unit test demonstrates the regression.  This test passes in Asterisk 1.8 (where this commit doesn't exist).  The test fails in trunk with this message:


START  /main/astobj2/ - astobj2_test2 
[test_astobj2.c:astobj2_test_2:481]: iterate take 3, expected '5', only saw '4' objects
END    /main/astobj2/ - astobj2_test2 Time: <1ms Result: FAIL

After reverting the changes to main/astobj2.c, this test passes in trunk.


Thanks,

Russell

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110603/d04f062f/attachment.htm>