Merge pull request #579 from evoskuil/master

Restore hash_head memory fence with corrected CAS writer.

Author: evoskuil

include/bitcoin/database/error.hpp

@@ -48,6 +48,12 @@ enum error_t : uint8_t
     integrity9,
     integrity10,
     integrity11,
+    integrity12,
+    integrity13,
+    integrity14,
+    integrity15,
+    integrity16,
+    integrity17,
 
     /// memory map
     open_open,

include/bitcoin/database/impl/primitives/arrayhead.ipp

@@ -137,7 +137,7 @@ Link CLASS::at(size_t key) const NOEXCEPT
         // xcode clang++16 does not support C++20 std::atomic_ref.
         ////const std::atomic_ref<integer> head(unsafe_byte_cast<integer>(raw));
         const auto& head = *pointer_cast<std::atomic<integer>>(raw);
-        return head.load(std::memory_order_acquire);
+        return head.load(std::memory_order_relaxed);
     }
     else
     {
@@ -167,7 +167,7 @@ bool CLASS::push(const Link& link, const Link& index) NOEXCEPT
         // xcode clang++16 does not support C++20 std::atomic_ref.
         ////const std::atomic_ref<integer> head(unsafe_byte_cast<integer>(raw));
         auto& head = *pointer_cast<std::atomic<integer>>(raw);
-        head.store(link, std::memory_order_acq_rel);
+        head.store(link, std::memory_order_relaxed);
     }
     else
     {

include/bitcoin/database/impl/primitives/hashhead.ipp

@@ -135,6 +135,9 @@ inline Link CLASS::top(const Link& index) const NOEXCEPT
         // xcode clang++16 does not support C++20 std::atomic_ref.
         ////const std::atomic_ref<integer> head(unsafe_byte_cast<integer>(raw));
         const auto& head = *pointer_cast<std::atomic<integer>>(raw);
+
+        // Acquire is necessary to synchronize with push release.
+        // Relaxed would miss next updates, so acquire is optimal.
         return head.load(std::memory_order_acquire);
     }
     else
@@ -169,7 +172,19 @@ inline bool CLASS::push(const Link& current, bytes& next,
         // xcode clang++16 does not support C++20 std::atomic_ref.
         ////const std::atomic_ref<integer> head(unsafe_byte_cast<integer>(raw));
         auto& head = *pointer_cast<std::atomic<integer>>(raw);
-        next = Link(head.exchange(current, std::memory_order_acq_rel));
+
+        integer top = head.load(std::memory_order_acquire);
+        do
+        {
+            // Compiler could order this after head.store, which would expose key
+            // to search before next entry is linked. Thread fence imposes order.
+            // A release fence ensures that all prior writes (like next) are
+            // completed before any subsequent atomic store. 
+            next = Link{ top };
+            std::atomic_thread_fence(std::memory_order_release);
+        }
+        while (!head.compare_exchange_weak(top, current,
+            std::memory_order_release, std::memory_order_acquire));
     }
     else
     {

include/bitcoin/database/impl/query/consensus.ipp

@@ -204,7 +204,29 @@ code CLASS::push_spenders(tx_links& out, const point& point,
 {
     auto it = store_.point.it(table::point::compose(point));
     if (!it)
+    {
+        const auto key1 = it.key();
+        const auto link1 = it.self();
+        const auto get1 = it.get();
+        it.reset();
+
+        if (key1 != table::point::compose(point))
+            return error::integrity12;
+        if (!link1.is_terminal())
+            return error::integrity13;
+        if (is_null(get1))
+            return error::integrity14;
+
+        table::point::record get{};
+        if (!store_.point.get(self, get))
+            return error::integrity15;
+        if (get.hash != point.hash())
+            return error::integrity16;
+        if (get.index != point.index())
+            return error::integrity17;
+
         return error::integrity4;
+    }
 
     point_links points{};
     do

include/bitcoin/database/primitives/iterator.hpp

@@ -50,17 +50,16 @@ class iterator
     iterator(memory_ptr&& data, const Link& start, Key&& key) NOEXCEPT;
     iterator(memory_ptr&& data, const Link& start, const Key& key) NOEXCEPT;
 
-    /// Advance to and return next iterator.
+    /// Advance to next and return false if none found.
     inline bool advance() NOEXCEPT;
 
     /// Expose the search key.
     inline const Key& key() const NOEXCEPT;
 
-    /// Advance to next match and return false if terminal (not found).
+    /// Return current link, terminal if not found.
     inline const Link& self() const NOEXCEPT;
 
     /// Access the underlying memory pointer.
-    // TODO: for use by hashmap, make exclusive via friend.
     inline const memory_ptr& get() const NOEXCEPT;
 
     /// Release the memory pointer, invalidates iterator.

src/error.cpp

@@ -41,6 +41,12 @@ DEFINE_ERROR_T_MESSAGE_MAP(error)
     { integrity9, "store corrupted9" },
     { integrity10, "store corrupted10" },
     { integrity11, "store corrupted11" },
+    { integrity12, "store corrupted12" },
+    { integrity13, "store corrupted13" },
+    { integrity14, "store corrupted14" },
+    { integrity15, "store corrupted15" },
+    { integrity16, "store corrupted16" },
+    { integrity17, "store corrupted17" },
 
     // memory map
     { open_open, "opening open file" },