Stores to m_leftright field need seq_cst

With a test too. The test needs -O3 on my mache to repro.

Closes #3

Author: mmcshane

CMakeLists.txt

@@ -2,7 +2,7 @@ cmake_minimum_required (VERSION 3.2.2)
 
 set(LEFTRIGHT_VERSION_MAJOR 1)
 set(LEFTRIGHT_VERSION_MINOR 1)
-set(LEFTRIGHT_VERSION_PATCH 1)
+set(LEFTRIGHT_VERSION_PATCH 3)
 set(LEFTRIGHT_VERSION ${LEFTRIGHT_VERSION_MAJOR}.${LEFTRIGHT_VERSION_MINOR}.${LEFTRIGHT_VERSION_PATCH})
 
 project(leftright VERSION ${LEFTRIGHT_VERSION} LANGUAGES CXX)

README.md

@@ -37,7 +37,9 @@ want to run the unit tests or generate docs you can use the cmake build. To
 perform an out-of-tree build
 
     $ cd "$(mktemp -d)"
-    $ cmake -DCMAKE_BUILD_TYPE=debug /path/to/leftright/repository
+    $ cmake -DCMAKE_BUILD_TYPE=release /path/to/leftright/repository
     $ make && make test
     $ make docs # generates doxygen documentation under docs/
 
+Use of release builds is suggested here as optimizations are more likely to
+trigger concurrency bugs.

include/mpm/leftright.h

@@ -291,14 +291,14 @@ namespace mpm
         if(read_left == m_leftright.load(std::memory_order_relaxed))
         {
             f(m_right);
-            m_leftright.store(read_right, std::memory_order_release);
+            m_leftright.store(read_right, std::memory_order_seq_cst);
             toggle_reader_registry(xlock);
             return f(m_left);
         }
         else
         {
             f(m_left);
-            m_leftright.store(read_left, std::memory_order_release);
+            m_leftright.store(read_left, std::memory_order_seq_cst);
             toggle_reader_registry(xlock);
             return f(m_right);
         }

test/test_leftright.cpp

@@ -1,6 +1,6 @@
 #include <mpm/leftright.h>
+#include <atomic>
 #include <map>
-#include <string>
 #include <catch2/catch.hpp>
 
 
@@ -78,6 +78,48 @@ TEST_CASE("noexcept specs", "[mpm.leftright]")
     //auto null_modifier = [](lrint::reference i){ return i; };
     //CHECK(0 == lri.modify(null_modifier));
 }
+TEST_CASE("hammer it", "[mpm.leftright]")
+{
+    // repro of https://github.com/mmcshane/leftright/issues/3
+
+    // this test may require compiler optimizations (e.g. -O3) to fail as
+    // expected. To induce failure, change the memory order used in the stores
+    // to basic_leftright::m_leftright performed in the two branches of
+    // basic_leftright::modify to std::memory_order_release.
+
+    struct Bigish {
+      public:
+        void add1() { a1 += 1; a2 += 1; a3 += 1; a4 += 1; }
+
+        alignas(MPM_LEFTRIGHT_CACHE_LINE_SIZE) int a1 = 0;
+        alignas(MPM_LEFTRIGHT_CACHE_LINE_SIZE) int a2 = 0;
+        alignas(MPM_LEFTRIGHT_CACHE_LINE_SIZE) int a3 = 0;
+        alignas(MPM_LEFTRIGHT_CACHE_LINE_SIZE) int a4 = 0;
+    };
+
+    mpm::leftright<Bigish> lrb{};
+
+    std::atomic_uint_fast32_t violations{0};
+    std::thread reader([&] {
+        for(int i = 0; i < 10000000; i++) {
+            auto observed = lrb.observe([](const Bigish& val) noexcept{ return val; });
+            auto a1 = observed.a1;
+            if (observed.a2 != a1 || observed.a3 != a1 || observed.a4 != a1) {
+                violations.fetch_add(1, std::memory_order_relaxed);
+            }
+        }
+    });
+
+    Bigish in{};
+    for(int i = 0; i < 10000000; i++){
+        in.add1();
+        lrb.modify([&](Bigish& b)noexcept{ b = in; });
+    }
+
+    reader.join();
+
+    CHECK(0 == violations.load(std::memory_order_relaxed));
+}
 
 /*
 struct unsafe_reader_registry