Fixes to ConcurrentList

 - Fix using of freed memory in hazard pointers
 - Remove memory leak on freeing node in debug build
 - Set release policy to main loop by default
 - Re-enable the ConcurrentList tests

diff --git a/gee/concurrentlist.vala b/gee/concurrentlist.vala
index 6d8e468..03d14bb 100644 (file)
--- a/gee/concurrentlist.vala
+++ b/gee/concurrentlist.vala
@@ -401,8 +401,11 @@ public class Gee.ConcurrentList<G> : AbstractList<G> {
                        HazardPointer.set_pointer<Node<G>?> (&_succ, null, 3);
                        HazardPointer.set_pointer<Node<G>?> (&_backlink, null);
 #if DEBUG
-                       G? old_data = HazardPointer.exchange_pointer (&_data, null);
-                       stderr.printf ("  Freeing node %p (with data %p)\n", this, old_data);
+                       HazardPointer<G?>? old_data = HazardPointer.exchange_hazard_pointer (&_data, null);
+                       stderr.printf ("  Freeing node %p (with data %p)\n", this, old_data != null ? old_data.get() : null);
+                       if (old_data != null) {
+                               old_data.release (HazardPointer.get_destroy_notify<G?> ());
+                       }
 #else
                        HazardPointer.set_pointer<G> (&_data, null);
 #endif

diff --git a/gee/hazardpointer.vala b/gee/hazardpointer.vala
index da0d8a8..d6dd7d1 100644 (file)
--- a/gee/hazardpointer.vala
+++ b/gee/hazardpointer.vala
@@ -164,8 +164,10 @@ public class Gee.HazardPointer<G> { // FIXME: Make it a struct
         */
        public static void set_pointer<G> (G **aptr, owned G? new_ptr, size_t mask = 0, size_t new_mask = 0) {
                HazardPointer<G>? ptr = exchange_hazard_pointer<G> (aptr, new_ptr, mask, new_mask, null);
-               if (ptr != null)
-                       ptr.release (get_destroy_notify<G> ());
+               if (ptr != null) {
+                       DestroyNotify<G> notify = get_destroy_notify<G> ();
+                       ptr.release ((owned)notify);
+               }
        }
 
        /**
@@ -200,7 +202,8 @@ public class Gee.HazardPointer<G> { // FIXME: Make it a struct
                void *old_rptr = (void *)((size_t)(old_ptr) | (mask & old_mask));
                bool success = AtomicPointer.compare_and_exchange((void **)aptr, old_rptr, new_rptr);
                if (success) {
-                       Context.get_current_context ()->release_ptr (old_ptr, get_destroy_notify<G> ());
+                       DestroyNotify<G> notify = get_destroy_notify<G> ();
+                       Context.get_current_context ()->release_ptr (old_ptr, (owned)notify);
                } else if (new_ptr != null) {
                        delete new_ptr;
                }
@@ -226,10 +229,10 @@ public class Gee.HazardPointer<G> { // FIXME: Make it a struct
         *
         * @param notify method freeing object
         */
-       public void release (DestroyNotify notify) {
+       public void release (owned DestroyNotify notify) {
                unowned G item = _node[false];
                _node.set (null);
-               Context.get_current_context ()->release_ptr (item, notify);
+               Context.get_current_context ()->release_ptr (item, (owned)notify);
        }
 
        /**
@@ -600,10 +603,10 @@ public class Gee.HazardPointer<G> { // FIXME: Make it a struct
                /**
                 * Add pointer to freed array.
                 */
-               internal inline void release_ptr (void *ptr, DestroyNotify notify) {
+               internal inline void release_ptr (void *ptr, owned DestroyNotify notify) {
                        FreeNode *node = new FreeNode ();
                        node->pointer = ptr;
-                       node->destroy_notify = notify;
+                       node->destroy_notify = (owned)notify;
                        _to_free.add (node);
                        if (_to_free.size >= THRESHOLD)
                                HazardPointer.try_free (_to_free);

diff --git a/tests/testconcurrentlist.vala b/tests/testconcurrentlist.vala
index 3bc5039..38b550f 100644 (file)
--- a/tests/testconcurrentlist.vala
+++ b/tests/testconcurrentlist.vala
@@ -28,11 +28,17 @@ public class ConcurrentListTests : ListTests {
        }
 
        public override void set_up () {
+               if (!policy_set) {
+                       HazardPointer.set_release_policy(HazardPointer.ReleasePolicy.MAIN_LOOP);
+                       policy_set = true;
+               }
                test_collection = new Gee.ConcurrentList<string> ();
        }
 
        public override void tear_down () {
                test_collection = null;
        }
+
+       private static bool policy_set = false;
 }
 

diff --git a/tests/testmain.vala b/tests/testmain.vala
index f05ae4d..20068e4 100644 (file)
--- a/tests/testmain.vala
+++ b/tests/testmain.vala
@@ -26,7 +26,7 @@ void main (string[] args) {
 
        TestSuite.get_root ().add_suite (new ArrayListTests ().get_suite ());
        TestSuite.get_root ().add_suite (new ArrayQueueTests ().get_suite ());
-       //TestSuite.get_root ().add_suite (new ConcurrentListTests ().get_suite ());
+       TestSuite.get_root ().add_suite (new ConcurrentListTests ().get_suite ());
        TestSuite.get_root ().add_suite (new FunctionsTests ().get_suite ());
        TestSuite.get_root ().add_suite (new HashMapTests ().get_suite ());
        TestSuite.get_root ().add_suite (new HashMultiMapTests ().get_suite ());