Cleaning up guano

* Lane::waiting on does not need to be volatile or anything else, all accesses are controlled through status acquire/release semantics
* this contains fixes about bad ordering of said accesses in Linda:send and Linda:receive

4 files changed, 8 insertions, 9 deletions

diff --git a/src/_pch.hpp b/src/_pch.hpp
index 1c7b7dc..495a959 100644
--- a/src/_pch.hpp
+++ b/src/_pch.hpp
@@ -60,7 +60,6 @@ extern "C"
 #pragma warning(disable : 5027) // 'x': move assignment operator was implicitly defined as deleted
 #pragma warning(disable : 5039) // 'x': pointer or reference to potentially throwing function passed to 'extern "C"' function under -EHc. Undefined behavior may occur if this function throws an exception.
 #pragma warning(disable : 5045) // Compiler will insert Spectre mitigation for memory load if /Qspectre switch specified
-#pragma warning(disable : 5220) // 'x': a non-static data member with a volatile qualified type no longer implies that compiler generated copy/move constructors and copy/move assignment operators are not trivial
 #pragma warning(disable : 5246) // 'x': the initialization of a subobject should be wrapped in braces
 
 #endif // _MSC_VER
diff --git a/src/lane.cpp b/src/lane.cpp
index 2bc3431..00f857e 100644
--- a/src/lane.cpp
+++ b/src/lane.cpp
@@ -945,9 +945,10 @@ CancelResult Lane::cancel(CancelOp const op_, std::chrono::time_point<std::chron
         // lane_->thread.get_stop_source().request_stop();
     }
     if (wakeLane_ == WakeLane::Yes) { // wake the thread so that execution returns from any pending linda operation if desired
-        std::condition_variable* const _waiting_on{ waiting_on };
-        if (status.load(std::memory_order_acquire) == Lane::Waiting && _waiting_on != nullptr) {
-            _waiting_on->notify_all();
+        if (status.load(std::memory_order_acquire) == Lane::Waiting) { // waiting_on is updated under control of status acquire/release semantics
+            if (std::condition_variable* const _waiting_on{ waiting_on }) {
+                _waiting_on->notify_all();
+            }
         }
     }
     // wait until the lane stops working with its state (either Suspended or Done+)
diff --git a/src/lane.hpp b/src/lane.hpp
index eb51aa3..b5be9ab 100644
--- a/src/lane.hpp
+++ b/src/lane.hpp
@@ -50,7 +50,6 @@ enum class WakeLane
     Yes
 };
 
-// NOTE: values to be changed by either thread, during execution, without locking, are marked "volatile"
 class Lane
 {
     public:
@@ -118,9 +117,9 @@ class Lane
     // M: sets to Pending (before launching)
     // S: updates -> Running/Waiting/Suspended -> Done/Error/Cancelled
 
-    std::condition_variable* volatile waiting_on{ nullptr };
-    //
+    // accessed under control of status acquire/release semantics
     // When status is Waiting, points on the linda's signal the thread waits on, else nullptr
+    std::condition_variable* waiting_on{ nullptr };
 
     std::atomic<CancelRequest> cancelRequest{ CancelRequest::None };
     static_assert(std::atomic<CancelRequest>::is_always_lock_free);
diff --git a/src/linda.cpp b/src/linda.cpp
index 1536da5..99d0cbe 100644
--- a/src/linda.cpp
+++ b/src/linda.cpp
@@ -673,9 +673,9 @@ LUAG_FUNC(linda_receive)
                         // change status of lane to "waiting"
                         _prev_status = _lane->status.load(std::memory_order_acquire); // Running, most likely
                         LUA_ASSERT(L_, _prev_status == Lane::Running); // but check, just in case
-                        _lane->status.store(Lane::Waiting, std::memory_order_release);
                         LUA_ASSERT(L_, _lane->waiting_on == nullptr);
                         _lane->waiting_on = &_linda->writeHappened;
+                        _lane->status.store(Lane::Waiting, std::memory_order_release);
                     }
                     // not enough data to read: wakeup when data was sent, or when timeout is reached
                     std::unique_lock<std::mutex> _guard{ _keeper->mutex, std::adopt_lock };
@@ -818,9 +818,9 @@ LUAG_FUNC(linda_send)
                             // change status of lane to "waiting"
                             _prev_status = _lane->status.load(std::memory_order_acquire); // Running, most likely
                             LUA_ASSERT(L_, _prev_status == Lane::Running); // but check, just in case
-                            _lane->status.store(Lane::Waiting, std::memory_order_release);
                             LUA_ASSERT(L_, _lane->waiting_on == nullptr);
                             _lane->waiting_on = &_linda->readHappened;
+                            _lane->status.store(Lane::Waiting, std::memory_order_release);
                         }
                         // could not send because no room: wait until some data was read before trying again, or until timeout is reached
                         std::unique_lock<std::mutex> _guard{ _keeper->mutex, std::adopt_lock };