aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBenoit Germain <benoit.germain@ubisoft.com>2024-11-13 11:05:32 +0100
committerBenoit Germain <benoit.germain@ubisoft.com>2024-11-13 11:05:32 +0100
commit2f2f29391012949385631e5bee7c35fed71392dd (patch)
tree382a2bbaa7dcbf5bda3ca322626d29d539f6f738
parentf2a3c033fc31332e78aa45d2d9deaf51359b584a (diff)
downloadlanes-2f2f29391012949385631e5bee7c35fed71392dd.tar.gz
lanes-2f2f29391012949385631e5bee7c35fed71392dd.tar.bz2
lanes-2f2f29391012949385631e5bee7c35fed71392dd.zip
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
-rw-r--r--src/_pch.hpp1
-rw-r--r--src/lane.cpp7
-rw-r--r--src/lane.hpp5
-rw-r--r--src/linda.cpp4
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"
60#pragma warning(disable : 5027) // 'x': move assignment operator was implicitly defined as deleted 60#pragma warning(disable : 5027) // 'x': move assignment operator was implicitly defined as deleted
61#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. 61#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.
62#pragma warning(disable : 5045) // Compiler will insert Spectre mitigation for memory load if /Qspectre switch specified 62#pragma warning(disable : 5045) // Compiler will insert Spectre mitigation for memory load if /Qspectre switch specified
63#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
64#pragma warning(disable : 5246) // 'x': the initialization of a subobject should be wrapped in braces 63#pragma warning(disable : 5246) // 'x': the initialization of a subobject should be wrapped in braces
65 64
66#endif // _MSC_VER 65#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
945 // lane_->thread.get_stop_source().request_stop(); 945 // lane_->thread.get_stop_source().request_stop();
946 } 946 }
947 if (wakeLane_ == WakeLane::Yes) { // wake the thread so that execution returns from any pending linda operation if desired 947 if (wakeLane_ == WakeLane::Yes) { // wake the thread so that execution returns from any pending linda operation if desired
948 std::condition_variable* const _waiting_on{ waiting_on }; 948 if (status.load(std::memory_order_acquire) == Lane::Waiting) { // waiting_on is updated under control of status acquire/release semantics
949 if (status.load(std::memory_order_acquire) == Lane::Waiting && _waiting_on != nullptr) { 949 if (std::condition_variable* const _waiting_on{ waiting_on }) {
950 _waiting_on->notify_all(); 950 _waiting_on->notify_all();
951 }
951 } 952 }
952 } 953 }
953 // wait until the lane stops working with its state (either Suspended or Done+) 954 // 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
50 Yes 50 Yes
51}; 51};
52 52
53// NOTE: values to be changed by either thread, during execution, without locking, are marked "volatile"
54class Lane 53class Lane
55{ 54{
56 public: 55 public:
@@ -118,9 +117,9 @@ class Lane
118 // M: sets to Pending (before launching) 117 // M: sets to Pending (before launching)
119 // S: updates -> Running/Waiting/Suspended -> Done/Error/Cancelled 118 // S: updates -> Running/Waiting/Suspended -> Done/Error/Cancelled
120 119
121 std::condition_variable* volatile waiting_on{ nullptr }; 120 // accessed under control of status acquire/release semantics
122 //
123 // When status is Waiting, points on the linda's signal the thread waits on, else nullptr 121 // When status is Waiting, points on the linda's signal the thread waits on, else nullptr
122 std::condition_variable* waiting_on{ nullptr };
124 123
125 std::atomic<CancelRequest> cancelRequest{ CancelRequest::None }; 124 std::atomic<CancelRequest> cancelRequest{ CancelRequest::None };
126 static_assert(std::atomic<CancelRequest>::is_always_lock_free); 125 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)
673 // change status of lane to "waiting" 673 // change status of lane to "waiting"
674 _prev_status = _lane->status.load(std::memory_order_acquire); // Running, most likely 674 _prev_status = _lane->status.load(std::memory_order_acquire); // Running, most likely
675 LUA_ASSERT(L_, _prev_status == Lane::Running); // but check, just in case 675 LUA_ASSERT(L_, _prev_status == Lane::Running); // but check, just in case
676 _lane->status.store(Lane::Waiting, std::memory_order_release);
677 LUA_ASSERT(L_, _lane->waiting_on == nullptr); 676 LUA_ASSERT(L_, _lane->waiting_on == nullptr);
678 _lane->waiting_on = &_linda->writeHappened; 677 _lane->waiting_on = &_linda->writeHappened;
678 _lane->status.store(Lane::Waiting, std::memory_order_release);
679 } 679 }
680 // not enough data to read: wakeup when data was sent, or when timeout is reached 680 // not enough data to read: wakeup when data was sent, or when timeout is reached
681 std::unique_lock<std::mutex> _guard{ _keeper->mutex, std::adopt_lock }; 681 std::unique_lock<std::mutex> _guard{ _keeper->mutex, std::adopt_lock };
@@ -818,9 +818,9 @@ LUAG_FUNC(linda_send)
818 // change status of lane to "waiting" 818 // change status of lane to "waiting"
819 _prev_status = _lane->status.load(std::memory_order_acquire); // Running, most likely 819 _prev_status = _lane->status.load(std::memory_order_acquire); // Running, most likely
820 LUA_ASSERT(L_, _prev_status == Lane::Running); // but check, just in case 820 LUA_ASSERT(L_, _prev_status == Lane::Running); // but check, just in case
821 _lane->status.store(Lane::Waiting, std::memory_order_release);
822 LUA_ASSERT(L_, _lane->waiting_on == nullptr); 821 LUA_ASSERT(L_, _lane->waiting_on == nullptr);
823 _lane->waiting_on = &_linda->readHappened; 822 _lane->waiting_on = &_linda->readHappened;
823 _lane->status.store(Lane::Waiting, std::memory_order_release);
824 } 824 }
825 // could not send because no room: wait until some data was read before trying again, or until timeout is reached 825 // could not send because no room: wait until some data was read before trying again, or until timeout is reached
826 std::unique_lock<std::mutex> _guard{ _keeper->mutex, std::adopt_lock }; 826 std::unique_lock<std::mutex> _guard{ _keeper->mutex, std::adopt_lock };