Document necessary memory barriers by adding appropriate tests

[ppetr]

I tried this patch (for 362a5a5b) to test if using std::memory_order_relaxed leads to observable undesirable side-effects on ARMv7 (inspired by this blog post), but so far without success.

diff -r 94f0dbefaa29 CMakeLists.txt
--- a/CMakeLists.txt	Sun Feb 06 14:47:10 2022 +0100
+++ b/CMakeLists.txt	Sun Feb 06 16:06:04 2022 +0100
@@ -17,6 +17,8 @@
 enable_testing()
 
 set(CMAKE_CXX_STANDARD 11)
+set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -march=native")
+
 # Set variables for subdirectories.
 # See https://stackoverflow.com/a/3769269/1333025.
 set(BENCHMARK_ENABLE_TESTING OFF CACHE BOOL
diff -r 94f0dbefaa29 simple_rcu/CMakeLists.txt
--- a/simple_rcu/CMakeLists.txt	Sun Feb 06 14:47:10 2022 +0100
+++ b/simple_rcu/CMakeLists.txt	Sun Feb 06 16:06:04 2022 +0100
@@ -17,7 +17,7 @@
 #target_link_libraries(local_3state_rcu INTERFACE absl::...)
 
 add_executable(local_3state_rcu_test local_3state_rcu_test.cc)
-target_link_libraries(local_3state_rcu_test gtest_main)
+target_link_libraries(local_3state_rcu_test local_3state_rcu absl::utility gtest_main)
 add_test(NAME local_3state_rcu_test COMMAND local_3state_rcu_test)
 
 add_library(rcu INTERFACE)
diff -r 94f0dbefaa29 simple_rcu/local_3state_rcu.h
--- a/simple_rcu/local_3state_rcu.h	Sun Feb 06 14:47:10 2022 +0100
+++ b/simple_rcu/local_3state_rcu.h	Sun Feb 06 16:06:04 2022 +0100
@@ -74,7 +74,7 @@
   // the reference pointed to by `Read()` remains unchanged.
   bool TriggerRead() noexcept {
     Index next_read_index =
-        next_read_index_.exchange(kNullIndex, std::memory_order_acq_rel);
+        next_read_index_.exchange(kNullIndex, std::memory_order_relaxed);
     if (next_read_index != kNullIndex) {
       read_.index = next_read_index;
       return true;
@@ -97,7 +97,7 @@
   // invalidated.
   bool TriggerUpdate() noexcept {
     Index old_next_read_index =
-        next_read_index_.exchange(update_.index, std::memory_order_acq_rel);
+        next_read_index_.exchange(update_.index, std::memory_order_relaxed);
     if (old_next_read_index == kNullIndex) {
       update_.RotateAfterNext();
       return true;
@@ -120,7 +120,7 @@
   bool TryTriggerUpdate() noexcept {
     Index old_next_read_index = kNullIndex;
     if (next_read_index_.compare_exchange_strong(
-            old_next_read_index, update_.index, std::memory_order_acq_rel)) {
+            old_next_read_index, update_.index, std::memory_order_relaxed)) {
       update_.RotateAfterNext();
       return true;
     } else {
diff -r 94f0dbefaa29 simple_rcu/local_3state_rcu_test.cc
--- a/simple_rcu/local_3state_rcu_test.cc	Sun Feb 06 14:47:10 2022 +0100
+++ b/simple_rcu/local_3state_rcu_test.cc	Sun Feb 06 16:06:04 2022 +0100
@@ -14,6 +14,10 @@
 
 #include "simple_rcu/local_3state_rcu.h"
 
+#include <atomic>
+#include <thread>
+
+#include "absl/utility/utility.h"
 #include "gtest/gtest.h"
 
 namespace simple_rcu {
@@ -129,5 +133,34 @@
   }
 }
 
+TEST(Local3StateRcuTest, ProperlyMemoryOrdered) {
+  Local3StateRcu<bool> rcu(false);
+  std::atomic<bool> finished(false);
+  std::thread updater_thread([&]() {
+    while (!finished.load(std::memory_order_relaxed)) {
+      rcu.Update() = false;
+      if (rcu.TriggerUpdate()) {
+        ASSERT_TRUE(rcu.Update()) << "Update observed a stale value";
+      } else {
+        rcu.Update() = true;
+        std::this_thread::yield();
+      }
+    }
+  });
+  for (int i = 0; i < 1000000;) {
+    rcu.Read() = true;
+    if (rcu.TriggerRead()) {
+      ASSERT_FALSE(rcu.Read())
+          << "Read observed a stale value at iteration " << i;
+      i++;
+    } else {
+      rcu.Read() = false;
+      std::this_thread::yield();
+    }
+  }
+  finished.store(true);
+  updater_thread.join();
+}
+
 }  // namespace
 }  // namespace simple_rcu

[ppetr]

I tested this on Raspberry Pi 4 with both ARMv8 (64-bit) and ARMv7 (32-bit) systems, but couldn't trigger a test failure.

I also tried to make just one thread to write and the other to read, instead of the above bi-directional approach, but also no luck.