Implemented lockfree list and added unit test for it.

src/data_structures/list/lockfree_list_new.hpp

@@ -1,39 +1,41 @@
 #pragma once
 
+#include "utils/crtp.hpp"
 #include <atomic>
 
 template <class T>
-static T *load(std::atomic<T *> &atomic)
+static T load(std::atomic<T> &atomic)
 {
-    return atomic.load(std::memory_order_acquire())
+    return atomic.load(std::memory_order_acquire);
 }
 
 template <class T>
-static void store(std::atomic<T *> &atomic, T *desired)
+static void store(std::atomic<T> &atomic, T desired)
 { // Maybe could be relaxed
-    atomic.store(desired, std::memory_order_release());
+    atomic.store(desired, std::memory_order_release);
 }
 
 template <class T>
-static bool cas(std::atomic<T *> &atomic, T *expected, T *desired)
+static bool cas(std::atomic<T> &atomic, T expected, T desired)
 { // Could be relaxed must be atleast Release.
-    return atomic.compare_exchange_strong(expected,desired,std::memory_order_seq_cst()));
+    return atomic.compare_exchange_strong(expected, desired,
+                                          std::memory_order_seq_cst);
 }
 
 template <class T>
 static T *swap(std::atomic<T *> &atomic, T *desired)
 { // Could be relaxed
-    return atomic.exchange(desired,std::memory_order_seq_cst()));
+    return atomic.exchange(desired, std::memory_order_seq_cst);
 }
 
 template <class T>
 class List
 {
-    class Node
-    {
-        friend class Iterator;
 
 private:
+    class Node
+    {
+    public:
         Node(const T &data) : data(data) {}
         Node(T &&data) : data(std::forward(data)) {}
 
@@ -43,40 +45,43 @@ class List
         std::atomic<bool> removed{false};
     };
 
-    class Iterator
+    template <class It>
+    class IteratorBase : public Crtp<It>
     {
         friend class List;
 
-        Iterator() : list(nullptr), curr(nullptr) {}
+    protected:
+        IteratorBase() : list(nullptr), curr(nullptr) {}
 
-        Iterator(List *list) : list(list)
+        IteratorBase(List *list) : list(list)
         {
             list->count++;
             reset();
         }
 
     public:
-        Iterator(const Accessor &) = delete;
+        IteratorBase(const IteratorBase &) = delete;
 
-        Iterator(Iterator &&other)
+        IteratorBase(IteratorBase &&other)
             : list(other.list), curr(other.curr), prev(other.prev)
         {
             other.list = nullptr;
             other.curr = nullptr;
         }
 
-        ~Iterator()
+        ~IteratorBase()
         {
             if (list == nullptr) {
                 return;
             }
 
-            auto head_rem = load(list->head_rem);
+            auto head_rem = load(list->removed);
             // Fetch could be relaxed
             // There exist possibility that no one will delete garbage at this
             // time.
-            if (list.count.fetch_sub(1) == 1 && head_rem != nullptr &&
+            if (list->count.fetch_sub(1) == 1 && head_rem != nullptr &&
-                cas(list->head_rem, head_rem,
+                cas<Node *>(
+                    list->removed, head_rem,
                     nullptr)) { // I am the last one and there is garbage to be
                                 // removed.
                 auto now = head_rem;
@@ -91,30 +96,32 @@ class List
         T &operator*()
         {
             assert(valid());
-            return *curr;
+            return curr->data;
         }
         T *operator->()
         {
             assert(valid());
-            return curr;
+            return curr->data;
         }
         operator T *()
         {
             assert(valid());
-            return curr;
+            return curr->data;
         }
 
         bool valid() { return curr != nullptr; }
 
-        Iterator &operator++()
+        // Iterating is wait free.
+        It &operator++()
         {
             assert(valid());
             do {
                 prev = curr;
                 curr = load(curr->next);
             } while (valid() && is_removed());
-            return this;
+            return this->derived();
         }
+        It &operator++(int) { return operator++(); }
 
         bool is_removed()
         {
@@ -122,28 +129,36 @@ class List
             return load(curr->removed);
         }
 
-        // Returns iterator to begining
+        // Returns IteratorBase to begining
         void reset()
         {
             prev = nullptr;
             curr = load(list->head);
             while (valid() && is_removed()) {
-                this ++;
+                operator++();
             }
         }
 
         // Adds to the begining of list
+        // It is lock free but it isn't wait free.
         void push(T &&data)
         {
             auto node = new Node(data);
-            auto next = nullptr;
+            Node *next = nullptr;
             do {
                 next = load(list->head);
-                store(next.next, next);
+                store(node->next, next);
             } while (!cas(list->head, next, node));
         }
 
-        // True only if this call removed the element.
+        // True only if this call removed the element. Only reason for fail is
+        // if
+        // the element is already removed.
+        // Remove has deadlock if another thread dies between marking node for
+        // removal
+        // and the disconnection.
+        // This can be improved with combinig the removed flag with prev.next or
+        // curr.next
         bool remove()
         {
             assert(valid());
@@ -157,28 +172,25 @@ class List
             return false;
         }
 
-        friend bool operator==(const Iterator &a, const Iterator &b)
+        friend bool operator==(const It &a, const It &b)
         {
-            return a->curr == b->curr;
+            return a.curr == b.curr;
         }
 
-        friend bool operator!=(const Iterator &a, const Iterator &b)
+        friend bool operator!=(const It &a, const It &b) { return !(a == b); }
-        {
-            return !(a == b);
-        }
 
     private:
         void find_and_disconnect()
         {
-            auto it = Iterator(list);
+            auto it = It(list);
             auto next = load(curr->next);
             while (it.valid()) {
-                if (it.succ == succ) {
+                if (it.curr == curr) {
                     if (it.disconnect()) {
                         return;
                     }
                     it.reset();
-                } else if (it.succ == next) { // Comparison with next is
+                } else if (it.curr == next) { // Comparison with next is
                                               // optimization for early return.
                     return;
                 } else {
@@ -206,6 +218,38 @@ class List
         Node *curr;
     };
 
+public:
+    class ConstIterator : public IteratorBase<ConstIterator>
+    {
+        friend class List;
+
+    public:
+        using IteratorBase<ConstIterator>::IteratorBase;
+
+        const T &operator*()
+        {
+            return IteratorBase<ConstIterator>::operator*();
+        }
+
+        const T *operator->()
+        {
+            return IteratorBase<ConstIterator>::operator->();
+        }
+
+        operator const T &()
+        {
+            return IteratorBase<ConstIterator>::operator T &();
+        }
+    };
+
+    class Iterator : public IteratorBase<Iterator>
+    {
+        friend class List;
+
+    public:
+        using IteratorBase<Iterator>::IteratorBase;
+    };
+
 public:
     List() = default;
 
@@ -216,10 +260,18 @@ public:
 
     Iterator begin() { return Iterator(this); }
 
+    ConstIterator begin() const { return ConstIterator(this); }
+
+    ConstIterator cbegin() const { return ConstIterator(this); }
+
     Iterator end() { return Iterator(); }
 
+    ConstIterator end() const { return ConstIterator(); }
+
+    ConstIterator cend() const { return ConstIterator(); }
+
 private:
     std::atomic<size_t> count{0};
     std::atomic<Node *> head{nullptr};
     std::atomic<Node *> removed{nullptr};
-}
+};

tests/unit/concurrent_list.cpp

@@ -0,0 +1,75 @@
+#define CATCH_CONFIG_MAIN
+#include "catch.hpp"
+
+#include "data_structures/list/lockfree_list_new.hpp"
+
+TEST_CASE("Conncurent List insert")
+{
+    List<int> list;
+    auto it = list.begin();
+    it.push(32);
+    it.reset();
+    REQUIRE(*it == 32);
+}
+
+TEST_CASE("Conncurent List iterate")
+{
+    List<int> list;
+    auto it = list.begin();
+    it.push(32);
+    it.push(7);
+    it.push(9);
+    it.push(0);
+    it.reset();
+
+    REQUIRE(*it == 0);
+    it++;
+    REQUIRE(*it == 9);
+    it++;
+    REQUIRE(*it == 7);
+    it++;
+    REQUIRE(*it == 32);
+    it++;
+    REQUIRE(it == list.end());
+}
+
+TEST_CASE("Conncurent List head remove")
+{
+    List<int> list;
+    auto it = list.begin();
+    it.push(32);
+    it.reset();
+
+    REQUIRE(it.remove());
+    REQUIRE(it.is_removed());
+    REQUIRE(!it.remove());
+
+    it.reset();
+    REQUIRE(it == list.end());
+}
+
+TEST_CASE("Conncurent List remove")
+{
+    List<int> list;
+    auto it = list.begin();
+    it.push(32);
+    it.push(7);
+    it.push(9);
+    it.push(0);
+    it.reset();
+
+    it++;
+    it++;
+    REQUIRE(it.remove());
+    REQUIRE(it.is_removed());
+    REQUIRE(!it.remove());
+
+    it.reset();
+    REQUIRE(*it == 0);
+    it++;
+    REQUIRE(*it == 9);
+    it++;
+    REQUIRE(*it == 32);
+    it++;
+    REQUIRE(it == list.end());
+}